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

misc pkgs: various cross fixes in preparation for darwin->linux #46857

Merged
merged 4 commits into from Sep 18, 2018
Merged

misc pkgs: various cross fixes in preparation for darwin->linux #46857

merged 4 commits into from Sep 18, 2018

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Sep 18, 2018 •
edited

Motivation for this change

Non-staging, and non-darwin-specific parts of #46534.

By commit:

  1. Avoids silly assertion failures and makes for more consistency
  2. Makes native and cross musl alike get new gnu-config scripts.
  3. The static stage GCC shouldn't be built with a libc, other than weird windows bootstrapping.
  4. libmpx requires libc, and so should be built only in the second GCC. We didn't hit this with other cross because it's an x86-only library.

Backport: #46858 46858.

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.

@Ericson2314 Ericson2314 mentioned this pull request Sep 18, 2018
Merged
9 tasks
@GrahamcOfBorg GrahamcOfBorg added 6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment labels Sep 18, 2018
matthewbauer
matthewbauer reviewed Sep 18, 2018
View reviewed changes
pkgs/stdenv/generic/make-derivation.nix Outdated
@@ -176,7 +174,7 @@ rec {
// {
# A hack to make `nix-env -qa` and `nix search` ignore broken packages.
# TODO(@oxij): remove this assert when something like NixOS/nix#1771 gets merged into nix.
name = assert validity.handled; name + lib.optionalString
name = assert (validity.handled && separateDebugInfo -> stdenv.hostPlatform.isLinux); name + lib.optionalString
Copy link
Member

Choose a reason for hiding this comment

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

I think we can just leave this off. It's awkward to put it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

But then I'd be removing the assertion altogether? if check-meta checked things besides meta that would clean this up, but that's a larger refactor.

Copy link
Member

Choose a reason for hiding this comment

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

It just doesn't seem like a reason for a package to be "broken". Maybe replace separateDebugInfo references above with separateDebugInfo && stdenv.hostPlatform.isLinux?

Copy link
Member Author

Choose a reason for hiding this comment

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

But then I'm still changing the interface? Also asking for separateDebugInfo and not getting it could be confusing, and finally re-tightening the interface after its so loosened is a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

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

2a28bc6 is the origin of the assert, BTW.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/iw94llkj05wgaz268mlzvgx8jkbi1ss0-gcc-wrapper-7.3.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/pcrn3iq0i5s590mlp8v6m7jp1rn7h61r-gcc-wrapper-7.3.0

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/jcq8f90rikrir6pmvjv48m4p2a89ss99-gcc-wrapper-7.3.0

@Ericson2314
mkDerivation: Make separateDebugInfo assertion lazier to match othe…
2111e7b 
…r assertions

This is needed to access attributes of derivations on platforms where
they cannot be built.
@Ericson2314
stdenv linux, stdenv cross: Harmonize extraNativeBuildInputs
f2bb59e 
Want to make sure these are the same per host platform, without duplication.
@Ericson2314
gccCrossStageStatic: Don't use MUSL, or any libc when targetting linux
28a323e 
Only the regular GCC is built with a libc dependency.
@Ericson2314
gcc: Clarify Disabling libmpx in cross stage static
2091133 
This isn't a MUSL thing, but just needed for cross compilation to x86.
No one had tried this when all cross compilation was to linux + glibc,
hence why no one noticed this until recently.
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/iw94llkj05wgaz268mlzvgx8jkbi1ss0-gcc-wrapper-7.3.0

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/jcq8f90rikrir6pmvjv48m4p2a89ss99-gcc-wrapper-7.3.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gcc

Partial log (click to expand)

/nix/store/pcrn3iq0i5s590mlp8v6m7jp1rn7h61r-gcc-wrapper-7.3.0

@Ericson2314 Ericson2314 merged commit 35378f0 into NixOS:master Sep 18, 2018
@Ericson2314 Ericson2314 deleted the darwin-to-linux-prep branch September 18, 2018 20:52
matthewbauer added a commit that referenced this pull request Sep 26, 2018
@matthewbauer
stdenv: partial revert of f2bb59e
f9c4075 
/cc @Ericson2314

PR was #46857

This line broke MacOS cross compilation. paxctl cannot be built on
macOS. Maybe it can be fixed, but no reason to break things
unnecessarily.

Regardless, you definitely need to be more careful about backporting.
I think it’s fine to move fast and break things on master but
with release-18.09 we should be more careful. Something like more
automated testing for cross compilation would also be
helpful (hopefully even making it block).
Ericson2314 pushed a commit that referenced this pull request Sep 26, 2018
@matthewbauer @Ericson2314
stdenv: partial revert of f2bb59e
607063f 
/cc @Ericson2314

PR was #46857

This line broke MacOS cross compilation. paxctl cannot be built on
macOS. Maybe it can be fixed, but no reason to break things
unnecessarily.

Regardless, you definitely need to be more careful about backporting.
I think it’s fine to move fast and break things on master but
with release-18.09 we should be more careful. Something like more
automated testing for cross compilation would also be
helpful (hopefully even making it block).

(cherry picked from commit f9c4075)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer left review comments

@dtzWill dtzWill Awaiting requested review from dtzWill

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Ericson2314 @GrahamcOfBorg @matthewbauer