Skip to content

pkgsMusl.cargo: fix build #190796

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

Closed
alyssais wants to merge 2 commits into from
Closed

pkgsMusl.cargo: fix build #190796

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
e94c4fa
rustc: default crt-static to false on musl
yuyuyureka Aug 13, 2022
4680698
pkgsMusl.cargo: fix build
alyssais Sep 11, 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
40 changes: 37 additions & 3 deletions pkgs/development/compilers/rust/cargo.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ lib, stdenv, pkgsHostHost
{ lib, stdenv, pkgsBuildHost, pkgsHostHost
, file, curl, pkg-config, python3, openssl, cmake, zlib
, installShellFiles, makeWrapper, cacert, rustPlatform, rustc
, libiconv, CoreFoundation, Security
Expand All @@ -17,6 +17,42 @@ rustPlatform.buildRustPackage {
inherit (rustc) tests;
};

# Upstream rustc still assumes that musl = static[1]. The fix for
# this is to disable crt-static by default for non-static musl
# targets.
#
# For every package apart from Cargo, we can fix this by just
# patching rustc to not have crt-static by default. But Cargo is
# built with the upstream bootstrap binary for rustc, which we can't
# easily patch. This means we need to find another way to make sure
# crt-static is not used during the build of pkgsMusl.cargo.
#
# 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 used in the Cargo build 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
postPatch = lib.optionalString (with stdenv.buildPlatform; isMusl && !isStatic) ''
mkdir -p .cargo
cat <<EOF >> .cargo/config
[host]
rustflags = "-C target-feature=-crt-static"
linker = "${pkgsBuildHost.stdenv.cc}/bin/${pkgsBuildHost.stdenv.cc.targetPrefix}cc"
[unstable]
host-config = true
target-applies-to-host = true
EOF
'';

# changes hash of vendor directory otherwise
dontUpdateAutotoolsGnuConfigScripts = true;

Expand Down Expand Up @@ -75,7 +111,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;
};
}
12 changes: 12 additions & 0 deletions pkgs/development/compilers/rust/rustc.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,18 @@ in stdenv.mkDerivation rec {

# Useful debugging parameter
# export VERBOSE=1
'' + lib.optionalString (stdenv.targetPlatform.isMusl && !stdenv.targetPlatform.isStatic) ''
# Upstream rustc still assumes that musl = static[1]. The fix for
# this is to disable crt-static by default for non-static musl
# targets.
#
# Even though Cargo will build build.rs files for the build platform,
# cross-compiling _from_ musl appears to work fine, so we only need
# to do this when rustc's target platform is dynamically linked musl.
#
# [1]: https://github.com/rust-lang/compiler-team/issues/422
substituteInPlace compiler/rustc_target/src/spec/linux_musl_base.rs \
Copy link
Contributor

@yu-re-ka yu-re-ka Sep 12, 2022

Choose a reason for hiding this comment

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

Could make this conditional to stdenv.hostPlatform.isMusl too, since otherwise it changes the behavior for people using the pkgsStatic.buildPackages.rustc manually. They would now have to manually set +crt-static to get a static binary, which they might not expect since upstream is explicitly keeping the default.

Copy link
Contributor

@yu-re-ka yu-re-ka Sep 14, 2022

Choose a reason for hiding this comment

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

Actually I don't think it's necessary. Most people who do this kind of stuff manually will use rustup.

Copy link
Member Author

@alyssais alyssais Sep 14, 2022

Choose a reason for hiding this comment

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

I think it's a good idea not to change the upstream behaviour where a user hasn't explicitly requested it, so I'm going to make this change. But I think it should be stdenv.targetPlatform so it's correct when building a musl cross compiler.

Copy link
Contributor

@yu-re-ka yu-re-ka Sep 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

But I think it should be stdenv.targetPlatform so it's correct when building a musl cross compiler.

Not sure about this. When the compiler is a cross-compiler from glibc to musl, the build.rs scripts will also run on glibc and this setting does not matter.
If I remember correctly what hostPlatform and targetPlatform are...

For example, targetPlatform.isMusl would be true for pkgsStatic.buildPackages.rustc, even though the change is not needed there?

Or the opposite, if we are building a cross-compiler from musl to glibc, targetPlatform.isMusl will be false, but it should still apply the change because the build.rs will be run with musl.

Copy link
Member Author

@alyssais alyssais Sep 16, 2022

Choose a reason for hiding this comment

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

Not sure about this. When the compiler is a cross-compiler from glibc to musl, the build.rs scripts will also run on glibc and this setting does not matter.
If I remember correctly what hostPlatform and targetPlatform are...

But this is the change for the default value of crt-static for all Musl builds in rustc, not the one for build.rs scripts in Cargo.

For example, targetPlatform.isMusl would be true for pkgsStatic.buildPackages.rustc, even though the change is not needed there?

Yes, the actual check I would do here is stdenv.targetPlatform.isMusl && !stdenv.targetPlatform.isStatic.

Or the opposite, if we are building a cross-compiler from musl to glibc, targetPlatform.isMusl will be false, but it should still apply the change because the build.rs will be run with musl.

Not sure about this one. I'll have to test it.

Copy link
Member Author

@alyssais alyssais Oct 27, 2022

Choose a reason for hiding this comment

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

Or the opposite, if we are building a cross-compiler from musl to glibc, targetPlatform.isMusl will be false, but it should still apply the change because the build.rs will be run with musl.

Not sure about this one. I'll have to test it.

I tested this by building pkgsMusl.pkgsCross.aarch64-multiplatform.{fd,cargo}, and both built fine even without this change. I don't entirely understand why, but it seems like just enabling it when targetPlatform is non-static Musl works fine.

--replace "base.crt_static_default = true" "base.crt_static_default = false"
'' + lib.optionalString (stdenv.isDarwin && stdenv.isx86_64) ''
# See https://github.com/jemalloc/jemalloc/issues/1997
# Using a value of 48 should work on both emulated and native x86_64-darwin.
Expand Down