Why isn't the convention in nixpkgs that `hash` is overridable too (along with version) so package overrides would actually be useful?

Time and again I find myself needing to override multiple attributes of a nixpkgs package (notably the entire src attribute) when I only need a new version that’s not yet in nixpkgs.

This is bad UX and could be solved by actually making a version override useful by including (at least) the hash and possibly some other attributes to be overridable?

Would it be feasible to agree on an improved packaging/derivation convention to facilitate this?

2 Likes

Would you mind providing a usage example snippet?

It’s because dependency injection, overriding settings (flags), and more use cases like this would all get conflated under .override if you just exposed ver/hash directly using the current facilities. But of course, if you have a better idea, feel free to share and propose it.

Do note though, you don’t have to override the entirety of src in most cases, but yeah, it’s still clunky to have nested overrides.

1 Like

Hey hey,

I’ve also run into this before, and just saw this other post: Isn't this a nix antipattern? . They suggested to maybe pass version as an input, although if multiple hash values need to be updated, passing them all separately would be quite annoying…

A standard could be to add a single easily overridable attrSet like:

pkgMeta = { 
  version = ...;
  hash = ...;
  npmDepsHash = ...;
  ... # other versioning/hash values here
}; 

and then have src pull from there? This could be put under passthru, or passed as an input maybe.

The reason multiple hashes exist is that there isn’t a single src for the more complex package systems that have multiple hashes. Besides, for stuff like npmDepsHash it’s actually already like that, overriding those looks like e.g. (very pseudocode):

{ someNpmPackage }:
someNpmPackage.overrideAttrs (old: {
  # Granted, this probably doesn't work because
  # the deps derivation will depend on `src` in
  # complex ways, but that's my point;
  # overriding just the hash isn't enough because
  # the packages are complex
  npmDepsHash = "hash";

  src = old.src.override {
    url = "somenewsource";
    hash = "hash2";
  };
})

I honestly don’t see the issue with this; yeah, it’s a little extra writing over just placing the attributes in the parent attrset, but this allows e.g. switching to a different fetcher. I don’t think this can be meaningfully improved without making a useability tradeoff.

Of course, some packages are written in ways that make doing overrides like this very hard (e.g. by let-binding everything and then just writing a wrapper), but that’s a per-package issue that can be resolved quite easily, not a high-level design issue.

1 Like

yea it’s not generally terrible to do now, but you can run into some very annoying nested attrs logic (like you say in the comment). For more complicated packages you could add a nested npmPkgMeta, but it would get complicated fast…

It’s probably better to just be more watchful with new complex packages, since finalAttrs and rec can replace let bindings (i think).

1 Like

rec cannot, just forget rec even exists as a language feature.

2 Likes

Side note: I couldn’t find a proper documentation yet that shows how to override the version and hashes of a rust program. Overriding the cargo hash is not so straightforward.

I just tried to do this ~1h ago, and I had to use 3 layers of nested overrideAttrs, and temporarily set outputHashAlgo in order to get the right outputHash from the error message.

I have not tested this, but I suspect using the cargoHash from finalAttrs would be able to fix this.

--- a/pkgs/build-support/rust/build-rust-package/default.nix
+++ b/pkgs/build-support/rust/build-rust-package/default.nix
@@ -127,7 +127,7 @@ lib.extendMkDerivation {
                 ;
               name = cargoDepsName;
               patches = cargoPatches;
-              hash = args.cargoHash;
+              hash = finalAttrs.cargoHash;
             }
             // depsExtraArgs
           );

It would also allow things like cargoPatches to be overrideAttrsed, where in the original change I had to specify patches twice.

2 Likes

At some point it would need documentation because I always struggle to override a Rust program, even after a couple of years of practicing with Nix :slight_smile:

1 Like

I hope the current interface isn’t the interface we stick with. buildRustPackage: use cargoHash from finalAttrs by Prince213 · Pull Request #472287 · NixOS/nixpkgs · GitHub seems exactly like what I wanted. If we are able to get it merged, documenting it would be a good next step. There are some discussions in the PR on what interface we want to stick with.

2 Likes

Thanks all for the contributions, it shows it still is a (mildly) contentious topic… (I was a bit busy the past days, finally managed to come back to this).

To answer @mightyiam: my latest case was beekeeper-studio (for no particular reason really, but unstable was on 5.5.5 and I wanted to try 5.5.7 with a quick overrideAttrs), and sure enough it’s not an absolute minimal example but still pretty simple.

I tested a local mod of the package that just adds hash to come from finalAttrs:

  src = let
    selectSystem = attrs: attrs.${stdenv.hostPlatform.system};
    asset = selectSystem {
      x86_64-linux = "beekeeper-studio_${finalAttrs.version}_amd64.deb";
      aarch64-linux = "beekeeper-studio_${finalAttrs.version}_arm64.deb";
      x86_64-darwin = "Beekeeper-Studio-${finalAttrs.version}-mac.zip";
      aarch64-darwin = "Beekeeper-Studio-${finalAttrs.version}-arm64-mac.zip";
    };
  in
    fetchurl {
      url = "https://github.com/beekeeper-studio/beekeeper-studio/releases/download/v${finalAttrs.version}/${asset}";
      hash =
        finalAttrs.hash or (selectSystem {
          x86_64-linux = "sha256-XV7PXoXA99BwPolg20vUVAAWhs3eBTmgLOEWZrq6Mq4=";
          aarch64-linux = "sha256-lfW7gjes/Kn8zCKh2LNItu4kwSr9dxYGviUjgXFiHzU=";
          x86_64-darwin = "sha256-EGsfHUYAxjgMBye+D8XzPdUqtXIG7X+DpJoViVcYGHU=";
          aarch64-darwin = "sha256-JfxhOS74xQP8MK2nBz3V0VPxHbXF5m9OtRI2m4ZWjk0=";
        });
    };

pretty much similar to what @figsoda has been saying above.

Indeed, IMHO just getting the hash from finalAttrs in addition to version could already solve the clunkyness of just testing a newer version by override for most standard packages, which is basically what I’m advocating for. It just seems such a no-brainer if you consider that getting only version from finalAttrs (in the src of the derivation) is completely useless on its own?

I agree it would be good to have a simpler way to override the version and hash in one go.

Maybe a first step could be a helper function which chains two overrides like this?

(hello.overrideAttrs {
    version = "2.12.1";
    __intentionallyOverridingVersion = true;
}).overrideAttrs (old: {src = old.src.overrideAttrs {
    hash = "sha256-jZkUKv2SV28wsM18tCqNxoCZmLxdYH2Idh9RLibH2yA="; 
};}

Turning it into a function:

let overrideVersion = ({pkg, version, hash}: (pkg.overrideAttrs {
    version = version;
    __intentionallyOverridingVersion = true;
}).overrideAttrs (old: {src = old.src.overrideAttrs {
    hash = hash; 
};}));
in overrideVersion {
    pkg = hello;
    version = "2.12.1";
    hash = "sha256-jZkUKv2SV28wsM18tCqNxoCZmLxdYH2Idh9RLibH2yA=";
}

Also it looks like it’s already a warning to only override the version:

evaluation warning: hello-2.12.2 was overridden with `version` but not `src` at <unknown file>:<unknown line>:<unknown column>.

                    This is most likely not what you want. In order to properly change the version of a package, override
                    both the `version` and `src` attributes:

                    hello.overrideAttrs (oldAttrs: rec {
                      version = "1.0.0";
                      src = pkgs.fetchurl {
                        url = "mirror://gnu/hello/hello-${version}.tar.gz";
                        hash = "...";
                      };
                    })

                    (To silence this warning, set `__intentionallyOverridingVersion = true` in your `overrideAttrs` call.)

A naive idea is to introduce a new attribute that is a doublet of version + hash and encourage people to use that instead; e.g.

  hashedVersion = {
    tag = "...";
    hash = "...";
  };

… and then use this as the source of truth for fetchers and mkDerivation (version can be derived from hashedVersion.tag, for example). We then gradually migrate to hashedVersion (similar to the namepname migration).

2 Likes

A bit tangential, but IIUC, wouldn’t verified-fetches help get rid of hashes entirely? But that may be only in cases where the commit hash is provided, since tags can still change unexpectedly.

fetchGit is very very rarely used.

1 Like