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

pkgsStatic: use clang for C compiler on Darwin #86223

Merged
merged 1 commit into from Sep 30, 2020
Merged

pkgsStatic: use clang for C compiler on Darwin #86223

merged 1 commit into from Sep 30, 2020

Conversation

pikajude
Copy link
Contributor

Motivation for this change

As title. Looks like people haven't really been using pkgsStatic on Darwin for a long time.

In addition to the testing steps listed below, I also ran some manual tests since pkgsStatic has very little test coverage in nixpkgs overall.

# build a static library
$ nix-build . -A pkgsStatic.bzip2
# build a bin that requires CF
$ nix-build . -A pkgsStatic.htop
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • macOS
  • 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 Apr 28, 2020
@emilazy
Copy link
Member

emilazy commented Apr 28, 2020 •
edited

macOS doesn't actually officially support static linking, right? I remember it being brittle and them going to some lengths to break building static executables over time.

Not that I object to supporting it, just wondering if it should maybe have some kind of warning on it.

Edit: Ah, I see; these still link to libSystem, so they're not really "static executables", just statically linked to their nixpkgs dependencies.

matthewbauer
matthewbauer reviewed Apr 28, 2020
View reviewed changes
pkgs/top-level/static.nix
makeStaticDarwin = stdenv: stdenv // {
makeStaticDarwin = stdenv_: let stdenv = stdenv_.override {
# extraBuildInputs are dropped in cross.nix, but darwin still needs them
extraBuildInputs = [ self.buildPackages.darwin.CF ];
Copy link
Member

Choose a reason for hiding this comment

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

This probably just works in native compilation

Copy link
Contributor Author

@pikajude pikajude Apr 28, 2020 •
edited

Choose a reason for hiding this comment

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

It does not. pkgsStatic.htop fails without it. (Although that does bring up the question of why extraBuildInputs exists at all.)

matthewbauer
matthewbauer reviewed Apr 28, 2020
View reviewed changes
pkgs/stdenv/cross/default.nix
@@ -63,6 +63,8 @@ in lib.init bootStages ++ [
# `tryEval` wouldn't catch, wrecking accessing previous stages
# when there is a C compiler and everything should be fine.
then throw "no C compiler provided for this platform"
else if crossSystem.isDarwin
Copy link
Member

Choose a reason for hiding this comment

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

We might need this to be an option. I think we want there to be a way to also use gcc for cross on Darwin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's possible but it would require nixpkgs to support Darwin as a cross target with a non-Darwin build platform, and I don't know if that's the case? I wasn't able to build a cc toolchain for it when I tried, anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah - that's not supported. But we do want to support building statically on native Darwin gcc. That might not be as useful as I was originally thinking though.

matthewbauer
matthewbauer approved these changes May 10, 2020
View reviewed changes
Copy link
Member

@matthewbauer matthewbauer left a comment

Choose a reason for hiding this comment

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

Looks good for now! The gcc option can be easily added later.

@infinisil infinisil merged commit 4aabac8 into NixOS:master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@matthewbauer matthewbauer matthewbauer approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

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
5 participants
@pikajude @emilazy @matthewbauer @Mic92 @infinisil