Skip to content

Superfluous use of finalAttrs.{pname,version} introducing overriding traps #310373

@oxalica

Description

@oxalica

Describe the bug

There are many code adopting finalAttrs to reuse variables, rather than provide an overridable option. Specifically, using finalAttrs.version and finalAttrs.pname in src = is incorrect since overriding them will NOT cause it to automatically use the source from new name/version, because the hash literal is still unchanged. It will be a silent incorrect behavior when the FOD is cached locally (reusing the old one), and will be a hash-mismatch error if it's not, both causing a hard-to-debug trap.
There are also usages in build script like cp -r ${finalAttrs.pname}-${finalAttrs.version}/* $out, which is still incorrect for .overideAttrs { pname = "foo-patched"; } use case.

mkDerivation (finalAttrs: {
  version = "1.2.3";
  src = fetchFromGitHub {
    repo = finalAttrs.pname; # so adding a `-patched` suffix would try to use another repo?
    rev = "v${finalAttrs.version}"; # oops, changing version will not fetch the new src, because of unchanged hash.
    hash = "..some literal hash..";
  };
  # So overriding either `pname` or `version` makes it fail to build.
  installPhase = ''
    cp -r ${finalAttrs.pname}-${finalAttrs.version}/* $out
  '';
})

A quick search shows that there are even more incorrect usages than correct usages on the first glance:

Expected usage

Users should always override src manually, thus making a overridable illusion via finalAttrs is more misleading than not supporting it at all.

mkDerivation rec {
  pname = "some-pkg";
  version = "1.2.3";
  src = fetchFromGitHub {
    repo = "some-pkg"; # or `pname`
    rev = "v1.2.3"; # or `"v${version}"`
    hash = "..some literal hash..";
  };
  installPhase = ''
    cp -r some-pkg-*/* $out
    # Or cp -r ${some-pkg}-*/* $out
  '';
}

It seems the even the good old rec is more correct for the pname usages because it's fixed at parse time and will not be overridden anyway, which is the intended behavior for repo and installPhase. 🤔

Inspired by #277994. cc @AndersonTorres


Add a 👍 reaction to issues you find important.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions