rustPlatform.cargoSetupHook: fix platform check

[alyssais]

Description of changes

Cargo will never need to link for the target platform — that'd be for the package being built to do at runtime. Cargo should know about the build and host linkers.

This fixes e.g. pkgsCross.musl64.fd from x86_64-linux.

Fixes: 67a4f82 ("rust: hooks: fix cross compilation")

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 23.11 Release Notes (or backporting 23.05 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.

[ghost]

Cargo will never need to link for the target platform — that'd be for the package being built to do at runtime.

This is a setup hook, which means it goes in nativeBuildInputs, so stdenv.targetPlatform actually refers to the hostPlatform of the package in which the hook appears.

[alyssais]

Ah, so we should undo this, and replace [target."${rust.toRustTarget stdenv.buildPlatform}"] above with [target."${rust.toRustTarget stdenv.hostPlatform}"], then? Because cargo should definitely never need to know the platform the setup hook was built on.

[ghost]

Hang on, I'm getting my builds caught up to staging. I'll sort it out.

[yu-re-ka]

For any change, please also investigate if pkgsCross.musl64.fd, pkgsStatic.fd etc. work and produce the right (statically or dynamically linked) binary

[ghost]

and produce the right (statically or dynamically linked) binary

How do I know if it's the right one? Is there some invocation which was segfaulting before? Or producing the wrong output?

If there is some property of these builds that is important to you, please consider adding a test to tests.cross.sanity. It's the closest thing we have to CI for cross compilation, because OfBorg.

[ghost]

Ah, so we should undo this

Yes.

and replace [target."${rust.toRustTarget stdenv.buildPlatform}"] above with [target."${rust.toRustTarget stdenv.hostPlatform}"], then? Because cargo should definitely never need to know the platform the setup hook was built on.

I think so. Testing this.

[ghost]

What was the bug that this PR fixes?

[ghost]

This is a setup hook, which means it goes in nativeBuildInputs, so stdenv.targetPlatform actually refers to the hostPlatform of the package in which the hook appears.

@Artturin: the dreaded "hooks are not spliced" problem has struck again.

[alyssais]

What was the bug that this PR fixes?

This fixes e.g. pkgsCross.musl64.fd from x86_64-linux.

[Artturin]

This is a setup hook, which means it goes in nativeBuildInputs, so stdenv.targetPlatform actually refers to the hostPlatform of the package in which the hook appears.

@Artturin: the dreaded "hooks are not spliced" problem has struck again.

That's only a issue inside the python set with its own hooks

pkgsCross.aarch64-multiplatform.__splicedPackages.rustPlatform.cargoSetupHook ? __spliced
true

[ghost]

Ah, I was thrown off by the title and:

Fixes: 67a4f82 ("rust: hooks: fix cross compilation")

which is a commit not a bug (?)

[ghost]

That's only a issue inside the python set with its own hooks

They're spliced if you access the hooks directly, but the hook that's finding its way into nativeBuildInputs is not spliced:

diff --git a/pkgs/build-support/rust/build-rust-package/default.nix b/pkgs/build-support/rust/build-rust-package/default.nix
index b75d429ef598..e2f8ad40f23f 100644
--- a/pkgs/build-support/rust/build-rust-package/default.nix
+++ b/pkgs/build-support/rust/build-rust-package/default.nix
@@ -114,7 +114,9 @@ stdenv.mkDerivation ((removeAttrs args [ "depsExtraArgs" "cargoUpdateHook" "carg

   patchRegistryDeps = ./patch-registry-deps;

-  nativeBuildInputs = nativeBuildInputs ++ lib.optionals auditable [
+  nativeBuildInputs =
+    assert args.pname == "fd" && !(cargoSetupHook?__spliced) -> throw "it is not spliced";
+    nativeBuildInputs ++ lib.optionals auditable [
     (buildPackages.cargo-auditable-cargo-wrapper.override {
       inherit cargo cargo-auditable;
     })

$ nix-instantiate . -A pkgsMusl.__splicedPackages.fd
error: it is not spliced

makeRustPlatform is a scope, but it isn't using makeScopeWithSplicing.

[ghost]

Yeah.

So this PR is correct at the moment, because our rust hooks aren't spliced.

When we get the rust hooks spliced we will need to revert this, however. But until then it's the right change.

#263019

[ghost]

Proof that the rust hooks are not spliced: #263082 (comment)