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

Rust-overlay seem to confuse $NIX_LDFLAGS when cross-compiling? #40

Closed
abbec opened this issue May 17, 2021 · 9 comments · Fixed by #41
Closed

Rust-overlay seem to confuse $NIX_LDFLAGS when cross-compiling? #40

abbec opened this issue May 17, 2021 · 9 comments · Fixed by #41
Labels
bug Something isn't working

Comments

@abbec
Copy link
Contributor

abbec commented May 17, 2021

I have an issue that I cannot quite make sense of. We updated this overlay from 0bcf4e7 to be2d768 and that changed the content in $NIX_LDFLAGS somehow?

0bcf4e7:

echo $NIX_LDFLAGS
-L/nix/store/r5y3drh0fdgmjyibgs6gxqvv457v5vwr-mcfgthreads-git-x86_64-w64-mingw32/lib -L/nix/store/glghra1n3r22zpbnfq6pgizn0zy02rdl-pthreads-w32-2.9.1-x86_64-w64-mingw32/lib

be2d768:

 echo $NIX_LDFLAGS
-liconv -L/nix/store/1wv35rsgcrwjz4jnqs3kq77k6d3qagaf-libc++-7.1.0/lib -L/nix/store/1wv35rsgcrwjz4jnqs3kq77k6d3qagaf-libc++-7.1.0/lib -L/nix/store/wahi8q1ybh9n805zbavs6ch4i8mfpzgy-libc++abi-7.1.0/lib -L/nix/store/wahi8q1ybh9n805zbavs6ch4i8mfpzgy-libc++abi-7.1.0/lib -L/nix/store/6bw7mnz3yp2bk2qvq4xi7vj0xz0h63mm-compiler-rt-7.1.0/lib -L/nix/store/6bw7mnz3yp2bk2qvq4xi7vj0xz0h63mm-compiler-rt-7.1.0/lib -L/nix/store/2jlggnhkjnpw938ak2w6rljlak8dzjbz-libiconv-osx-10.12.6/lib -L/nix/store/2jlggnhkjnpw938ak2w6rljlak8dzjbz-libiconv-osx-10.12.6/lib -L/nix/store/r5y3drh0fdgmjyibgs6gxqvv457v5vwr-mcfgthreads-git-x86_64-w64-mingw32/lib -L/nix/store/r5y3drh0fdgmjyibgs6gxqvv457v5vwr-mcfgthreads-git-x86_64-w64-mingw32/lib -L/nix/store/glghra1n3r22zpbnfq6pgizn0zy02rdl-pthreads-w32-2.9.1-x86_64-w64-mingw32/lib -L/nix/store/glghra1n3r22zpbnfq6pgizn0zy02rdl-pthreads-w32-2.9.1-x86_64-w64-mingw32/lib

The latter erroneously contains build platform dependencies. How can this overlay change that?

@oxalica oxalica added the bug Something isn't working label May 17, 2021
@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

This is the only change between the two outputs as well, nothing else changed.

@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

This was introduced in d3956f0 and it seems to be using a "targettarget" dependency, "A list of dependencies whose host platform matches the new derivation's target platform". This is not true for example for libiconv for macOS when cross-compiling to Windows. I.e. libiconv for macOS (the build platform) does not match the new derivation's target platform. But I might have confused the nomenclature used here.

Shouldn't things like libiconv rather be in propagatedNativeBuildInputs? It's existence in nix-support/propagated-target-target-deps causes it to show up in NIX_LDFLAGS at least, causing a build fail with x86_64-w64-mingw32-ld: cannot find -liconv (the windows program does not need libiconv)

@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

Furthermore, should the code really be using stdenv.targetPlatform? Should it not be using hostPlatform and buildPlatform as here: https://github.com/NixOS/nixpkgs/blob/056af90d7ea2b9f567b9da0c771002c485c9b818/pkgs/build-support/rust/hooks/default.nix#L21. My understanding was that targetPlatform is really only used when dealing with single-target things like gcc (which needs to be built on a build platform for a target platform to produce binaries for a host platform)? Again, I might have misunderstood this but it has seemed to work so far :)

@oxalica
Copy link
Owner

oxalica commented May 17, 2021

should the code really be using stdenv.targetPlatform?

Actually, it doesn't matter. Since rustc is a cross compiler, which itself support multiple target. The only difference is what std libraries are packed in final derivation by us (extensions). When building the compiler itself, hostPlatform and targetPlatform are always the same. (They differs when using crossSystem)

Consider: you are on darwin, you select both darwin's and linux's std, so that you can both compiling native binaries and cross compiling linux binaries. In this case, we can only decide whether to include libiconv in (compiler's) runtime, since you can use the same rustc to compile to difference targets.
I also tried to add RPATH to darwin std, but it doesn't work.
I'm considering wrapping the rustc binary to make condition from --target argument.

BTW: the libiconv dependency of std is (unexpectedly) introduced in about 1.52.0 and is currently fixed by rust-lang/libc#2150 . Hope we can get rid of this in the next stable release.

@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

Hmm, but stdenv/setup seems to disagree. I have fixed the liviconv dependency on the build platform (darwin in my case) by adding it to depsBuildBuild (it is only needed when actually building for Darwin (or for build.rs) not when cross-compiling to for example Windows). And libiconv should not be in NIX_LDFLAGS, that is wrong. I.e. libiconv for macOS should not show up as a link argument to a windows binary, which it currently does!

Unfortunately, the introduction of the cross compile fixes mentioned above renders this overlay unusable for us when it worked fine before that :(

@oxalica
Copy link
Owner

oxalica commented May 17, 2021

Do you mean to change this into:

depsBuildBuildPropagated =
   self.lib.optional (self.stdenv.hostPlatform.isDarwin) self.libiconv;

Sorry I'm not an expert of cross compilation nor a mac user. If you already made it work, PR is welcome!

@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

Yes, that is what I meant. I can whip up a PR tomorrow :)

@abbec
Copy link
Contributor Author

abbec commented May 17, 2021

In short, I think the platforms are mixed up here (and they do have confusing names, I agree). From the manual on cross-compilation: "In summary, build is the platform on which a package is being built, host is the platform on which it will run. The third attribute, target, is relevant only for certain specific compilers and build tools (read gcc, binutils)."

Host is the one that tripped me up initially that it is not the platform we are currently on, instead it is the host platform of the new derivation.

@abbec
Copy link
Contributor Author

abbec commented May 18, 2021

PR in #41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants