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.

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

Jump to bottom

darwin stdenv: Make stdenv.cc, not stdenv itself, bring in libcxx #33042

Merged
merged 1 commit into from Dec 26, 2017
Merged

darwin stdenv: Make stdenv.cc, not stdenv itself, bring in libcxx #33042

merged 1 commit into from Dec 26, 2017

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Dec 25, 2017 •
edited

Motivation for this change

stdenvNoCC should not inject any C++ standard library, just as it doesn't inject any C standard library. stdenv still does, but only indirectly through stdenv.cc. Wrapped clangs can be simplified now that they don't need to worry about clobbering CoreFoundation when replacing the C++ standard library implementation.

This generally-good cleanup should assist with debugging some C++ failures in #26805.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@Ericson2314 Ericson2314 added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Dec 25, 2017
@Ericson2314 Ericson2314 added this to Needed by the big PR---nice to move pick off pieces of it and move here, rebasing the big PR on top in Cross compilation Dec 25, 2017
@Ericson2314
Copy link
Member Author

@nixborg build

@nixborg
Copy link

nixborg commented Dec 25, 2017

Jobset created at https://hydra.mayflower.de/jobset/nixos/pr-33042

orivej
orivej approved these changes Dec 25, 2017
View reviewed changes
Copy link
Contributor

@orivej orivej left a comment

Choose a reason for hiding this comment

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

I have not tested, but the changes look good.

The title seems misleading, as stdenv still brings libcxx, but stdenvNoCC no longer does.

LnL7
LnL7 approved these changes Dec 25, 2017
View reviewed changes
Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

Great! I was thinking of making libcxx something first-class in the cc-wrapper, similar to libc. But this is already gets rid of most of the weirdness.

@Ericson2314 Ericson2314 changed the title darwin stdenv: Make stdenv.cc, not stdenv, bring in libcxx darwin stdenv: Make stdenv.cc, not stdenv directly, bring in libcxx Dec 26, 2017
@Ericson2314 Ericson2314 changed the title darwin stdenv: Make stdenv.cc, not stdenv directly, bring in libcxx darwin stdenv: Make stdenv.cc, not stdenv itself, bring in libcxx Dec 26, 2017
@Ericson2314
darwin stdenv: Make stdenv.cc, not stdenv, bring in libcxx
4651407 
stdenvNoCC should not inject any C++ standard library, just as it
doesn't inject any C standard library. stdenv still does, but only
indirectly through stdenv.cc. Wrapped clangs can be simplified now that
they don't need to worry about clobbering CoreFoundation when replacing
the C++ standard library implementation.

This generally-good cleanup should assist with debugging some C++
failures in NixOS#26805.
@Ericson2314
Copy link
Member Author

@orivej expanded title and description, hopefully that helps.

@LnL7 yeah that might still be a good idea, but this is an easy incremental step only impacting Darwin.

@Ericson2314
Copy link
Member Author

The nixborg job got stalled, but I added this to #26805 and built stdenv at least.

@Ericson2314 Ericson2314 merged commit 696f3e7 into NixOS:staging Dec 26, 2017
@Ericson2314 Ericson2314 deleted the darwin-cxx branch December 26, 2017 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orivej orivej orivej approved these changes

@LnL7 LnL7 LnL7 approved these changes

Assignees
No one assigned
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 10.rebuild-darwin: 501+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 0
Projects
No open projects
Cross compilation
Needed by the big PR---nice to move p...
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Ericson2314 @nixborg @orivej @LnL7 @GrahamcOfBorg