How to patch in an overlay

I’m trying to patch coreutils with an overlay. I’ve successfully managed to override it in ~/.config/nixpkgs/config.nix but I would like to use the overlays system as recommended by the community. The following has worked in config.nix:

coreutils-progressbar = pkgs.coreutils.overrideAttrs (oldAttrs: rec {
  name = "coreutils-patched";
  patches = oldAttrs.patches ++ [(fetchpatch {
    name = "advcpmv-0.8-8.30.patch";
    url = "https://raw.githubusercontent.com/mrdrogdrog/advcpmv/master/advcpmv-0.8-8.30.patch";
    sha256 = "0mw0ramg4ydqdqs33kw9m0rjvw5fvfa0scsq753pn7biwx6gx9hx";
  })];
  postPatch = oldAttrs.postPatch + ''
    sed '2i echo Skipping usage vs getopt test && exit 77' -i ./tests/misc/usage_vs_getopt.sh
  '';
});

I tried the following overlay expression:

self: super:

{
  coreutils = super.coreutils.overrideAttrs (oldAttrs: rec {
    patches = oldAttrs.patches ++ [
      (self.fetchpatch {
        url = "https://raw.githubusercontent.com/mrdrogdrog/advcpmv/master/advcpmv-0.8-8.30.patch";
        sha256 = "0mw0ramg4ydqdqs33kw9m0rjvw5fvfa0scsq753pn7biwx6gx9hx";
      })
    ];
    postPatch = oldAttrs.postPatch + ''
      sed '2i echo Skipping usage vs getopt test && exit 77' -i ./tests/misc/usage_vs_getopt.sh
    '';
  });
}

But very weirdly, I get the following error:

error: anonymous function at <nixpkgs>/build-support/fetchurl/boot.nix:5:1 called with unexpected argument 'meta', at <nixpkgs>/build-support/fetchpatch/default.nix:14:1

The only meta attribute I can think of in this context is of coreutils itself.

Besides solving this specific issue. I have an extra question regarding overlays in general I’d happy to know the answer to:

Say a package’s current patches attribute is null and not an empty list. In that case, an overlay similar to mine wouldn’t work because oldAttrs.patches is null and so the evaluation will fail on ++. How can one guarantee, using the right expression that even if patches are added in other overlays or in <nixpkgs>, all of them will be applied? The same can be asked and answered for other ‘optional’ attributes such as e.g postPatch.

Thanks again in advance.

coreutils is everywhere. It is even used in fetchpatch. So the easiest would be to replace self.fetchpatch with builtins.fetchurl. I wonder why this doesn’t cause infinite recursion…

Second easy way is to use fetchpatch from reimport of nixpkgs:

let fetchpatch = (import super.path {}).fetchpatch;
in ...

This way you import nixpkgs twice, but oh well.

But I also wonder what is the correct way to override this in overlay.

Say a package’s current patches attribute is null and not an empty list.

patches = (let oldPatches = oldAttrs.patches or []; in if oldPatches == null then [] else oldPatches) ++ [ ... ];

This should be the correct thing. Wonder if this can be simplified.

First of all, thanks for the quick reply! I’ve 3 Questions:

  1. Why revert to builtins.fetchurl? is there no builtins.fetchpatch?
  2. Would it help avoid the recursion if I’d rename the new package as I’ve renamed it when I defined it in config.nix? I guess I’d wish to avoid rebuilds to all of my packages just because of this small patch right?

As for that patches = null workaround you’ve suggested, I asked mostly for the sake of getting an expert’s view on this from a programming point of view so I wonder: Could it be possible to use something simpler like this:

patches = oldAttrs.patches or [] ++ [
  (self.fetchpatch {
    ...
  })
];
]

?

EDIT:

I’ve tried to rename the new coreutils attribute to coreutils-progressbar and it worked. I’m still unsure as for the 1st and last question…

  1. No, there is no builtins.fetchpatch. I’ve used builtins.fetchurl to break the cycle.
  2. Yes, sure! That’s actually the answer why it worked in config.nix and stopped working in overlay.
  3. Yup, this is fine, only wrap in parens: (oldAttrs.patches or []) ++ [.... But, in rare cases there can be null value:
$ rg "patches.*=.*null"
...
pkgs/tools/filesystems/e2fsprogs/default.nix
19:  patches = if stdenv.hostPlatform.libc == "glibc" then null

pkgs/development/tools/misc/help2man/default.nix
16:  patches = if stdenv.hostPlatform.isCygwin then [ ./1.40.4-cygwin-nls.patch ] else null;
...

I guess those are bugs.

So you are saying that if patches is explicitly set to null, as in those cases you’ve found with rg then (oldAttrs.patches or []) wouldn’t work?

yup

$ nix build '(with import ./.{};
   e2fsprogs.overrideAttrs (old: { patches = (old.patches or []) ++ []; }))'
error: value is null while a list was expected, at (string):1:63

Interesting… I guess this is a small issue with the language that could have been workaround if those if ... then statements wouldn’t use null explicitly at all right? In any case, I better understand the intention of your original workaround now.

Thanks a lot for your time @danbst !