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

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

Merged
merged 5 commits into from
Mar 2, 2021

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
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants