Nixpkgs should standardize on `srcHash` (making version bumps nicer)

Development
1

In a recent post, I opined about how the standard version bump pattern is currently this:

  my-package = super.my-package.overrideAttrs (finalAttrs: prevAttrs: {
    version = "...";
    src = prevAttrs.src.override {
      tag = "v${finalAttrs.version}";
      hash = "...";
    };
  });

We’ve started to standardize on the whole finalAttrs.version thing, which is good — when a package uses that internally, the override simplifies to:

  my-package = super.my-package.overrideAttrs (prevAttrs: {
    version = "...";
    src = prevAttrs.src.override {
      hash = "...";
    };
  });

But! Wouldn’t it be even nicer if we could do this?

  my-package = super.my-package.overrideAttrs {
    version = "...";
    srcHash = "...";
  };

What would that take? In packages that already use the finalAttrs.version pattern, all we would need to do is insert finalAttrs.srcHash or before the hash string. (And update the documentation to reflect this new convention.)

How hard would that be to do? Here’s a script that handles the migration for about 2/3 of the packages in by-name:

See Bash script 
#!/usr/bin/env nix-shell
#!nix-shell -i bash -p bash coreutils ed git ripgrep
nix-instantiate --extra-experimental-features pipe-operators --eval --raw - \
  --arg-from-file usesFinalAttrs <(rg -l '^[^#]*finalAttrs\.version' pkgs/by-name/*/*/package.nix) \
  <<'NIX' | ed -Es | rg --no-context-separator -B1 '\?' | rg -v '^\?$'
{ usesFinalAttrs }:
let
  pkgs = import ./. { overlays = [ ]; };
  inherit (pkgs) lib;

  adHocExclusions = [
    "aapt"
    "caido-cli"
    "checkstyle"
    "eas-cli"
    "fm-go"
    "fop"
    "gitnuro"
    "gitolite"
    "goku"
    "google-app-engine-go-sdk"
    "h2"
    "hakuneko"
    "isabelle"
    "jol"
    "jpmml-evaluator"
    "keycloak-config-cli"
    "kiro-cli-unwrapped"
    "kotlin-interactive-shell"
    "mustang-cli"
    "mvnd"
    "ninjabrain-bot"
    "opendataloader-pdf"
    "pgsql-tools"
    "photonvision"
    "racer"
    "resilio-sync"
    "scmccid"
    "secp256k1-jdk"
    "spectral-language-server"
    "ticktick"
    "unison-ucm"
    "winbox3"
    "wire-desktop"
    "xadrian"
    "zbctl"
  ];

  packagePaths =
    lib.filterAttrs (nm: _: (builtins.tryEval (pkgs.${nm} ? src.hash)).value)
    <| lib.flip builtins.removeAttrs adHocExclusions
    <| builtins.listToAttrs
    <| map (value: {
      name = builtins.elemAt (lib.path.subpath.components value) 3;
      inherit value;
    })
    <| lib.splitString "\n"
    <| lib.removeSuffix "\n"
    <| usesFinalAttrs;

  positions =
    lib.filterAttrs (nm: pos: pos.file == toString ./${packagePaths.${nm}})
    <| builtins.mapAttrs (nm: _: builtins.unsafeGetAttrPos "src" pkgs.${nm})
    <| packagePaths;

  makeEd = name: pos: ''
    e ${pos.file}
    !# %
    ${toString pos.line};+5s/^( *(hash|sha256) = )"/\1finalAttrs.srcHash or "/
    w
  '';
in
lib.concatMapAttrsStringSep "" makeEd positions
NIX

echo "The above packages have not been modified because a hash was not found close"
echo "enough to the definition of src."
echo
echo "If any package names are printed below this point, they should be added to"
echo "the exclusion list."

nix-instantiate --extra-experimental-features pipe-operators --eval --raw - \
  --arg-from-file changedFiles <(git status -uno --porcelain=v1 | cut -c4-) \
  <<'NIX'
{ changedFiles }:
let
  pkgs = import ./. { overlays = [ ]; };
  inherit (pkgs) lib;

  packageNames =
    map (value: builtins.elemAt (lib.path.subpath.components value) 3)
    <| lib.splitString "\n"
    <| lib.removeSuffix "\n"
    <| changedFiles;

  ignoresSrcHash = name:
    let
      pkg = pkgs.${name};
    in
    pkg.src.outPath == (pkg.overrideAttrs { srcHash = lib.fakeHash; }).src.outPath;
in
lib.concatStringsSep "\n" <| builtins.filter ignoresSrcHash packageNames
NIX

This wouldn’t work for every package, just like the finalAttrs.version trick doesn’t work for every package. In particular, some packages have multiple src-like things and may therefore need multiple srcHash-like attributes, if it even makes sense for them to adopt this pattern. But all it would take to make this version bump pattern nicer for many packages is the will to do it.

Is it a little weird to have an attribute on derivations that Nixpkgs only reads but never defines? Yeah, but does it really matter? Does that outweigh the benefit of telling users that they can version bump many packages without having to ceremonially pass a lambda to overrideAttrs and separately override the src?

Why shouldn’t we do this?

10 Likes
2

Sounds like a decent idea. The necessary incantations to version bump a package via override have always seemed a bit overly complicated to me, too. This is a better idea than any I’ve seen before about how to fix it.

3 Likes
3

I like the idea, another alternatives (which I think has been discussed before) is to add a srcHash attribute in the derivation function that could be modified via override.

# whatever/package.nix
{
  stdenv,
  fetchFromGitHub,
  # could be overriden via package.override { srcHash = "…"; }
  srcHash ? "sha256-…",
}:

Regardless of the how, I agree that this needs to be addressed. Some hash overrides are very annoying to do, and this would go a long way to help improve it.

4 
(whatever.override {
  srcHash = "...";
}).overrideAttrs {
  version = "...";
}

Also doesn’t fill me with pride, honestly. We could also stuff version in there, though, and then it’d be comparable from the user’s perspective.

We don’t have a very good story to tell, I don’t think, about the difference between package file arguments and derivation attributes; but if I were to try to confabulate one (or possibly regurgitate one that I half-remember reading courtesy of someone else; not trying to steal anyone’s credit but my memory does this sometimes), it’d be something like ‘package file arguments are for providing dependencies and feature selectors/flags, and derivation attributes are for the specification of and implementation details of this derivation’. Version bumps feel like they more naturally belong in the latter category. But this ‘rule’ gets violated all over the place, so ehhh?

2 Likes
5

Even though I would appreciate a simplification of the interface, at the same time, I do not really like this idea.

It implies that every way to specify a src would also need a hash. Even though that would be true within the nixpkgs “bubble”, it wouldn’t be outside of it.

It still wouldn’t solve how to version bump wrapped packages.

In my opinion, even the current pattern is a misdirection. It hides the fact that overriding src has always been the relevant change. Bumping the version has always been “cosmetic”.

13 Likes