applyPatches: skip creating additional drv when no patches

[amarshall]

Description of changes

If there are no patches or postPatch, then this will just do work to make something equivalent to src. Instead, just return src in that case.

There are a few instances in nixpkgs where the patches to applyPatches is dynamic, and may be empty, so this optimizes those. It also removes the want to remove applyPatches as an optimization just because the patches are empty when it may semantically make sense to keep it (because e.g. patches to mkDerivation doesn’t work as expected).

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[SuperSandro2000]

Having to change this is probably a disadvantage.

[amarshall]

I actually think this is better because it doesn’t couple to the implementation of name in src. FWIW there are already 80+ instances matching the regex sourceRoot = .*src\.name in Nix files in nixpkgs, and this was the only instance of source-patched in all of nixpkgs.

[SuperSandro2000]

I think the main problem I have with this PR is the unintended name change when no patches are supplied

[amarshall]

Could you elaborate a bit on the concern? Is it that it may cause unforseen breakage upon merge? Confuse future users? Something else?

Tbh, I would be for a tree-wide (plus doc) change from sourceRoot = "source/…" → sourceRoot = "${src.name}/…" because, tbh, I was flummoxed for a while upon initially using applyPatches why the sourceRoot wasn’t working anymore. If it had always been used ${src.name} instead of source it would have just worked. Alternatively could change the name in applyPatches… I don’t really have a strong opinion either way.

[SuperSandro2000]

Is it that it may cause unforseen breakage upon merge? Confuse future users?

yes and yes

---

I don't, please ask in nixos-dev on matrix

[amarshall]

So looks like sourceRoot = "${src.name}…" is now the preferred way #245388

[SuperSandro2000]

Can you also check mastodon and proxysql?

@ofborg build mastodon proxysql

[amarshall]

@SuperSandro2000 I rebased off latest master, resolved conflicts, built mastodon and proxysql successfully (locally, ofborg seemed to never build them, sat queued forever), and compared them both with diffoscope vs. master builds and the only differences were store path hashes.

[oxij]

I also noticed this and wanted to remove it in bitwarden, so this would be nice.

[RaitoBezarius]

@amarshall Apparently, there's some conflicts (?).

[amarshall]

@RaitoBezarius rebased and resolved conflicts—was trivial formatting. Also:

• Verified build of random pkgs that use applyPatches (mastodon,proxysql,zammad)
• Confirmed using applyPatches with patches = [ ] on hello results in identical drv and no rebuild
• Minor formatting change
• Added back explanatory comment that was requested in an earlier review and I think got lost during a previous conflict resolution