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

nixStatic: set derivation name to "nix-static-${version}" #119310

Closed
wants to merge 1 commit into from

Conversation

sternenseemann
Copy link
Member

This is a workaround for the issue a lot of people have been having
where nix-env -u would think that nixStatic was a newer version of nix
and “upgrade” it to that package, breaking setups in the process.

Reference #118481

Motivation for this change
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.

@edolstra
Copy link
Member

Since overrideDerivation is evil, maybe this could be fixed at the source, namely in the nix derivation? I.e. do

name = "nix" + (if enableStatic then "-static" else "");

and somehow prevent the x86_64-unknown-linux-musl part from being appended (which is probably a bug in itself, since that stuff shouldn't be in the version attribute).

@sternenseemann
Copy link
Member Author

Since overrideDerivation is evil, maybe this could be fixed at the source, namely in the nix derivation? I.e. do

name = "nix" + (if enableStatic then "-static" else "");

Hm, I'm not sure. I see this renaming mainly as workaround, I don't think we necessarily need to reflect static compilation in all cases in the derivation name, i. e. I don't see why the derivation name for pkgsStatic.nix should be nix-static-2.3.10-x86_64-unknown-linux-musl as well since we don't do anything similar for any package in that set (as far as I know).

and somehow prevent the x86_64-unknown-linux-musl part from being appended (which is probably a bug in itself, since that stuff shouldn't be in the version attribute).

It is not actually in the version attribute, only in the name attribute because dontAddHostSuffix == false in

name = "${attrs.name or "${attrs.pname}-${attrs.version}"}-${stdenv.hostPlatform.config}";

I'd say this is expected behavior since we do use the cross compilation infrastructure for building pkgsStatic, but it is less than ideal. Maybe we can put the host suffix before the version attribute? (cc @Ericson2314 @matthewbauer) or alternatively somehow make mkDerivation not append the suffix in pkgs/top-level/static.nix.

This is a workaround for the issue a lot of people have been having
where nix-env -u would think that nixStatic was a newer version of nix
and “upgrade” it to that package, breaking setups in the process.

Reference NixOS#118481
@sternenseemann
Copy link
Member Author

Since overrideDerivation is evil, maybe this could be fixed at the source, namely in the nix derivation? I.e. do

The best we can do with overrideAttrs is nix-static-2.3.10-x86_64-unknown-linux-musl. This should be enough as well, right?

sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Apr 13, 2021
This allows stdenv.mkDerivation to append -static to the name for
pkgsStatic.nix / nixStatic which should make nix-env stop thinking
that nixStatic is a newer version of nix.

This change replaces NixOS#119310.
@sternenseemann
Copy link
Member Author

I'd prefer #119317 as a solution to this.

@domenkozar domenkozar closed this Apr 13, 2021
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