Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: add support for cross-compilation #152

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 21 additions & 33 deletions crate2nix/Cargo.nix
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
, pkgs ? import nixpkgs { config = {}; }
, lib ? pkgs.lib
, stdenv ? pkgs.stdenv
, buildRustCrate ? pkgs.buildRustCrate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to remove this argument? If yes, why?

[I just like to keep top-level interface changes minimally, and for whatever reason I made this a top-level argument.]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kolloch see what @lopsided98 said in the PR description:

Unfortunately, I haven't figured out a way to do this without potentially breaking existing users, as well as the debugging tools. This is because there is no longer a single version of buildRustCrate used by all crates, and instead there are separate ones for each platform. This makes it difficult to pass around buildRustCrateFunc.

I can attest that this makes sense from the usual cross perspective.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do

buildRustCrate' ? pkgs: pkgs.buildRustCrate

if you prefer, @kolloch. That will work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and buildRustCrate could be deprecated and given a null default, so we can migrate with

Suggested change
, buildRustCrate ? pkgs.buildRustCrate
, buildRustCrate ? null
, buildRustCrateFromPkgs ? if buildRustCrate != null then _: buildRustCrate else traceDeprecatedMessage (pkgs: pkgs.buildRustCrate)

With traceDeprecatedMessage left as a bikeshed to the reader :).

# This is used as the `crateOverrides` argument for `buildRustCrate`.
, defaultCrateOverrides ? pkgs.defaultCrateOverrides
# The features to enable for the root_crate or the workspace_members.
Expand Down Expand Up @@ -2291,7 +2290,6 @@ rec {
{ packageId
, features ? rootFeatures
, crateOverrides ? defaultCrateOverrides
, buildRustCrateFunc ? null
, runTests ? false
, testCrateFlags ? [ ]
, testInputs ? [ ]
Expand All @@ -2305,26 +2303,18 @@ rec {
, testInputs
}:
let
buildRustCrateFuncOverriden =
if buildRustCrateFunc != null
then buildRustCrateFunc
else
(
if crateOverrides == pkgs.defaultCrateOverrides
then buildRustCrate
else
buildRustCrate.override {
defaultCrateOverrides = crateOverrides;
}
);
buildRustCrateOverrides =
if crateOverrides == pkgs.defaultCrateOverrides
then { }
else {
defaultCrateOverrides = crateOverrides;
};
builtRustCrates = builtRustCratesWithFeatures {
inherit packageId features;
buildRustCrateFunc = buildRustCrateFuncOverriden;
inherit packageId features buildRustCrateOverrides;
runTests = false;
};
builtTestRustCrates = builtRustCratesWithFeatures {
inherit packageId features;
buildRustCrateFunc = buildRustCrateFuncOverriden;
inherit packageId features buildRustCrateOverrides;
runTests = true;
};
drv = builtRustCrates.${packageId};
Expand All @@ -2350,7 +2340,7 @@ rec {
{ packageId
, features
, crateConfigs ? crates
, buildRustCrateFunc
, buildRustCrateOverrides
, runTests
, target ? defaultTarget
} @ args:
Expand All @@ -2368,13 +2358,10 @@ rec {
target = target // { test = runTests; };
}
);
buildByPackageId = packageId: buildByPackageIdImpl packageId;

# Memoize built packages so that reappearing packages are only built once.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can do this again now that the hash thing has been changed in NixOS/nixpkgs#105305?

On the other hand, isn't the comment wrong? I would assume this is just saving eval time, and the derivations will be identical and thus deduplicated either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was also that this was an eval optimization.

I don't think that PR affects this. packageId doesn't have anything to do with the hash; IIRC packageId is just the name and possibly the version if there are multiple. Even if you were to extend it to include the platform, it is not obvious which packages need to be built for which platforms until you walk through the dependency tree.

Copy link
Collaborator

@Ericson2314 Ericson2314 Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK fair point, then I think it might be nice to do the same corecursive stuff we do in nixpkgs?

let
buildByPackageIdForPkgsMemo = mkBuildByPackageIdForPkgsMemo pkgs;
mkBuildByPackageIdForPkgsMemo = pkgs: {
  crates = lib.mapAttrs (packageId: value: buildByPackageIdForPkgs pkgs packageId) crateConfigs;
  build = mkBuildByPackageIdForPkgsMemo pkgs.buildPackages;
};

It's fine that this contains too much stuff, again cause lazy eval: a good time for space tradeoff.

You use crates for dependencies, build.crates for dev-dependencies, and a nasty backtracking of switching from crates to build.crates for any proc macro crate. (Shame on them for not making proc macros a different type of dependency! I did mention it when proc macros in Cargo first came out...)

builtByPackageId =
lib.mapAttrs (packageId: value: buildByPackageId packageId) crateConfigs;
buildByPackageIdImpl = packageId:
buildByPackageIdForPkgs = pkgs: packageId:
let
# proc_macro crates must be compiled for the build architecture
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love that you specify the reason here in a comment.

cratePkgs = if crateConfig.procMacro or false then pkgs.buildPackages else pkgs;
features = mergedFeatures."${packageId}" or [ ];
crateConfig' = crateConfigs."${packageId}";
crateConfig =
Expand All @@ -2385,14 +2372,16 @@ rec {
(crateConfig'.devDependencies or [ ]);
dependencies =
dependencyDerivations {
inherit builtByPackageId features target;
inherit features target;
buildByPackageId = buildByPackageIdForPkgs cratePkgs;
dependencies =
(crateConfig.dependencies or [ ])
++ devDependencies;
};
buildDependencies =
dependencyDerivations {
inherit builtByPackageId features target;
inherit features target;
buildByPackageId = buildByPackageIdForPkgs cratePkgs.buildPackages;
dependencies = crateConfig.buildDependencies or [ ];
};
filterEnabledDependenciesForThis = dependencies: filterEnabledDependencies {
Expand Down Expand Up @@ -2424,13 +2413,13 @@ rec {
dependenciesWithRenames;
versionAndRename = dep:
let
package = builtByPackageId."${dep.packageId}";
package = crateConfigs."${dep.packageId}";
in
{ inherit (dep) rename; version = package.version; };
in
lib.mapAttrs (name: choices: builtins.map versionAndRename choices) grouped;
in
buildRustCrateFunc
cratePkgs.buildRustCrate.override buildRustCrateOverrides
(
crateConfig // {
src = crateConfig.src or (
Expand All @@ -2449,24 +2438,23 @@ rec {
}
);
in
builtByPackageId;
lib.mapAttrs (packageId: value: buildByPackageIdForPkgs pkgs packageId) crateConfigs;

/* Returns the actual derivations for the given dependencies. */
dependencyDerivations =
{ builtByPackageId
{ buildByPackageId
, features
, dependencies
, target
}:
assert (builtins.isAttrs builtByPackageId);
assert (builtins.isList features);
assert (builtins.isList dependencies);
assert (builtins.isAttrs target);
let
enabledDependencies = filterEnabledDependencies {
inherit dependencies features target;
};
depDerivation = dependency: builtByPackageId.${dependency.packageId};
depDerivation = dependency: buildByPackageId dependency.packageId;
in
map depDerivation enabledDependencies;

Expand Down
1 change: 0 additions & 1 deletion crate2nix/templates/Cargo.nix.tera
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
, pkgs ? import nixpkgs { config = {}; }
, lib ? pkgs.lib
, stdenv ? pkgs.stdenv
, buildRustCrate ? pkgs.buildRustCrate
# This is used as the `crateOverrides` argument for `buildRustCrate`.
, defaultCrateOverrides ? pkgs.defaultCrateOverrides
# The features to enable for the root_crate or the workspace_members.
Expand Down
54 changes: 21 additions & 33 deletions crate2nix/templates/nix/crate2nix/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
{ pkgs
, lib
, stdenv
, buildRustCrate
, defaultCrateOverrides
, strictDeprecation ? true
, crates ? { }
Expand Down Expand Up @@ -162,7 +161,6 @@ rec {
{ packageId
, features ? rootFeatures
, crateOverrides ? defaultCrateOverrides
, buildRustCrateFunc ? null
, runTests ? false
, testCrateFlags ? [ ]
, testInputs ? [ ]
Expand All @@ -176,26 +174,18 @@ rec {
, testInputs
}:
let
buildRustCrateFuncOverriden =
if buildRustCrateFunc != null
then buildRustCrateFunc
else
(
if crateOverrides == pkgs.defaultCrateOverrides
then buildRustCrate
else
buildRustCrate.override {
defaultCrateOverrides = crateOverrides;
}
);
buildRustCrateOverrides =
if crateOverrides == pkgs.defaultCrateOverrides
then { }
else {
defaultCrateOverrides = crateOverrides;
};
builtRustCrates = builtRustCratesWithFeatures {
inherit packageId features;
buildRustCrateFunc = buildRustCrateFuncOverriden;
inherit packageId features buildRustCrateOverrides;
runTests = false;
};
builtTestRustCrates = builtRustCratesWithFeatures {
inherit packageId features;
buildRustCrateFunc = buildRustCrateFuncOverriden;
inherit packageId features buildRustCrateOverrides;
runTests = true;
};
drv = builtRustCrates.${packageId};
Expand All @@ -221,7 +211,7 @@ rec {
{ packageId
, features
, crateConfigs ? crates
, buildRustCrateFunc
, buildRustCrateOverrides
, runTests
, target ? defaultTarget
} @ args:
Expand All @@ -239,13 +229,10 @@ rec {
target = target // { test = runTests; };
}
);
buildByPackageId = packageId: buildByPackageIdImpl packageId;

# Memoize built packages so that reappearing packages are only built once.
builtByPackageId =
lib.mapAttrs (packageId: value: buildByPackageId packageId) crateConfigs;
buildByPackageIdImpl = packageId:
buildByPackageIdForPkgs = pkgs: packageId:
let
# proc_macro crates must be compiled for the build architecture
cratePkgs = if crateConfig.procMacro or false then pkgs.buildPackages else pkgs;
features = mergedFeatures."${packageId}" or [ ];
crateConfig' = crateConfigs."${packageId}";
crateConfig =
Expand All @@ -256,14 +243,16 @@ rec {
(crateConfig'.devDependencies or [ ]);
dependencies =
dependencyDerivations {
inherit builtByPackageId features target;
inherit features target;
buildByPackageId = buildByPackageIdForPkgs cratePkgs;
dependencies =
(crateConfig.dependencies or [ ])
++ devDependencies;
};
buildDependencies =
dependencyDerivations {
inherit builtByPackageId features target;
inherit features target;
buildByPackageId = buildByPackageIdForPkgs cratePkgs.buildPackages;
dependencies = crateConfig.buildDependencies or [ ];
};
filterEnabledDependenciesForThis = dependencies: filterEnabledDependencies {
Expand Down Expand Up @@ -295,13 +284,13 @@ rec {
dependenciesWithRenames;
versionAndRename = dep:
let
package = builtByPackageId."${dep.packageId}";
package = crateConfigs."${dep.packageId}";
in
{ inherit (dep) rename; version = package.version; };
in
lib.mapAttrs (name: choices: builtins.map versionAndRename choices) grouped;
in
buildRustCrateFunc
cratePkgs.buildRustCrate.override buildRustCrateOverrides
(
crateConfig // {
src = crateConfig.src or (
Expand All @@ -320,24 +309,23 @@ rec {
}
);
in
builtByPackageId;
lib.mapAttrs (packageId: value: buildByPackageIdForPkgs pkgs packageId) crateConfigs;

/* Returns the actual derivations for the given dependencies. */
dependencyDerivations =
{ builtByPackageId
{ buildByPackageId
, features
, dependencies
, target
}:
assert (builtins.isAttrs builtByPackageId);
assert (builtins.isList features);
assert (builtins.isList dependencies);
assert (builtins.isAttrs target);
let
enabledDependencies = filterEnabledDependencies {
inherit dependencies features target;
};
depDerivation = dependency: builtByPackageId.${dependency.packageId};
depDerivation = dependency: buildByPackageId dependency.packageId;
in
map depDerivation enabledDependencies;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
{ lib, crate2nix }:
let
fakeCrates = {
fakeCrates = packageId: {
"pkg_id1" = "pkg_id1";
"pkg_id2" = "pkg_id2";
"pkg_id3" = "pkg_id3";
};
}.${packageId};
fakeDependencies = [
{
name = "id1";
Expand All @@ -23,7 +23,7 @@ let
];
dependencyDerivations = features: dependencies:
crate2nix.dependencyDerivations {
builtByPackageId = fakeCrates;
buildByPackageId = fakeCrates;
target = crate2nix.defaultTarget;
inherit features dependencies;
};
Expand Down
Loading