Skip to content

Comments

darwin: introduce darwinArch, apply in {cc,bintools}-wrappers#114817

Merged
Ericson2314 merged 5 commits intoNixOS:stagingfrom
thefloweringash:darwin-arch
Mar 2, 2021
Merged

darwin: introduce darwinArch, apply in {cc,bintools}-wrappers#114817
Ericson2314 merged 5 commits intoNixOS:stagingfrom
thefloweringash:darwin-arch

Conversation

@thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Mar 2, 2021

Motivation for this change

Extracted out of #105026. This, as well as #111988, try to make the darwin {cc,bintools}-wrappers use the correct flags for the target platform. A version of these flags previously existed only in the xcode wrapper.

Tested only as part of #105026. It changes the compiler and linker flags for all darwin packages, so there's a potential for a lot of breakage.

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.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Mar 2, 2021
@thefloweringash thefloweringash changed the title Darwin arch darwin: introduce darwinArch, apply in {cc,bintools}-wrappers Mar 2, 2021
@ofborg ofborg bot added the 6.topic: stdenv Standard environment label Mar 2, 2021
@ofborg ofborg bot added the 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. label Mar 2, 2021
@ofborg ofborg bot requested review from Synthetica9, davidtwco, peti and veprbl March 2, 2021 08:47
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Mar 2, 2021
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Great!

@Ericson2314 Ericson2314 merged commit 09d0e10 into NixOS:staging Mar 2, 2021
@bobrik
Copy link
Contributor

bobrik commented Mar 3, 2021

Looks like it broke stdenv on x86_64-darwin:

clang-7: error: cannot use 'cpp-output' output with multiple -arch options
mig: fatal: "<no name yet>", line -1: no SubSystem declaration
make[2]: *** [Makefile:754: kcmrpc.h] Error 1
make[2]: Leaving directory '/private/tmp/nix-build-libkrb5-1.18.drv-0/krb5-1.18/src/lib/krb5/ccache'
make[1]: *** [Makefile:1154: all-recurse] Error 1
make[1]: Leaving directory '/private/tmp/nix-build-libkrb5-1.18.drv-0/krb5-1.18/src/lib/krb5'
make: *** [Makefile:1004: all-recurse] Error 1
builder for '/nix/store/hawwz9cvywyqsn3180cl3afnpf7k47y0-libkrb5-1.18.drv' failed with exit code 2
building '/nix/store/y269ail4wsxszqgszhx00nsph44gsyhw-binutils-2.35.1.drv'...
cannot build derivation '/nix/store/fgxnwij7bnq1iw4c581ndaiyxd3d5a38-bootstrap-stage2-stdenv-darwin.drv': 1 dependencies couldn't be built

@thefloweringash
Copy link
Member Author

Looks like it broke stdenv on x86_64-darwin:

I was so sure it had been tested as part of the apple-silicon branch. I'm building staging now to debug. I'm hoping it'll be fixed by the commit titled darwin.bootstrap_cmds: use correct arch in "mig".

@bobrik
Copy link
Contributor

bobrik commented Mar 3, 2021

@thefloweringash looks like thefloweringash@178ad9a indeed fixes the issue.

@thefloweringash
Copy link
Member Author

@thefloweringash looks like thefloweringash@178ad9a indeed fixes the issue.

Excellent! Thank you for testing.

I've opened this as #114944.

@thefloweringash thefloweringash deleted the darwin-arch branch March 19, 2021 08:52
@thefloweringash
Copy link
Member Author

thefloweringash commented Mar 19, 2021

This broke llvmPackages_11.compiler-rt and the problem seems to come from the intermediate .a archives having multiple architectures. More broadly, I think this PR will interfere with universal binaries and compiler-rt just happens to be the one I noticed. I infer that -arch flags combine, which means this should be providing a default value, not always passing a value.

github-actions bot pushed a commit to thefloweringash/nixpkgs that referenced this pull request Mar 19, 2021
Workaround for build failure after adding mandatory -arch
argument. Nixpkgs targets a single architecture, so this saves a small
amount of wasted build effort.

See NixOS#114817 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants