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

Jump to bottom

gcc: place cross-compiled target libraries in lib output #81844

Merged
merged 2 commits into from Mar 6, 2020
Merged

gcc: place cross-compiled target libraries in lib output #81844

merged 2 commits into from Mar 6, 2020

Conversation

lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Mar 5, 2020 •
edited

Motivation for this change

This is my take on an alternative, subjectively cleaner implementation of #58606 and #81781. The only functional difference is that the libraries are placed in $lib/$targetConfig/lib (which is the directory convention using by the GCC build scripts) rather than $lib/lib. This requires a minor change to cc-wrapper. Subjectively, I believe it is cleaner because it unifies the handling of the cross and non-cross cases and is therefore easier to reason about.

I have tested this change by building pkgsCross.armv7l-hf-multiplatform.hello and pkgsCross.aarch64-multiplatform.hello.

While searching for potential ramifications of this change, I found a patchelf invocation that is no longer needed. I have verified that the lib output of the native x86_64 builds of all supported GCC versions still contain no references to their corresponding out output after this change.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc @illegalprime @Floki @matthewbauer @eamsden

@matthewbauer matthewbauer changed the base branch from master to staging March 5, 2020 21:31
@lopsided98
Copy link
Contributor Author

I removed the sotruss commit because I didn't see any references to build time bash. If someone thinks it is still needed, it can be included in a different PR.

@matthewbauer
Copy link
Member

Yeah, this is definitely an improvement over the multi part if statements. If @illegalprime and @eamsden are okay with it, I'll merge this one and close out the other two.

@veprbl veprbl added this to WIP in Staging via automation Mar 5, 2020
@veprbl veprbl moved this from WIP to Ready in Staging Mar 5, 2020
@matthewbauer matthewbauer merged commit b8102aa into NixOS:staging Mar 6, 2020
Staging automation moved this from Ready to Done Mar 6, 2020
This was referenced Mar 6, 2020
Closed
Closed
@Ericson2314
Copy link
Member

I hope someday the location of thtings can be the same for native and cross, but if it solves a real problem it is fine for now. The GCC build system is too much of a mess to fight slowly, it has to be an all or nothing attack.

@lopsided98
Copy link
Contributor Author

@Ericson2314 Isn't your eventual goal to always cross-compile and use prefixed compilers (#44583 #21471), in which case this would be the correct solution and $targetConfig would be set even for native builds.

@Ericson2314
Copy link
Member

@lopsided98 Yes, but my goal is also to build (or at least have build outputs as if we built) standard libraries like regular libraries, which means everything goes in $lib/lib. Given using GCC's monolithic build system, that would mean putting target vs host libs in separate outputs so they would both be in $output/lib.

@lopsided98 lopsided98 deleted the gcc-separate-output-cross branch March 6, 2020 14:04
@misuzu
Copy link
Contributor

misuzu commented Mar 12, 2020

This probably broke i686 builds: https://hydra.nixos.org/build/114095842

@lopsided98 lopsided98 mentioned this pull request Mar 13, 2020
Merged
10 tasks
@lopsided98
Copy link
Contributor Author

#82510 should fix that.

@matthewbauer matthewbauer mentioned this pull request Jun 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

@edolstra edolstra Awaiting requested review from edolstra

@peti peti Awaiting requested review from peti

@davidtwco davidtwco Awaiting requested review from davidtwco

@Synthetica9 Synthetica9 Awaiting requested review from Synthetica9

@veprbl veprbl Awaiting requested review from veprbl

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv
Projects
Staging
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@lopsided98 @matthewbauer @Ericson2314 @misuzu