rustPlatform: cargo test is now called with the same environment variables as cargo build

[lolbinarycat]

this means that cargo dependancies will no longer be built twice.

fixes #291222

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[SuperSandro2000]

@ofborg build lsd ripgrep

[SuperSandro2000]

This PR rebuilds a lot of packages which means we must target staging. Please follow the contributing guide to not potentially ping a lot of people.

[lolbinarycat]

@SuperSandro2000 done.

[SuperSandro2000]

LGTM with my limited knowledge about the rust ecosystem

[bryango]

This is awesome! It may significantly shorten build time of many rust packages... Can this be revived? It seems there are some merge conflicts now, but that probably won't be too difficult to resolve. Friendly ping @alyssais in case you are interested 😆

[SuperSandro2000]

I've just rebased the PR on staging.

[bryango]

@ofborg build lsd ripgrep

[trofi]

Bisect says 683f97e rustPlatform: cargo test is now called with the same environment variables as cargo buil broke rust-cbindgen in staging as:

$ nix build --no-link -f. -L rust-cbindgen
...
rust-cbindgen>      Running tests/profile.rs (target/x86_64-unknown-linux-gnu/release/deps/profile-fec32decafabbade)
rust-cbindgen> running 3 tests
rust-cbindgen> test bin_explicit_release_build ... FAILED
rust-cbindgen> test bin_explicit_debug_build ... FAILED
rust-cbindgen> test bin_default_uses_debug_build ... FAILED
rust-cbindgen> failures:
rust-cbindgen> ---- bin_explicit_release_build stdout ----
rust-cbindgen> thread 'bin_explicit_release_build' panicked at tests/profile.rs:99:5:
rust-cbindgen> assertion `left == right` failed
rust-cbindgen>   left: ["release", "x86_64-unknown-linux-gnu"]
rust-cbindgen>  right: ["release"]
rust-cbindgen> note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
rust-cbindgen> ---- bin_explicit_debug_build stdout ----
rust-cbindgen> thread 'bin_explicit_debug_build' panicked at tests/profile.rs:93:5:
rust-cbindgen> assertion `left == right` failed
rust-cbindgen>   left: ["debug", "x86_64-unknown-linux-gnu"]
rust-cbindgen>  right: ["debug"]
rust-cbindgen> ---- bin_default_uses_debug_build stdout ----
rust-cbindgen> thread 'bin_default_uses_debug_build' panicked at tests/profile.rs:87:5:
rust-cbindgen> assertion `left == right` failed
rust-cbindgen>   left: ["debug", "x86_64-unknown-linux-gnu"]
rust-cbindgen>  right: ["debug"]
rust-cbindgen> failures:
rust-cbindgen>     bin_default_uses_debug_build
rust-cbindgen>     bin_explicit_debug_build
rust-cbindgen>     bin_explicit_release_build
rust-cbindgen> test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.59s
rust-cbindgen> error: test failed, to rerun pass `--test profile`

[bryango]

Ah that's not good... I think it's due to the additional CARGO_BUILD_TARGET variable:

nix-repl> :p pkgs.rust.envVars.setEnv
env \
"CC_X86_64_UNKNOWN_LINUX_GNU=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/cc" \
"CXX_X86_64_UNKNOWN_LINUX_GNU=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/c++" \
"CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/cc" \
"CC_X86_64_UNKNOWN_LINUX_GNU=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/cc" \
"CXX_X86_64_UNKNOWN_LINUX_GNU=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/c++" \
"CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/cc" \
"CARGO_BUILD_TARGET=x86_64-unknown-linux-gnu" \
"HOST_CC=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/cc" \
"HOST_CXX=/nix/store/zznja5f8v3jafffyah1rk46vpfcn38dv-gcc-wrapper-13.3.0/bin/c++" \

The failing cbindgen/tests/profile.rs is here: https://github.com/mozilla/cbindgen/blob/master/tests/profile.rs. I will try to look into this.

Bisect says 683f97e rustPlatform: cargo test is now called with the same environment variables as cargo buil broke rust-cbindgen in staging

Also, just out of curiosity: how do you know about staging breakages so fast? Is there any hydra instance currently running on it?

[trofi]

Bisect says 683f97e rustPlatform: cargo test is now called with the same environment variables as cargo buil broke rust-cbindgen in staging

Also, just out of curiosity: how do you know about staging breakages so fast? Is there an hydra instance currently running on it?

No hydra I'm aware of. I occasionally try to build my local system against staging to catch early regressions and to test local changes.

[bryango]

Ok, so indeed the culprit is CARGO_BUILD_TARGET, which changes the layout of the output directory. This is documented here: https://doc.rust-lang.org/cargo/guide/build-cache.html

[SuperSandro2000]

Can we just filter that one variable out?

[bryango]

Can we just filter that one variable out?

This would probably diminish the benefits of this PR, namely cargo test might fail to locate the cache from a previous build (which is in the CARGO_BUILD_TARGET subdirectory).

[alyssais]

We could send upstream a patch to unset that variable in the test. Alternatively we could just skip the test if it's not suitable for our setup.

[lolbinarycat]

alternatively, if it's just rustgm-cbindgen, we could unset that variable in that specific package (in a pre-test hook)

this would mean specifically that package would be built twice, but that's a lot better than double building every single package.

still probably better to just skip the check phase for it and notify upstream.

[bryango]

I have sent a patch upstream:

• Fix: Ignore CARGO_BUILD_TARGET in tests mozilla/cbindgen#1010

and they have already responded kindly. A downstream nixpkgs PR #348031 is also created, as shown below.