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

go: add CoreFoundation and Security frameworks for cgo on darwin #90592

Closed
wants to merge 2 commits into from

Conversation

bouk
Copy link
Contributor

@bouk bouk commented Jun 16, 2020

Here I'm adding some CGO flags to ensure the c compiler is able to find system frameworks on Darwin. These frameworks need to be always available, to ensure some parts of the standard library (notably crypto/x509) can be compiled.

I've also added a test that reproduces the issue.

Motivation for this change

See #56348 for background

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 Jun 16, 2020
postInstall = optionalString stdenv.isDarwin (with darwin.apple_sdk.frameworks; ''
wrapProgram $out/share/go/bin/go \
--suffix CGO_CFLAGS ' ' '-iframework ${CoreFoundation}/Library/Frameworks -iframework ${Security}/Library/Frameworks' \
--suffix CGO_LDFLAGS ' ' '-F${CoreFoundation}/Library/Frameworks -F${Security}/Library/Frameworks'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could also use NIX_CFLAGS and NIX_LDFLAGS, not sure what's best?

@kalbasit
Copy link
Member

kalbasit commented Jun 16, 2020

I have not tried this yet, but I can confirm that this is a constant source of annoyance for us because Go crashes anytime we import something that uses C. On my end (at work) I have this snippet which works (copied only relevant parts, but you get the idea):

  # in a let statement
  appleFrameworks = with pkgs.darwin.apple_sdk.frameworks; [
    "-F${CoreFoundation}/Library/Frameworks"
    "-F${CoreServices}/Library/Frameworks"
    "-F${Foundation}/Library/Frameworks"
    "-F${Security}/Library/Frameworks"
  ];

        # in a NixOS option for the environment.
        CGO_CFLAGS = optionalString isDarwin
          (builtins.concatStringsSep " " (builtins.filter (e: e != "") ([
            (builtins.getEnv "CGO_CFLAGS") ] ++ appleFrameworks)));
        CGO_LDFLAGS = builtins.concatStringsSep " " (builtins.filter (e: e != "") (
          [ (builtins.getEnv "CGO_LDFLAGS") ]
          ++ optionals isDarwin (["-lstdc++" "-lm" ] ++ appleFrameworks)

I'm not sure if we need all frameworks though.

@bouk
Copy link
Contributor Author

bouk commented Jun 17, 2020

I looked through the go source, CoreFoundation and Security is all that's needed

@Mic92
Copy link
Member

Mic92 commented Jun 17, 2020

We already do in go:

depsTargetTargetPropagated = optionals stdenv.isDarwin [ Security Foundation ];

This means in a nix-shell there will be both frameworks presents.
Does go pickup our NIX_LDFLAGS/NIX_CFLAGS_COMPILE?
If not rather than wrapping the go compiler we could add a build support hook in go that also sets CGO_CFLAGS and CGO_LDFLAGS. All our C/C++ build infrastructure is build around nix-shell's hence we should stick for that with golang too. There things like pkg-config that also only work meaningful with nix-shell.
The problem of hard-coding is that it makes the go compiler inflexible. For instance it would include darwin frameworks even if you want no frameworks or frameworks for a different platform.

@Mic92
Copy link
Member

Mic92 commented Jun 17, 2020

Also cc @LnL7 and @Ericson2314

@bouk
Copy link
Contributor Author

bouk commented Jun 17, 2020

Yep, it works with nix-shell. Under the hood cgo uses $CC. I have somehow overlooked that those frameworks were already being propagated.

But what I (and others) are running against is that you can't use the compiler directly outside of a nix-shell, since those NIX_LDFLAGS are not set.

Another solution might be for nix-Darwin et al to add flags that are set through depsTargetTargetPropagated (and similar things) in the environment, so the user's shell picks them up. This would emulate being in a nix-shell at all times.

I'd personally prefer the wrapper approach since it reduces the scope of where the flags get set.

@bouk
Copy link
Contributor Author

bouk commented Jun 23, 2020

Closing in favor of #91347

@bouk bouk closed this Jun 23, 2020
@bouk bouk deleted the go-darwin-cgo-frameworks branch June 23, 2020 10:58
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

3 participants