I've upstreamed the icon change but they only release every six months. Once the new version is released I'll remove the patch from nixpkgs, but in the meantime it would be nice to keep it.

Can you add the commit you upstreamed as a patch ... And remove it from the tree?

Good idea, it didn't even occur to me. Updated.

Hash mismatch

 wanted: sha256:09ip3ppb81szn85l32zh3xiyp9kblcz8d0ngw83ilyr7p796m402
  got:    sha256:1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0

Note that fetchpatch normalizes the generated patch, so if you for example retrieved the hash from
nix-prefetch-url it won't be correct.

Note that fetchpatch normalizes the generated patch, so if you for example retrieved the hash from
nix-prefetch-url it won't be correct.

I did use nix-prefetch-url to retrieve the hash.

The build succeeds for me with either 09ip3ppb81szn85l32zh3xiyp9kblcz8d0ngw83ilyr7p796m402 or 1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0 set in fetchpatch. Not sure what's going on there. Other values do fail for me with a hash mismatch.

I guess I should use 1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0? Not sure why you're getting different results (or why both hashes work for me).

Finally got a chance to look at this. The tl;dr is that I updated the hash to 1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0 and nix-review is now happy.

More context:

nix-build was happy with either hash. In fact, I've been building my system packages from a local branch with the "bad" hash for the past few days and it's been working fine. nix-build does complain if I change the hash to some other value.

nix-review, on the other other hand, would only work with 1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0. Not sure what it's doing differently to nix-build; I'll have to look into that.

Compounding my testing was the fact that nix-review wip would fail to build the package every time with a "patch failed to apply" error. The only way I could get it to build was to commit my changes and use nix-review rev. Again, something I need to figure out as it took a while for me to realise that the error had nothing to do with the patch.

For fixed-output derivations if the hash already matches a path in the store nix will skip the download and just use the existing path. Since you used nix-prefetch-url and used that hash I didn't get downloaded because the path already existed.

So changing the hash so that it no longer matches forces a rebuild, and then you get the mismatch because it wasn't correct.

I tend to do Trust on first use where you use a placeholder hash that for sure will be wrong

sha256 = "0000000000000000000000000000000000000000000000000000";

and then when the build fails with a hash mismatch you then substitute it with that hash
and assume it's correct.

I've done this locally and the hash used is correct

hash mismatch in fixed-output derivation '/nix/store/7spb1nsg7dj2bji2bgpai806vy4bmja4-2aa1166087e44e6585b26cedf1fe81b3f3ba547f.patch':
  wanted: sha256:0000000000000000000000000000000000000000000000000000
  got:    sha256:1jqrdv4wy18shg1fdabdb421232hg5micphkixcyzxd1lrmvadg0

more info at https://nixos.org/nixos/nix-pills/nix-store-paths.html

Thank you @lightbulbjim for fixing this.

If you have more questions about the hash issue, feel free to ask me.
There's also lots of people in #nixos (freenode) who'd also enjoy explaining this.

Thanks for merging and especially for the help. I appreciate it!