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

WIP: add support for cross-compilation #152

wants to merge 1 commit into from

Conversation

lopsided98
Copy link
Contributor

This adds support for cross-compiling using crate2nix to Linux targets. With this PR, passing pkgs = pkgsCross.<platform> to the generated Cargo.nix will cross-compile the crate.

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.

This PR also includes #142 because I needed it to test some of my crates.

@lopsided98
Copy link
Contributor Author

cc @Ericson2314

@Ericson2314
Copy link
Collaborator

Is anyone going to review this? I'm happy to help weasel around back compat issues if that's what's needed, but I'd like someone to confirm that's indeed problem and to what extent, first.

@joprice
Copy link

joprice commented Oct 14, 2020

Is there an example of using this? I tried it out locally and get an error when it tries to link bootstrap.

@lopsided98
Copy link
Contributor Author

I have only used this to build some private packages, but I just tried to cross-compile crate2nix using the following command:

nix-build -E 'with import <nixpkgs> {}; import ./. { pkgs = pkgsCross.armv7l-hf-multiplatform; }'

If you remove nix-prefetch-git from the dependencies (due to NixOS/nixpkgs#66741), it mostly works, but fails to build tera for some reason I've yet to look into.

I'm not sure where exactly it is failing for you. Could you post the command you are using as well as the end of the log?

@joprice
Copy link

joprice commented Oct 16, 2020

I realized I have a more complicated setup than I thought: I need a particular nightly, so I'm using the moz-overlay, and I'm compiling statically. It turns out the previous errors were caused by my inconsistent passing of cross packages and not using buildPackages where I should, so now I'm now able to link dynamically, but not statically. The issue seems to be the build script not linking:

error: linking with `cc` failed: exit code: 1
  |
  = note: "cc" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-m64" "-Wl,--eh-frame-hdr" "-L" "/nix/store/qdcv8z57rcfwnxnpfsczmcxp6jblaf8m-rust-1.49.0-nightly-2020-10-10-b1af43bc6/lib/rustlib/x86_64-unknown-linux-gnu/lib" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.0.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.1.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.2.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.3.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.4.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.5.rcgu.o" "target/build/nix/build_script_build.build_script_build.7rcbfp3g-cgu.6.rcgu.o" "-o" "target/build/nix/build_script_build" "target/build/nix/build_script_build.1enide7h7a8vrbgy.rcgu.o" "-Wl,--gc-sections" "-static" "-no-pie" "-Wl,-zrelro" "-Wl,-znow" "-nodefaultlibs" "-L" "target/buildDeps" "-L" "/nix/store/qdcv8z57rcfwnxnpfsczmcxp6jblaf8m-rust-1.49.0-nightly-2020-10-10-b1af43bc6/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-Wl,--start-group" "-Wl,-Bstatic" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-6e0e72ef3f331f94.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libpanic_unwind-eed7c8ea6eea20e8.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libminiz_oxide-637cb1b53c807e95.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libadler-099cf0af4375543b.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libobject-b5f18e83369ef257.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libaddr2line-3bb19daa4485d5fe.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libgimli-5298ab0591e7fb29.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_demangle-2e4947d254d0b599.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libhashbrown-1532436f783b0405.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_alloc-3d12d76f5782439f.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libunwind-87f8d20d4e058c86.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcfg_if-d41f1ff31e4e0f27.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/liblibc-0d5ea4f2d39b8e27.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/liballoc-31288459e6a43502.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/librustc_std_workspace_core-c52e5d6301e1bd59.rlib" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-2675a9a46b5cec89.rlib" "-Wl,--end-group" "/nix/store/3f6c9z0y2v0h35w7skd3pipqvh88h14r-rust/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcompiler_builtins-f51baad7bbcb81c4.rlib" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-lgcc_eh" "-lgcc" "-Wl,-Bdynamic"
  = note: /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -lutil
          /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -lrt
          /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -lpthread
          /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -lm
          /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -ldl
          /nix/store/p792j5f44l3f0xi7ai5jllwnxqwnka88-binutils-2.31.1/bin/ld: cannot find -lc
          collect2: error: ld returned 1 exit status
          

error: aborting due to previous error

setting SOURCE_DATE_EPOCH to timestamp 1602371010 of file syn-1.0.44/Cargo.toml
unpacking sources
unpacking source archive /nix/store/hjqy27dq1bc1gq9afyh4bwrv5yaqgh1c-which-3.1.1.tar.gz
patching sources
configuring
source root is which-3.1.1
Running cd .
setting SOURCE_DATE_EPOCH to timestamp 1583553216 of file which-3.1.1/Cargo.toml
patching sources
configuring
Running cd .
Building build.rs (syn)
Running rustc --crate-name build_script_build build.rs --crate-type bin -C debuginfo=2 -C codegen-units=10 -C target-feature=+crt-static --edition 2018 --cfg feature="clone-impls" --cfg feature="default" --cfg feature="derive" --cfg feature="extra-traits" --cfg feature="full" --cfg feature="parsing" --cfg feature="printing" --cfg feature="proc-macro" --cfg feature="quote" --cfg feature="visit-mut" --out-dir target/build/syn --emit=dep-info,link -L dependency=target/buildDeps --cap-lints allow --color always
building
Building src/lib.rs (which)
Running rustc --crate-name which src/lib.rs --out-dir target/lib -L dependency=target/deps --cap-lints allow -C debuginfo=2 -C codegen-units=10 --remap-path-prefix=/build=/ --extern libc=/nix/store/gfny6275zp8z2vv792g4pnfnwxy1x057-rust_libc-0.2.79-lib/lib/liblibc-d402711b81.rlib -C target-feature=+crt-static --edition 2015 -C metadata=a553ffe869 -C extra-filename=-a553ffe869 --crate-type lib --color always
builder for '/nix/store/0hkpkfavj343a84nssyqs3phz2fqmy1m-rust_nix-0.15.0-armv7l-unknown-linux-gnueabihf.drv' failed with exit code 1
cannot build derivation '/nix/store/ddh2hvibpimmr97ng3z7kwi4hamrf259-rust_client-0.1.0-armv7l-unknown-linux-gnueabihf.drv': 1 dependencies couldn't be built
error: build of '/nix/store/ddh2hvibpimmr97ng3z7kwi4hamrf259-rust_client-0.1.0-armv7l-unknown-linux-gnueabihf.drv' failed 

I tried providing overrides, which worked for dependency crates that have build.rs scripts, but not for the final one defined in the workspace:

  client = attrs: {          
        nativeBuildInputs = [                 
          pkgs.buildPackages.glibc.static
        ];
        buildInputs = [
          pkgs.glibc.static
        ];
      };

Is this the right approach for providing dependencies to build scripts?

@joprice
Copy link

joprice commented Oct 16, 2020

I also see this error with crt-static

error: the `#[proc_macro_attribute]` attribute is only usable with crates of the `proc-macro` crate type
   --> src/lib.rs:186:1
    |
186 | #[proc_macro_attribute]
    | ^^^^^^^^^^^^^^^^^^^^^^^

error: the `#[proc_macro_derive]` attribute is only usable with crates of the `proc-macro` crate type
   --> src/lib.rs:207:1
    |
207 | #[proc_macro_derive(ProcMacroHack)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: the `#[proc_macro_attribute]` attribute is only usable with crates of the `proc-macro` crate type
   --> src/lib.rs:219:1
    |
219 | #[proc_macro_attribute]
    | ^^^^^^^^^^^^^^^^^^^^^^^

which seems similar to this issue rust-lang/rust#71651. Maybe targetFeatures shouldn't be used for both proc macros and build scripts?

@Ericson2314
Copy link
Collaborator

Ericson2314 commented Oct 16, 2020

I see:

note: "cc"

I think you want a prefixed cc to be there? That would be a good sign that it is using the right wrapped CC the adds the flags for musl, etc.

@joprice
Copy link

joprice commented Oct 16, 2020

I think that's correct because the build script is being built for the x86 host architecture.

@joprice
Copy link

joprice commented Oct 16, 2020

Nevermind I think I made an assumption about the name "build_script_build.build_script_build". Not sure I'm right there.

@Ericson2314
Copy link
Collaborator

Oh I didn't see the horizontal scrollbar. Perhaps you can make a public reproduction of the issue so we could dive deeper?

@joprice
Copy link

joprice commented Oct 17, 2020

Sure. I'll put a repo together.

@joprice
Copy link

joprice commented Oct 17, 2020

Here it is https://github.com/joprice/rust-nix-test.

You should be able to repro with nix-build. Let me know if you need more info.

I also had some othre local workarounds I tried that I can add back in where I attempted to add static libc to the crates that were having issues, but it didn't seem to help much:

 hasBuildScript = attrs: attrs // {                                                                   
      nativeBuildInputs = (attrs.nativeBuildInputs or [ ]) ++ [ cross.buildPackages.glibc ];
      buildInputs = (attrs.buildInputs or [ ]) ++ [ cross.glibc.static ];
      extraRustcOpts = attrs.extraRustcOpts ++ extraRustcOpts;
    };

I then added that to crateOverrides for each dependency that was linking to glibc. Is there a way to do that more generally, or should that not be needed?

@Ericson2314
Copy link
Collaborator

Thanks! I'll be able to look tomorrow.

@nagisa
Copy link
Contributor

nagisa commented Oct 23, 2020

I believe crate2nix is not actively maintained for some time. I just forked this for my own purposes and started addressing problems that matter to me in my fork.

@Ericson2314
Copy link
Collaborator

https://github.com/kolloch/crate2nix/commits/master Oh I see, the commits have been fairly rote as of late.

@andir
Copy link
Collaborator

andir commented Oct 23, 2020 via email

@kolloch kolloch self-requested a review November 23, 2020 19:57
@kolloch
Copy link
Collaborator

kolloch commented Nov 23, 2020

Hi @lopsided98, @Ericson2314, @nagisa, @joprice,

Sorry, if there is still interest in this, I will find time this week (probably during the weekend). Just ping here. A real-time channel for chatting would be nice to resolve any issues near-time then. E.g. a video conference on Saturday evening (Zurich time).

@Ericson2314
Copy link
Collaborator

I can't speak for @lopsided98, but I would really like to see this be finished, and perhaps can do a call then. I'll have to catch up on exactly what's here too!

@joprice
Copy link

joprice commented Nov 23, 2020

I don't want to put any undue pressure as I'm currently only using it for hobby purposes, but I'm willing to pitch in if I can.

@lopsided98
Copy link
Contributor Author

My priorities shifted elsewhere recently, so I didn't get a chance to finish this. I'm available this weekend to work on this.

This is my understanding of the known issues:

  • Backwards compatibility regarding the removal of the buildRustCrate argument
  • Build failure while cross-compiling crate2nix. Unfortunately I didn't record the error. I will try to reproduce it.
  • musl static build fails. Based on Don't use static crt by default when build proc-macro rust-lang/rust#69519, I agree with @joprice that the issue described in this comment is probably caused by passing -C target-feature=+crt-static, which bypasses the logic in that PR. Normally, the crt-static feature would be specified in the target spec JSON. My first thought is that the correct solution might be to make pkgsStatic.buildPackages.rustc build (which would provide a rustc with the correct target spec), and provide a way for crate2nix to know what target features are specified in the target spec (for dependency resolution).

@lopsided98
Copy link
Contributor Author

This is the error I'm running into while trying to cross-compile crate2nix:

Running rustc --crate-name tera src/lib.rs --out-dir target/lib -L dependency=target/deps --cap-lints allow -C opt-level=3 -C codegen-units=8 --remap-path-prefix=/build=/ --extern globwalk=/nix/store/6ljrk7wx3zj274lc5iyn6gbc79fp3afv-rust_globwalk-0.8.0-armv7l-unknown-linux-gnueabihf-lib/lib/libglobwalk-934342f2ce.rlib --extern lazy_static=/nix/store/xp2dhq4lflv8wiciffhy6qi7ynh37rrp-rust_lazy_static-1.4.0-armv7l-unknown-linux-gnueabihf-lib/lib/liblazy_static-28e1bf1896.rlib --extern pest=/nix/store/w0b7mqzz5x6nk4s2imfylrlj8rbmny74-rust_pest-2.1.3-armv7l-unknown-linux-gnueabihf-lib/lib/libpest-19c5caddb3.rlib --extern pest_derive=/nix/store/a7jjv88gdh4ammlz82jpwdkajqh91bhg-rust_pest_derive-2.1.0-lib/lib/libpest_derive-906e1d4536.so --extern regex=/nix/store/8k15lv58jgnkr1q6palb9rm3hl36rc95-rust_regex-1.3.6-armv7l-unknown-linux-gnueabihf-lib/lib/libregex-125ab14a6c.rlib --extern serde=/nix/store/yscsvvw87299n4wk388pl5dwf9arya3f-rust_serde-1.0.106-armv7l-unknown-linux-gnueabihf-lib/lib/libserde-c85fcc631e.rlib --extern serde_json=/nix/store/qhswm66qjzayhrq6a32wnp8lclmmpj9m-rust_serde_json-1.0.51-armv7l-unknown-linux-gnueabihf-lib/lib/libserde_json-80b064783c.rlib --edition 2018 --target armv7-unknown-linux-gnueabihf -C linker=armv7l-unknown-linux-gnueabihf-gcc -C metadata=aeed81cbc0 -C extra-filename=-aeed81cbc0 --crate-type lib --color always
error[E0461]: couldn't find crate `ucd_trie` with expected target triple armv7-unknown-linux-gnueabihf which `pest` depends on
 --> src/parser/mod.rs:4:5
  |
4 | use pest::iterators::Pair;
  |     ^^^^
  |
  = note: the following crate versions were found:
          crate `ucd_trie`, target triple x86_64-unknown-linux-gnu: /nix/store/sqrq8gfqzn1x87lj80f700xj6jxdjnq2-rust_ucd-trie-0.1.3-lib/lib/libucd_trie-71f45caf6f.rlib

error: aborting due to previous error

@kolloch
Copy link
Collaborator

kolloch commented Nov 28, 2020

@lopsided98 Should have sent you contact details via the contact form on your webpage.

Got the CI/CD running again and merge the PR that you are dependent on.

@@ -7,13 +7,14 @@
, 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 :).

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.

@Ericson2314
Copy link
Collaborator

@kolloch ping me on IRC if you still want to work on this real time.

@lopsided98
Copy link
Contributor Author

The tera failure is a bug in buildRustCrate. It occurs when the a crate depends on the host and build versions of the same crate. The rlib hash suffix doesn't include the platform, so both versions have the same rlib name, and clobber each other when they are added to the dependency directory.

@lopsided98
Copy link
Contributor Author

See NixOS/nixpkgs#105305

@lopsided98
Copy link
Contributor Author

@joprice I was able to make a modified version of your repo build, albeit with stable Rust and musl. See below for the diff.

rust-nix-test diff
diff --git a/common.nix b/common.nix
index a119809..750d9ae 100644
--- a/common.nix
+++ b/common.nix
@@ -1,46 +1,21 @@
 let
   mozOverlay = import (builtins.fetchTarball https://github.com/mozilla/nixpkgs-mozilla/archive/master.tar.gz);
-  pkgs = (
-    import <nixpkgs>
-      {
-        overlays = [
-          mozOverlay
-          (
-            self: super: {
-              rustChannel = self.rustChannelOf {
-                rustToolchain = ./rust-toolchain;
-              };
-              cargo = self.rustChannel.rust;
-              rustc = self.rustChannel.rust;
-            }
-          )
-        ];
-      }
-  ).pkgsCross.armv7l-hf-multiplatform;
-  alsaLibStatic = (pkgs.alsaLib.overrideAttrs
-    (o: {
-      configureFlags = [
-        "--enable-shared=no"
-        "--enable-static=yes"
-      ];
-    }));
+  pkgs = (import <nixpkgs> {}).pkgsCross.armv7l-hf-multiplatform.pkgsStatic;
   crateOverrides = pkgs.defaultCrateOverrides // {
     alsa-sys = attrs: attrs // {
       nativeBuildInputs = (attrs.nativeBuildInputs or [ ]) ++ [
         pkgs.buildPackages.pkg-config
       ];
-      buildInputs = (attrs.buildInputs or [ ]) ++ [ alsaLibStatic ];
+      buildInputs = (attrs.buildInputs or [ ]) ++ [ pkgs.alsaLib ];
     };
     prost-build = attrs: attrs // {
       PROTOC = "${pkgs.buildPackages.protobuf}/bin/protoc";
     };
     client = attrs: attrs // {
       PROTOS_DIR = ./proto;
-      buildInputs = [
+      nativeBuildInputs = [
         # this is needed to format the generated grpc code
         pkgs.buildPackages.rustfmt
-        # TODO: add to all packages (expect proc macro and build scripts)?
-        # pkgs.glibc.static
       ];
     };
   };
@@ -48,10 +23,6 @@ let
     ./Cargo.nix
     {
       release = false;
-      # NOTE: cross building works with this disabled
-      targetFeatures = [
-        "crt-static"
-      ];
       defaultCrateOverrides = crateOverrides;
     };
 in
@@ -59,6 +30,6 @@ in
   pkgs = pkgs;
   build = cargo_nix.allWorkspaceMembers;
   shell = pkgs.mkShell {
-    nativeBuildInputs = [ pkgs.buildPackages.rustChannel.rust ];
+    nativeBuildInputs = [ pkgs.buildPackages.rustc ];
   };
 }

I looked into using the Mozilla overlay, but I couldn't see how to make it use musl on armv7l. It looks like std is built for armv7-unknown-linux-musleabihf, but I don't see a way of combining that with the x86_64-unknown-linux-gnu rustc.

I don't think we really should be passing -C target-feature= to rustc in most cases. Features should normally be specified in the target spec and if crates need to know about them, there should be some way to getting them from rustc through Nix for crate2nix to use.

On the other hand, if we want to try to make crt-static work for now, we could add some logic to mimic what rustc does and not pass it to proc macros. IMO that is out of scope of this PR though.

@lopsided98
Copy link
Contributor Author

I think that leaves only the backwards compatibility and debug tool breakage issues, which I still haven't figured out a good way to solve.

@joprice
Copy link

joprice commented Nov 29, 2020

I think I only had to use a specific nightly because it had some improved handling for static compilation. I'll try your changes on the original code and see if the remaining deps compile, and add them to the test repo if they don't.

@@ -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...)

@joprice
Copy link

joprice commented Nov 29, 2020

@lopsided98 I tried your changes and got this error:

thread 'main' panicked at 'when targeting MUSL either the rust.musl-root option or the target.$TARGET.musl-root option must be specified in config.toml', src/bootstrap/sanity.rs:208:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /build/rustc-1.45.2-src/build/bootstrap/debug/bootstrap build --jobs 10
Build completed unsuccessfully in 0:00:31
make: *** [Makefile:18: all] Error 1

I assume this fix relies on a change you did in nixpkgs as well, since using the latest commit of nixpkgs worked.

@lopsided98
Copy link
Contributor Author

@joprice Yes, you need NixOS/nixpkgs#105314

@Gaelan
Copy link

Gaelan commented Jan 4, 2021

Works great for me (aarch64 -> armv6l).

What more needs to be done to merge this?

@Ericson2314
Copy link
Collaborator

N.B. I am preparing an additional commit with my suggestions.

@Ericson2314
Copy link
Collaborator

OK, PR against the PR at https://github.com/lopsided98/crate2nix/pull/1.

@kolloch
Copy link
Collaborator

kolloch commented Jan 7, 2021

Heyo @Ericson2314, the test failure on linux looks legit. Feel free to ignore the one on Mac OS.

@kolloch
Copy link
Collaborator

kolloch commented Jan 7, 2021

I merged the follow up from @Ericson2314 .

Thank you for this :)

@kolloch kolloch closed this Jan 7, 2021
@kolloch
Copy link
Collaborator

kolloch commented Jan 7, 2021

Here it is https://github.com/joprice/rust-nix-test.

You should be able to repro with nix-build. Let me know if you need more info.

I also had some othre local workarounds I tried that I can add back in where I attempted to add static libc to the crates that were having issues, but it didn't seem to help much:

 hasBuildScript = attrs: attrs // {                                                                   
      nativeBuildInputs = (attrs.nativeBuildInputs or [ ]) ++ [ cross.buildPackages.glibc ];
      buildInputs = (attrs.buildInputs or [ ]) ++ [ cross.glibc.static ];
      extraRustcOpts = attrs.extraRustcOpts ++ extraRustcOpts;
    };

I then added that to crateOverrides for each dependency that was linking to glibc. Is there a way to do that more generally, or should that not be needed?

I would love if there was some doc in the README.md -- looks like you already collected some experience on how to use it.

@kolloch
Copy link
Collaborator

kolloch commented Jan 7, 2021

For reference, this was the follow up PR: #160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants