CONTRIBUTING: allow fetching unaccepted changes if the URL is stable

[marcin-serwin]

Reword documentation to allow using fetchpatch for changes that are not yet merged upstream. Additionally, put a greater emphasis on URL stability.

fixes #400873

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• 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/)
• 25.05 Release Notes (or backporting 24.11 and 25.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.

---

Add a 👍 reaction to pull requests you find important.

[emilazy]

cc @alyssais

The common case (GitHub) does GC such commits, so this will probably nudge people in the wrong direction on average.

[marcin-serwin]

The common case (GitHub) does GC such commits

Hmm, @eclairevoyant claimed otherwise in #400873 (comment). The only official statement from GH I found is this blog post where they state:

Normally, we don’t remove any unreachable data from repositories. But occasionally we do, usually to remove sensitive data, like passwords or SSH keys from your repository’s history.

So I think that GH commits could be considered safe.

[alyssais]

This does not match our experience in practice.

[alyssais]

Arbitrary example: can you look at previous force pushes of #123456? I can't.

[emilazy]

“Nothing up my sleeve” number: #54321. PR from 2019, the first commit in the force‐push event is a 404. Also true of #123456 from 2021, and #234581 from 2023 (had to increment until I found a PR with a force push).

GitHub do routinely run garbage collection; IIRC they state somewhere that it happens every 50 pushes. If they started explicitly retaining force‐pushed PR commits at some point, it would have to have been within the past couple of years, and I haven’t seen anything suggesting it’s changed.

[marcin-serwin]

I see. I still think the current wording is confusing since stable links to unmerged changes are possible. I've changed the wording around garbage collection to be stronger and namedropped GtiHub as an example.

[alyssais]

Much better. If you want, you could include https://lore.kernel.org/ as an example of patches that do have likely-stable URLs.

[emilazy]

Thanks, this looks good. I appreciate the mention that GitHub and other forges shouldn’t be relied upon to act this way.

I do think that “unmerged patches should be vendored” is generally the right heuristic, because the kinds of settings where you do have stable archiving of patches (mailing lists, mostly?) are also the kind where it’s often hard to get a usable direct URL to the patches. The kernel is certainly an exception, though.

[marcin-serwin]

I do think that “unmerged patches should be vendored” is generally the right heuristic

I agree that it's a good heuristic but if we want to keep it should be framed as an advice and not as a rule.

[eclairevoyant]

Hmm, @eclairevoyant claimed otherwise in #400873 (comment).

I was incorrect, I checked some recent PRs and their commits are gone.

In any case the current PR makes the guidelines clearer and more consistent.

[phanirithvij]

- - is already proposed upstream but not merged yet

isn't that the case when we should vendor?

[eclairevoyant]

it can be misleading. people read it to mean "if it's not even proposed upstream don't vendor" even though that's not what it says of course