Skip to content

treewide: replace direct PR references to commits in patches#370724

Closed
PerchunPak wants to merge 1 commit intoNixOS:stagingfrom
PerchunPak:remove-direct-pr-references
Closed

treewide: replace direct PR references to commits in patches#370724
PerchunPak wants to merge 1 commit intoNixOS:stagingfrom
PerchunPak:remove-direct-pr-references

Conversation

@PerchunPak
Copy link
Member

@PerchunPak PerchunPak commented Jan 3, 2025

If PR change, the patch will fail with hash mismatch.

Found using rg 'url ?= ?"https://github.com/.+/pull/\d+.(diff|patch)"'

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:
  • 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.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: crystal Programming language - https://crystal-lang.org/ labels Jan 3, 2025
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this PR did change, I hope this doesn't break anything

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nixpkgs no longer ships anything older 1.2, I remove it then

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash didn't change

$ nix hash to-sri --type sha256 1bwivimpi2hiil3zdnl5qkds1inyn239wgxbn3y8l2pwyppnnfl0
sha256-gDpr7/X8Cor8sKs/noaw3sag28SF2vYHjRGKeGvcka8=

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5c5
< @@ -105,6 +105,15 @@ media_output_dri_init (VADriverContextP ctx)
---
> @@ -105,6 +105,15 @@
21c21
< @@ -112,6 +121,7 @@ media_output_dri_init (VADriverContextP ctx)
---
> @@ -112,6 +121,7 @@

@github-actions github-actions bot added the 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. label Jan 3, 2025
@PerchunPak PerchunPak force-pushed the remove-direct-pr-references branch from d94b43b to c710a3d Compare January 3, 2025 21:35
@github-actions github-actions bot added the 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. label Jan 3, 2025
@PerchunPak
Copy link
Member Author

PerchunPak commented Jan 3, 2025

Built krun, redis, vaapi-intel-hybrid, python311Packages.tpm2-pytss, python312Packages.tpm2-pytss, python313Packages.tpm2-pytss and everything is good (other packages are marked as broken)

The CI fails because of #370456

@lucasew
Copy link
Contributor

lucasew commented Jan 3, 2025

Please add the PR link as comments on the patches for reference

@emilazy
Copy link
Member

emilazy commented Jan 4, 2025

This is not really sufficient because GitHub will GC the old commits. The only truly safe way to vendor active PRs is to vendor the patches in tree.

(fetchpatch2 without ?full_index=1 is also a reproducibility issue because it does not filter out the index … lines that vary with repository size.)

@PerchunPak
Copy link
Member Author

This is not really sufficient because GitHub will GC the old commits. The only truly safe way to vendor active PRs is to vendor the patches in tree.

In that case, the best solution would be to download patches from cache and vendor it. This PR prevents random hash failures if someone does a mass rebuild of fetchpatch*

@PerchunPak
Copy link
Member Author

I gave up on creating full_index treewide, here is my take on this if someone wants to try:

rg -HIl fetchpatch2 | xargs -I{} perl -0777 -i -pe 's/(fetchpatch2(?:.*\n.*)+url ?= ?".*github[^"]*)(")/$1?full_index=1$2/g' {}

False positives are:

  • It doesn't detect if full_index is already present
  • It works on fetchurl

I gave up after seeing those two

@PerchunPak PerchunPak force-pushed the remove-direct-pr-references branch from c710a3d to b812c65 Compare January 4, 2025 10:57
@PerchunPak
Copy link
Member Author

Looks like there is no interest in reviewing this, so I will just close it

@PerchunPak PerchunPak closed this Jan 14, 2025
@PerchunPak PerchunPak deleted the remove-direct-pr-references branch January 14, 2025 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: crystal Programming language - https://crystal-lang.org/ 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants