-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
Make pkgsStatic set "static" argument to true #96223
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-distinguish-pkgsmusl-and-pkgsstatic/8745/5 |
So far, we've just been overriding individual packages in the overlay. I agree this is rather unsystematic, basically taking on the constraints of a downstream project despite living in the Nixpkgs proper. That said, we don't add arbitrary configuration to the top-level like this. We would instead probably want to put something in |
And... the best practice is (generally) as follows?
|
@Ericson2314 What about this, updated version of patch?
This way, to add something specific to static build, change will be only in one place -- in derivation itself, without need to keep list in |
And packages would be written like this? { stdenv, foo, bar
, static ? stdenv.targetPlatform.isStatic
}: (So that they can be configured "individually" as well, though I expect most use cases will be fully static builds anyway.) |
Not sure about this. If you keep So actually I expect it to be used like this:
|
Well, for me... static isn't all-or-nothing and the flag mainly means whether the output for that package will contain |
I do not see reason to not build ".o/.a". Put .so into libs, .a into dev (as it is done in Debian). |
I don't think that works very well in practice. Many build tools assume they both reside in the same directory, it's more difficult for you to choose which is used, etc. EDIT: most notably, |
Let's start with { stdenv, foo, bar
, static ? stdenv.targetPlatform.isStatic
}: As @vcunat says as it's a nice small step that nevertheless solves the problem. |
Am I correct that you want me to remove manual overrides in pkgs/top-level/static.nix and add |
No, sorry, I was just commenting on how we might transition away from having the overlay at all. I didn't realize you had ready pushed new stuff to this. Will review in a moment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a little bit more work left, thanks.
lib/systems/default.nix
Outdated
@@ -76,6 +76,7 @@ rec { | |||
# uname -r | |||
release = null; | |||
}; | |||
isStatic = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good to set a default here, but let's make it a bit more nuanced, based on
nixpkgs/pkgs/stdenv/cross/default.nix
Line 41 in ab24734
++ (if (with crossSystem; isWasm || isRedox) then [(import ../../top-level/static.nix)] else []); |
isStatic = false; | |
isStatic = final.isWasm || final.isRedox; |
pkgs/top-level/static.nix
Outdated
@@ -43,6 +43,10 @@ self: super: let | |||
# ++ optional (super.stdenv.hostPlatform.libc == "glibc") ((flip overrideInStdenv) [ self.stdenv.glibc.static ]) | |||
; | |||
|
|||
exposeStatic = super: super // { | |||
targetPlatform = super.targetPlatform // { isStatic = true; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we shouldn't be overriding *Platform
in overlays, for a number of obscure reasons.
Instead, we should change the definition of pkgsStatic
to include isStatic = true;
in the crossSystem
argument to Nixpkgs.
That, and the pkgs/stdenv/cross/default.nix
line I already pointed out above, and are the only places static.nix
is used. The fancier defaulting suggestion I made ensures that latter is already in sync with the use of the overlay, so changing pkgsCross
like so is the only other fixup needed.
This change allows derivations to distinguish dynamic musl build and static musl build in cases where upstream build system can't detect it by itself.
@Ericson2314 Did changes as you requested, works for me as checked in #96223 (comment) |
Close PR #96012 (thanks). This "static style" was discussed on: #96223 (comment)
Signed-off-by: Arthur Gautier <[email protected]> Amended by vcunat (isMusl != isStatic). #96223 (comment)
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.
This change allows derivations to define "static ? false" arguments to
have way to distinguish dynamic musl build and static musl build in
cases where upstream build system can't detect it by itself.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)