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

bintools-wrapper: add support for frameworks #41914

Closed
wants to merge 1 commit into from

Conversation

uri-canva
Copy link
Contributor

Motivation for this change

Equivalent to what was done in 7bea6aa for cc-wrapper.

This adds the right -F flags to NIX_LDFLAGS when adding frameworks to build inputs.

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/)
  • Fits CONTRIBUTING.md.

@uri-canva
Copy link
Contributor Author

See https://github.com/NixOS/nixpkgs/pull/41911/files#diff-4746b81564a01e241a84d455575437e7R60 for example of somewhere where NIX_LDFLAGS had to be added, but NIX_CFLAGS_COMPILE already had the right values.

@matthewbauer
Copy link
Member

This is probably good to do! I am still trying to figure out why most of our stuff still works without these changes though.

@Ericson2314
Copy link
Member

I think the compiler takes -F and turns it into -L. I didn't know cctools ld took -F directly.

@uri-canva
Copy link
Contributor Author

It's possible a lot of the packages' build definitions will call CC to link instead of LD, and NIX_CFLAGS_COMPILE has those flags.

@uri-canva
Copy link
Contributor Author

It's hard to test this because a lot of packages on darwin don't build successfully on staging. Should I rebase on stable? Is darwin checked on hydra?

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 19, 2018

You can rebase on git merge-base master staging, and then target staging but your base branch is a master one and likely to be good then.

@vcunat vcunat added the 6.topic: darwin Running or building packages on Darwin label Aug 30, 2018
@vcunat
Copy link
Member

vcunat commented Aug 30, 2018

Ping?

I'm not a Darwin person; I just wonder what will make it to 18.09, and this seems safe enough for non-Darwin.

@Ericson2314
Copy link
Member

I guess this is not necessary. But somebody knowing more should confirm.

@uri-canva
Copy link
Contributor Author

This isn't necessary per se, I wrote in the workaround in the Bazel derivation:

export NIX_LDFLAGS="$NIX_LDFLAGS -F${CoreFoundation}/Library/Frameworks -F${CoreServices}/Library/Frameworks -F${Foundation}/Library/Frameworks"

It just would be better to not need this, since bintools already handles libraries on its own, it should do the same for frameworks.

I think the reason it's the only derivation that requires this is that it doesn't pass on compiler flags to the linker.

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.

None yet

5 participants