Skip to content

fix pkgsMusl.cargo via cargoSetupHook #198311

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.

Sign up for GitHub

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

Merged
yu-re-ka merged 2 commits into from
Oct 30, 2022
Merged

fix pkgsMusl.cargo via cargoSetupHook #198311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd5a9a3
cargoSetupHook: pass host config flags
yuyuyureka Oct 28, 2022
e76e5a5
cargo: no longer broken on musl
yuyuyureka Oct 28, 2022
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
2 changes: 1 addition & 1 deletion pkgs/build-support/rust/hooks/cargo-setup-hook.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ cargoSetupPostUnpackHook() {
cat ${tmp_config} >> .cargo/config

cat >> .cargo/config <<'EOF'
@rustTarget@
@cargoConfig@
EOF

echo "Finished cargoSetupPostUnpackHook"
Expand Down
36 changes: 29 additions & 7 deletions pkgs/build-support/rust/hooks/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,37 @@ in {
# The `.nativeDrv` stanza works like nativeBuildInputs and ensures cross-compiling has the right version available.
diff = "${diffutils.nativeDrv or diffutils}/bin/diff";

# Target platform
rustTarget = ''
[target."${rust.toRustTarget stdenv.buildPlatform}"]
# We want to specify the correct crt-static flag for both
# the build and host platforms. This is important when the wanted
# value for crt-static does not match the defaults in the rustc target,
# like for pkgsMusl or pkgsCross.musl64; Upstream rustc still assumes
# that musl = static[1].
#
# By default, Cargo doesn't apply RUSTFLAGS when building build.rs
# if --target is passed, so the only good way to set crt-static for
# build.rs files is to use the unstable -Zhost-config Cargo feature.
# This allows us to specify flags that should be passed to rustc
# when building for the build platform. We also need to use
# -Ztarget-applies-to-host, because using -Zhost-config requires it.
#
# When doing this, we also have to specify the linker, or cargo
# won't pass a -C linker= argument to rustc. This will make rustc
# try to use its default value of "cc", which won't be available
# when cross-compiling.
#
# [1]: https://github.com/rust-lang/compiler-team/issues/422
cargoConfig = ''
[host]
"linker" = "${ccForBuild}"
${lib.optionalString (stdenv.buildPlatform.config != stdenv.hostPlatform.config) ''
[target."${shortTarget}"]
"linker" = "${ccForHost}"
''}
"rustflags" = [ "-C", "target-feature=${if stdenv.buildPlatform.isStatic then "+" else "-"}crt-static" ]

[target."${shortTarget}"]
"linker" = "${ccForHost}"
"rustflags" = [ "-C", "target-feature=${if stdenv.hostPlatform.isStatic then "+" else "-"}crt-static" ]

[unstable]
host-config = true
target-applies-to-host = true
Copy link
Member

@symphorien symphorien Oct 30, 2022

Choose a reason for hiding this comment

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

I'm very puzzled as to why you set target-applies-to-host to true: the doc says

It requires the -Zhost-config and -Ztarget-applies-to-host command-line options to be set, and that target-applies-to-host = false is set in the Cargo configuration file.

https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#host-config

Copy link
Contributor Author

@yu-re-ka yu-re-ka Oct 30, 2022

Choose a reason for hiding this comment

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

There is two things here:

  • Is the target-applies-to-host unstable feature enabled?
  • If yes, this unlocks a new target-applies-to-host option

Also see, in the same section:

Setting -Zhost-config changes the default for target-applies-to-host to false from true.

Copy link
Member

@symphorien symphorien Oct 30, 2022

Choose a reason for hiding this comment

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

ok I understand. Then the change looks sensible to me, and if things build with it then go for it.

'';
};
} ./cargo-setup-hook.sh) {};
Expand Down
2 changes: 0 additions & 2 deletions pkgs/development/compilers/rust/cargo.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,5 @@ rustPlatform.buildRustPackage {
maintainers = with maintainers; [ retrry ];
license = [ licenses.mit licenses.asl20 ];
platforms = platforms.unix;
# weird segfault in a build script
broken = stdenv.targetPlatform.isMusl && !stdenv.targetPlatform.isStatic;
};
}