fetchpatch2 allows lengths of commit hashes to change

[NiklasGollenstede]

Describe the bug

The point of fetchpatch and fetchpatch2 is to prevent changes in API-generated patches that don't affect the patched tree from affecting the hash of the fetched patch.

GitHub dynamically chooses the length of abbreviated commit hashes in a number of places, and (I guess) also in the generated patches.

Some time around August, a commit hash in the following patch changed from 5d7ffe2d91910 to 5d7ffe2d91910a.

/* was: */ (pkgs.fetchpatch2 { url = "https://github.com/NixOS/nixpkgs/commit/eeff6c493373d3fff11421b55309fab6a1d4ec7d.diff"; sha256 = "sha256-Sqt1FQG+0j9dJMtZiBX+dK3UW5SH5Gtg8zVFTs0F23s="; })
/* now: */ (pkgs.fetchpatch2 { url = "https://github.com/NixOS/nixpkgs/commit/eeff6c493373d3fff11421b55309fab6a1d4ec7d.diff"; sha256 = "sha256-Z5vENYxMNBmTUuhzug9Kgi7/21J1X6d7BxqovyOv9fg="; })

Old patch output

diff --git a/pkgs/os-specific/linux/systemd/default.nix b/pkgs/os-specific/linux/systemd/default.nix
index 5d7ffe2d91910..14d5e29c372a88 100644
--- a/pkgs/os-specific/linux/systemd/default.nix
+++ b/pkgs/os-specific/linux/systemd/default.nix
@@ -178,6 +178,13 @@
     # need (AFAICT).
     # See https://github.com/systemd/systemd/pull/20479 for upstream discussion.
     ./0019-core-handle-lookup-paths-being-symlinks.patch
+
+    # fixes reproducability of dbus xml files
+    # Should no longer be necessary with v251.
+    (fetchpatch {
+      url = "https://github.com/systemd/systemd/pull/22174.patch";
+      sha256 = "sha256-RVhxUEUiISgRlIP/AhU+w1VHfDQw2W16cFl2TXXyxno=";
+    })
   ] ++ lib.optional stdenv.hostPlatform.isMusl (
     let
       oe-core = fetchzip {

I still had that file in a local store. The new output is identical, except it uses 5d7ffe2d91910a as the commit hash.

Steps To Reproduce

That's difficult, since GitHub does not provide the old patch file anymore.
I assume the only difference in the patch files was exactly that hash.

Expected behavior

Stable output by fetchpatch and fetchpatch2.

fetchpatch2 is fairly new and hardly used in nixpkgs. Maybe it should be updated to normalize/strip the hashes? Or we get a fetchpatch3 ...

Notify maintainers

@Artturin

[Artturin]

Git auto scales the commit length depending on the amount of commits so that should be the issue

https://github.com/orgs/community/discussions/12531

?full_index=1

[NiklasGollenstede]

Yes. That may very well be the cause for the change in the original generated patch. But regardless of _why_, there are arbitrarily shortened commit hashes in the "normalized" output patch, that, as far as I can tell, have no meaning for the application of the patch. _That_ should be fixed. I am not sure in whose "responsibility" that falls. nixpkgs or patchutils?

--- Or I suppose we could reply on this (community/community#12531 (comment)):

It seems like there is already an undocumented option: `?full_index=1`

But that we should then ... somehow make sure that  nix users/uses actually use (automatically/documentation/?).

[jtojnar]

There is another workaround in anduril/jetpack-nixos@8bf417e#diff-79b35d81aae8a2f44ce607ad52c948baa40a3cd4b6532e6a2af14b708d31f8f7 as mentioned in #266556

[NiklasGollenstede]

Yes, that probably works as well. I'd be inclined to have the fetcher do both, add the URL parameter (pre-fetch) and filter the lines. That way, for the output hah to change, it would take GitHub changing that aspect of the patch generation (e.g. doing that undocumented flag) _and_ us missing something in the filter.