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

stdenv/adapters.nix: add isStatic when static stdenv adapter is used #97047

Closed
wants to merge 2 commits into from

Conversation

matthewbauer
Copy link
Member

@matthewbauer matthewbauer commented Sep 3, 2020

This corrects an issue from
#96223, where darwin/macOS static
compilation is broken. The issue is that stdenv.hostPlatform.isStatic
is only set on Linux:

isStatic = true;

and not macOS or other platforms. This means that we can’t static
compile on platforms platforms that can’t cross-compile to themselves.
To fix this, move isStatic from stdenv.hostPlatform to stdenv. This
avoids the need to have stdenv.hostPlatform != stdenv.buildPlatform.

We still can have something similar to isStatic in
stdenv.hostPlatform, but instead of being a configurable value, it is
now called ‘hasDynamicLoader’. This should be true for everything but
a few specific platforms we support.

This also makes some weird cases less confusing. For instance, import nixpkgs { crossSystem = { system = builtins.currentSystem; isStatic = true; } would be expected to provide static stdenv, but it actually doesn't.

/cc @vcunat @Ericson2314 @KAction

@matthewbauer matthewbauer requested a review from vcunat September 3, 2020 20:59
@ofborg ofborg bot added 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Sep 3, 2020
@Ericson2314
Copy link
Member

This also makes some weird cases less confusing. For instance, import nixpkgs { crossSystem = { system = builtins.currentSystem; isStatic = true; } would be expected to provide static stdenv, but it actually doesn't.

My intent would just be to fix it so it does. I still fail to see why we need to architect this differently than we do with all the other variations. Also isn't stdenv.hostPlatform != stdenv.buildPlatform the norm anyways, unless one happens to also use `musl natively?

@matthewbauer
Copy link
Member Author

I still fail to see why we need to architect this differently than we do with all the other variations. Also isn't stdenv.hostPlatform != stdenv.buildPlatform the norm anyways, unless one happens to also use `musl natively?

That's true on Linux, but on macOS you can reuse the same compiler / linker / libc for partially static compilation. Ideally we could reuse the native toolchain for static compilation on Linux, but Glibc doesn't support full static linking. If stdenv.hostPlatform != stdenv.buildPlatform, you end up building a full GCC toolchain you don't need.

Static compilation really is different from cross compilation, since native static compilation is a perfectly reasonable thing to do. We do end up reusing some infrastructure here, like strictDeps, but those are separate concepts for a good reason! If we ever find a way to decouple libc & gcc from each other, I think this would be more obvious - switching libc's would just be self: super: { libc = self.musl; } - no crossSystem config necessary, and you'd get to reuse GCC.

Perhaps we should add a "not fully static" Glibc package set to make this more clear. So pkgsPartlyStatic would build all .a libs, but still use Glibc's ld-linux interpreter (this also has benefits for things that need dlopen which doesn't work in `pkgsStatic).

@vcunat
Copy link
Member

vcunat commented Sep 7, 2020

Generally the meaning of isStatic seems to have some unclear edge cases. If everything is static or everything is dynamic, it's clear, but for example, using this adapter on per-package basis may give unexpected problems – though perhaps we don't really care for such edge cases (yet)?

I converted some isMusl cases to isStatic, and even in that small sample I think there were multiple cases: some because the package itself was producing static libs, some because of a dependency being (only) a static lib.

@Ericson2314
Copy link
Member

@matthewbauer but if we reused GCC by building libraries separately like we do with LLVM, we would reduce the GCC even with stdenv.buildPlatform != stdenv.hostPlatform. I don't want to do weird things just because the GCC derivation is monolithic.

@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@Ericson2314
Copy link
Member

@matthewbauer I had to repeat the nix derivation in for static builds in https://github.com/NixOS/nix/blob/92438c70d2291a02577e02b9462f53c23817aebd/flake.nix#L442-L477 because the overlay would try to override the new nix derivation I was putting in. I take this as evidence that having an overlay is the wrong approach. Note that switching selfs to supers would cure this exact problem, have cause the opposite problem where you override for some reason but do still want the static changes.

@FRidh
Copy link
Member

FRidh commented Jan 25, 2021

please avoid merges, rebase instead.

@matthewbauer matthewbauer changed the base branch from master to staging January 25, 2021 19:47
@matthewbauer matthewbauer changed the base branch from staging to master January 25, 2021 19:47
@matthewbauer matthewbauer force-pushed the stdenv-is-static branch 2 times, most recently from 7b821f6 to aa8e35b Compare January 25, 2021 20:01
@matthewbauer
Copy link
Member Author

matthewbauer commented Jan 25, 2021

@Ericson2314 Can you take a look at this again? I've updated the description with a more clear reason of why it's needed. Importantly, stdenv.hostPlatform.isStatic is false on pkgsStatic in darwin (https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/stage.nix#L214-L232). This means that static is broken after the stdenv.hostPlatform.isStatic refactors. This should fix the regression with the least amount of pain.

@matthewbauer matthewbauer changed the base branch from master to staging January 25, 2021 20:05
@matthewbauer matthewbauer changed the base branch from staging to master January 25, 2021 20:05
@Ericson2314
Copy link
Member

I still don't like putting it in stdenv and not hostPlatform. I would rather just pass crossSystem in the darwin case too.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 25, 2021
@matthewbauer
Copy link
Member Author

matthewbauer commented Jan 25, 2021

I still don't like putting it in stdenv and not hostPlatform. I would rather just pass crossSystem in the darwin case too.

This results in:

Package ‘Xcode.app’ in /nix/store/a3n3hnskf2srvphnn13d1sql8ln2jq0d-source/pkgs/os-specific/darwin/xcode/default.nix:36 has an unfree license (‘unfree’), refusing to evaluate.

a) To temporarily allow unfree packages, you can use an environment variable
   for a single invocation of the nix tools.

     $ export NIXPKGS_ALLOW_UNFREE=1

b) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

Alternatively you can configure a predicate to whitelist specific packages:
  { nixpkgs.config.allowUnfreePredicate = pkg: builtins.elem (lib.getName pkg) [
      "Xcode.app"
    ];
  }

c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

(use '--show-trace' to show detailed location information)

because of

else if stdenv.targetPlatform.libc == "libSystem" then darwin.xcode
. I'm open to ideas - but I think this is the only way to get it working without having to get stdenv.hostPlatform != stdenv.buildPlatform gcc working on darwin.

@Ericson2314
Copy link
Member

@matthewbauer What if we made useLLVM true on Darwin -- not by changing the way Darwin is bootstrapped, but making useLLVM confirm with it in the Darwin case. Since useLLVM already makes cross use LLVM and not GCC, I think this static darwin barely-cross would just work?

@matthewbauer
Copy link
Member Author

useLLVM doesn't have any effect because we already have https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/cross/default.nix#L66-L67. The thing that pulls in xcode is a usage of binutils in bash at

++ optional stdenv.hostPlatform.isDarwin binutils
. I think any usage of gcc / binutils will end up causing this problem, regardless of useLLVM.

This corrects an issue from
NixOS#96223, where darwin/macOS static
compilation is broken. The issue is that stdenv.hostPlatform.isStatic
is only set on Linux:

https://github.com/NixOS/nixpkgs/blob/916815204eac9e4a41f263dd9855e51380135674/pkgs/top-level/stage.nix#L221

and not macOS or other platforms. This means that we can’t static
compile on platforms platforms that can’t cross-compile to themselves.
To fix this, move isStatic from stdenv.hostPlatform to stdenv. This
avoids the need to have stdenv.hostPlatform != stdenv.buildPlatform.
Darwin breaks when you pass in -static. Note, this shouldn’t be set on
non-static executable situations, but we don’t have an
isStaticExecutables yet.
@matthewbauer
Copy link
Member Author

Any objections to merging this & then trying to solve the stdenv.hostPlatform != stdenv.buildPlatform issue later?

@ofborg ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 27, 2021
@Ericson2314
Copy link
Member

Ehh we still have the precedent of stdenv.isCross which I don't like. I am taking a stab at this, starting with

--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -8673,7 +8673,7 @@ in
   }));

   crossLibcStdenv = overrideCC stdenv
-    (if stdenv.hostPlatform.useLLVM or false
+    (if with stdenv.hostPlatform; isDarwin || useLLVM or false
      then buildPackages.llvmPackages_8.lldClangNoLibc
      else buildPackages.gccCrossStageStatic);

(

@Ericson2314
Copy link
Member

Started in #110914.

@stale
Copy link

stale bot commented Jul 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 26, 2021
@Artturin Artturin marked this pull request as draft February 19, 2023 02:19
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Feb 19, 2023
@Artturin
Copy link
Member

42b5817 added isStatic to darwin too.

@Artturin Artturin closed this Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: haskell 6.topic: python 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants