New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
patchutils: various fixes #53449
patchutils: various fixes #53449
Conversation
Since there is no maintainer, based on |
166f1b3
to
0b1831b
Compare
I would have expected way more rebuilds, but I guess I forgot that |
But then again maybe the fetchpatch output changed somehow with the update :/ |
Yes, please check some fetchpatch hashes, as I think I remember we had issues previously. |
Looks like they did actually change |
Apparently the new patches miss these two lines at the top: diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c
index 26dc39a41..c5065bc97 100644 |
This is the offending comment: twaugh/patchutils@14261ad It fixes the header detecton and makes sure extended git headers are not dropped as they were before (apparently by accident). I got it wrong in my previous comment: the new patch does not miss those lines, while the previous ones did. Not sure what to do here. Revert the commit? Never upgrade patchutils? Adjust all hashes in nixpkgs somehow? |
This isn't the first time at all: #25154 (and links) |
Interesting, thanks for the link. I personally don't care very much for the update, so I'd be fine with leaving that out. But then I'll at least add a comment to avoid duplicate work in the future. What is your preference? Keep it at 0.3.3, have both 0.3.3 and 0.3.4 or update to 0.3.4 while reverting that one commit? |
Well, ATM it's probably not worth it to change all the hashes in nixpkgs (I haven't looked at their changes though). Over longer term it might be best to split the version we use for |
|
Right, I agree that even for |
That was a security issue in |
Right, I believe so. I got confused by a debian thread. |
0.3.3 -> 0.3.4 doesn't seem to be relevant for security. |
I just ran into these bugs today, so 👍 to fixing them. Thanks for investigating how to fix it! |
Are there any updates on this pull request, please? |
@timokau What is the state of this PR? It should at least be rebased to current master and squashed. |
IIRC the issue is that it will change all the |
Thank you for your contributions.
|
How about merging the other changes, without the version bump, since some people seem to have run into issues without them? |
@ajs124 I don't plan to work on this any time soon. I'd be happy to review if you want to take this over though. |
I marked this as stale due to inactivity. → More info |
Motivation for this change
Some of the patchutils scripts weren't working because the perl-shebang wasn't generated properly. When that was fixed, they still didn't work because they depended on each other.
This fixes those issues by adding perl to build inputs and wrapping the scripts with
PATH
. While at it, I also update patchutils to the latest version which makes the patch unnecessary and fixed the tests.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)