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] Rust cross compilation #50866

Closed
wants to merge 1 commit into from
Closed

[WIP] Rust cross compilation #50866

wants to merge 1 commit into from

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Nov 21, 2018

Status

  • rustc
  • fix non-cross rustc
  • buildRustPackage
  • some packages need fixes & testing
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@Mic92 Mic92 requested a review from LnL7 as a code owner November 21, 2018 01:58
@Mic92
Copy link
Member Author

Mic92 commented Nov 21, 2018

Still compiling. Cross-compiled LLVM is also still broken.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: libpfm, openssl

Partial log (click to expand)

gzipping man pages under /nix/store/m9izkl3p2hr6xngm529jl1gy0ql3gj42-openssl-1.0.2p-man/share/man/
patching script interpreter paths in /nix/store/m9izkl3p2hr6xngm529jl1gy0ql3gj42-openssl-1.0.2p-man
checking for references to /build/ in /nix/store/m9izkl3p2hr6xngm529jl1gy0ql3gj42-openssl-1.0.2p-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/q66rhjlqjkn194c6zji5sjmr3p6z1z6y-openssl-1.0.2p-debug
patching script interpreter paths in /nix/store/q66rhjlqjkn194c6zji5sjmr3p6z1z6y-openssl-1.0.2p-debug
checking for references to /build/ in /nix/store/q66rhjlqjkn194c6zji5sjmr3p6z1z6y-openssl-1.0.2p-debug...
cannot link '/nix/store/.links/0jv8zns4raaa2vc5gjwxqm9n5wx9qi6lbv8556dg6zg80aw3z0yr' to '/nix/store/m9izkl3p2hr6xngm529jl1gy0ql3gj42-openssl-1.0.2p-man/share/man/man3/EVP_PKEY_derive_set_peer.3.gz': No space left on device
cannot link '/nix/store/.links/0jv8zns4raaa2vc5gjwxqm9n5wx9qi6lbv8556dg6zg80aw3z0yr' to '/nix/store/m9izkl3p2hr6xngm529jl1gy0ql3gj42-openssl-1.0.2p-man/share/man/man3/EVP_PKEY_derive_init.3.gz': No space left on device
/nix/store/k4dhk7hqqy3kzmgdvs0j5pi88gfjr4pp-libpfm-4.10.1
/nix/store/pl3km7jksa9r6zrvrwsh5djaxgk004s0-openssl-1.0.2p-bin

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: libpfm, openssl

Partial log (click to expand)

checking for references to /build/ in /nix/store/818azw7kbva6jznsj438h05lh33r83wv-openssl-1.0.2p...
shrinking RPATHs of ELF executables and libraries in /nix/store/995mh5wj5dayr041hjldyw2a9wp94qr5-openssl-1.0.2p-man
gzipping man pages under /nix/store/995mh5wj5dayr041hjldyw2a9wp94qr5-openssl-1.0.2p-man/share/man/
patching script interpreter paths in /nix/store/995mh5wj5dayr041hjldyw2a9wp94qr5-openssl-1.0.2p-man
checking for references to /build/ in /nix/store/995mh5wj5dayr041hjldyw2a9wp94qr5-openssl-1.0.2p-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/8arzdc863bwrqx50jv441vqfxfbxpjgg-openssl-1.0.2p-debug
patching script interpreter paths in /nix/store/8arzdc863bwrqx50jv441vqfxfbxpjgg-openssl-1.0.2p-debug
checking for references to /build/ in /nix/store/8arzdc863bwrqx50jv441vqfxfbxpjgg-openssl-1.0.2p-debug...
/nix/store/d8g0sv493ar86xvz97bc6ndlv6h3l5g4-libpfm-4.10.1
/nix/store/7y00xv5kwkjvy3pq4npq6finarpyxwc5-openssl-1.0.2p-bin

nativeBuildInputs = [unzip];
propagatedNativeBuildInputs = [ findXMLCatalogs ];
nativeBuildInputs = [ unzip ];
propagatedBuildInputs = [ findXMLCatalogs ];
Copy link
Member

Choose a reason for hiding this comment

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

This is a actually a bug I think. "propagatedNativeBuildInputs" should be correct. Will have a fix soon hopefully.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I can wait a bit, I just get the things working properly and not blocking on nitpicks.

Copy link
Member

Choose a reason for hiding this comment

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

findXMLCatalogs does sound like a tool that should be propagatedNativeBuildInputs.

@@ -105,6 +105,10 @@ let
'';

postFixup = ''
# fixes cross-compilation to pick the right perl,
# since perl is also in nativeBuildInputs
(sed -i -e '1 s!^.*$!${perl}/bin/perl!' $bin/bin/c_rehash)
Copy link
Member

Choose a reason for hiding this comment

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

I didn't think this would be a problem now that #49608 is merged. Do you know where this comes from? It also looks like openssl supports passing in HASHBANGPERL to do this explicity.

Copy link
Member Author

Choose a reason for hiding this comment

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

HASHBANGPERL is only supported in openssl 1.1, but we still use openssl 1.0.2 in most places.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also #49608 was reverted again in staging by @LnL7

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the stdenv was broken. Since this is the second time I think that change has a bit too much impact to merge into staging directly, we should run it through a separate jobset first to get an overview of the breakages.

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably don't need this change to make cross-compiling rust packages work. So I will split it of probably later.

@GrahamcOfBorg
Copy link

Timed out, unknown build status on x86_64-darwin (full log)

Attempted: openssl

The following builds were skipped because they don't evaluate on x86_64-darwin: libpfm

Partial log (click to expand)

ld: warning: directory not found for option '-L../bootstrap/x86_64-apple-darwin/usr/lib'
ld: warning: undefined base symbol '__TMC15SwiftFoundation19_NSCFConstantString' for alias '___CFConstantStringClassReference'
installing
post-installation fixup
strip is /nix/store/2ws9cmamvr7xyvdg4d2nnd1bmr1zjrrq-bootstrap-tools/bin/strip
patching script interpreter paths in /nix/store/awcr3pzcd5vpqwh3lhb0ijxbv72vf9l7-swift-corefoundation
cannot build derivation '/nix/store/4xqfqm5lbdqpb9qds1xxrzmyss3818h3-stdenv-darwin.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/mv8zixg5sy9lqnqnn15ca4wl2vxipii6-perl-5.28.0.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/7lipgdn8jvyy7k45idq1sh17k7bpj9z3-openssl-1.0.2p.drv': 2 dependencies couldn't be built
error: build of '/nix/store/7lipgdn8jvyy7k45idq1sh17k7bpj9z3-openssl-1.0.2p.drv' failed

@Mic92
Copy link
Member Author

Mic92 commented Nov 21, 2018

Mhm, I currently stuck at this fix-point stuff that is used to create buildRustPackage. It somehow puts a non-native rustc/cargo into nativeBuildInputs:

nix-repl> pkgsCross.aarch64-multiplatform.fd.nativeBuildInputs
[ «derivation /nix/store/d9abfxjb0icbjg7k8j62pasgdj7s8zwl-git-2.19.1.drv» «derivation /nix/store/gnvkwv8mls6a2n4bpw3xdpp6fh1d1lyc-cargo-1.30.0-aarch64-unknown-linux-gnu.drv» «derivation /nix/store/hibvax8p73n9imhi9ix7z55ka5hl47ai-rustc-1.30.1-aarch64-unknown-linux-gnu.drv» ]

@Mic92
Copy link
Member Author

Mic92 commented Nov 21, 2018

Build inputs are clean now.

$ nix-repl> pkgsCross.aarch64-multiplatform.fd.nativeBuildInputs
[ «derivation /nix/store/zp9aqlm1gg9k2hz990yvvwfgja3ybyiy-cargo-1.30.0.drv» «derivation /nix/store/4sjcxw65jn8l90ql2ivn2znlpw24fc09-rustc-1.30.1.drv» «derivation /nix/store/d9abfxjb0icbjg7k8j62pasgdj7s8zwl-git-2.19.1.drv» «derivation /nix/store/8ns0q8dc8bcxyc178ck6zcmhp39rvhwa-nss-cacert-3.40.drv» ]

I think most cross-compile fixes from this pr are actually not needed.
I will separate them later into a new pull request.

@Mic92
Copy link
Member Author

Mic92 commented Nov 25, 2018

Compilation works now!

$ rustc --target=aarch64-unknown-linux-gnu foo.rs
$ file foo
foo: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /nix/store/1r1pdrdwgj6k3wy3bhrxjiqjdxf9z2f5-glibc-2.27-aarch64-unknown-linux-gnu/lib/ld-linux-aarch64.so.1, for GNU/Linux 2.6.32, with debug_info, not stripped

@Mic92
Copy link
Member Author

Mic92 commented Nov 28, 2018

The first working buildRustPackage!

$ file result/bin/fd
result/bin/fd: ELF 64-bit LSB shared object, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /nix/store/1r1pdrdwgj6k3wy3bhrxjiqjdxf9z2f5-glibc-2.27-aarch64-unknown-linux-gnu/lib/ld-linux-aarch64.so.1, for GNU/Linux 2.6.32, not stripped

"-DTARGET_TRIPLE=${stdenv.hostPlatform.config}"

"-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.targetPlatform.config}"
"-DLLVM_TARGETS_TO_BUILD=${llvmTarget stdenv.hostPlatform};${llvmTarget stdenv.targetPlatform}"
Copy link
Member

Choose a reason for hiding this comment

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

Can we do this for the other LLVMs?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will split this off into a different pull request: #51226


"${setBuild}.jemalloc=${hostJemalloc}/lib/libjemalloc_pic.a"
"${setHost}.jemalloc=${hostJemalloc}/lib/libjemalloc_pic.a"
"${setTarget}.jemalloc=${targetJemalloc}/lib/libjemalloc_pic.a"
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ericson2314 this one seems actually undefined, when I try to build nix-build default.nix -A pkgsCross.aarch64-multiplatform.rustc

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that's OK. For now. I need to change something in booter.nix to avoid these issues. Need to throw in a buildPackages to get the cross compiler vs cross compiler compiler, which is the immediate need anyways.

@Ericson2314
Copy link
Member

Is this just blocked on ofborg?

@Mic92
Copy link
Member Author

Mic92 commented Jan 2, 2019

It also requires to fix a few cargo packages that depend on ./target/release to be present, as now cross-compiled packages are put into a architecture subdirectory that can be accessed by using $releaseDir

@Ericson2314
Copy link
Member

If you want to list those, I'm happy to divide + conquer on fixing that + eval failures.

@Mic92
Copy link
Member Author

Mic92 commented Jan 2, 2019

I will not have a look at this the next few days, so you could just fix the
few packages right away as it is only string-replacement in 11 packages. (target/release -> $releaseDir)

$ ag -l 'target/release'
pkgs/development/tools/rust/racerd/default.nix
pkgs/development/tools/misc/ycmd/default.nix
pkgs/build-support/rust/default.nix
pkgs/tools/text/ripgrep/default.nix
pkgs/tools/misc/vdirsyncer/default.nix
pkgs/tools/misc/fd/default.nix
pkgs/tools/misc/bat/default.nix
pkgs/applications/altcoins/zcash/librustzcash/default.nix
pkgs/applications/networking/dyndns/cfdyndns/default.nix
pkgs/applications/misc/alacritty/default.nix
pkgs/applications/window-managers/wtftw/default.nix

This should probably also mentioned in the changelog since this also affects out-of-tree rust packages.

@Ericson2314
Copy link
Member

We talked briefly about this on IRC. I think it should action be changed back to target/release since we only install for a single platform at once. This could be done with some fixup phase thing.

@illegalprime
Copy link
Member

should armv7l-unknown-linux-gnueabihf be mapped to armv7-unknown-linux-gnueabihf (since that's the triple rust supports)?
also re: target/release, is it just a matter of adding:

"mv ${releaseDir} target/"

after cargo build?
this looks great BTW, can't wait!

@@ -36,21 +37,22 @@ let
cargoDepsCopy="$sourceRoot/${cargoVendorDir}"
'';

ccForBuild="${buildPackages.stdenv.cc}/bin/${buildPackages.stdenv.cc.targetPrefix}cc";
Copy link
Member

Choose a reason for hiding this comment

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

The build cc is almost never prefixed. I suppose there could be some weird case where it needs to be though.

@illegalprime
Copy link
Member

illegalprime commented Feb 22, 2019

I rebased this on the latest nixos-unstable and did the renaming for target/release as a postBuild hook, maybe it'll help:
fa82ebc...illegalprime:rust-cross
still testing but fd seems to build, ripgrep will build after #54510 i believe

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

@illegalprime can you just open a new pull request?
Then we can continue from there.

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

We can get rid of $releaseDir in this case I think:
fa82ebc...illegalprime:rust-cross#diff-2c7a7586fcf2d0d0d778bdddcf893a1bR124

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

and because people might have overwritten postBuild this could be moved to buildPhase itself.

@illegalprime
Copy link
Member

will do! I believe pkgs.pkgsCross.aarch64-multiplatform.glibc is broken on staging so I'm trying to bisect. i'll post a PR when i have some concrete information on what works or not

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

You don't need to bisect. This was a commit where I introduced C.UTF-8: #56143 (comment)
The build process of this locale puts a non-native libc into the library path unfortunately.

@vcunat
Copy link
Member

vcunat commented Feb 22, 2019

We could temporarily drop the new locale when cross-compiling or something like that, right? EDIT: I mean, we worked without it until recently, so it shouldn't be too bad...

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

@vcunat that might work also it would break other packages that assumes we have it, which might be python packages rather soon.

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

the problem is $(rtld-prefix) in the glibc makefile, which contains the host ld binary and is prefixed in the locale generation.

@Mic92
Copy link
Member Author

Mic92 commented Feb 22, 2019

It is not too complicated to call localedef manually:
http://www.linuxfromscratch.org/clfs/view/clfs-2.0/arm/cross-tools/glibc.html
They seems to differentiate between endianness though by a flag, which we can query from our stdenv too.

@illegalprime
Copy link
Member

thanks for linking that @Mic92!

bisecting is weird, maybe I don't understand it enough or all of the merge commits are making it complicated but this didn't find the right commit:

git bisect start nixpkgs/staging nixpkgs-channels/nixos-unstable
git bisect run nix build -f . pkgs.pkgsCross.aarch64-multiplatform.glibc

however this did:

git bisect start nixpkgs/staging HEAD~600
git bisect run nix build -f . pkgs.pkgsCross.aarch64-multiplatform.glibc

picking 600 commits in the past was just arbitrary and I'm lucky it found the commit , but how should one do this bisect in the future? do i have to flatten out the history to do it right or is my understanding just poor?

@vcunat
Copy link
Member

vcunat commented Feb 23, 2019

Are you sure that nixpkgs-channels/nixos-unstable was a good commit (at that time)? I had bisected across merges many times and haven't noticed any issue with the algorithm, except that merges complicate the whole concept.

@vcunat
Copy link
Member

vcunat commented Feb 23, 2019

it would break other packages that assumes we have it

Well, better than breaking glibc build already :-) It's why I meant it as a temporary workaround. The "assumes we have it" part won't be portable to non-glibc anyway, but perhaps those don't need it or something...

@Mic92
Copy link
Member Author

Mic92 commented Feb 23, 2019

@vcunat this should be the proper fix instead: #56235

@Mic92
Copy link
Member Author

Mic92 commented Mar 13, 2019

Superseded by #56540

@Mic92 Mic92 closed this Mar 13, 2019
@Mic92 Mic92 deleted the rust-cross branch March 13, 2019 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants