-
Notifications
You must be signed in to change notification settings - Fork 340
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
patch utility misapplies change #533
Comments
On 12/6/24 16:31, Chris Wailes wrote:
Specifically, one of the changes is applied [here]
Fuzz support is misidentifying the location to apply. (Sadly a known
failure of fuzz factor: 5 of your six context lines are identical in the
two locations and you're not removing any.) The default -F value is
"context minus 1" so with 3 leading/trailing context lines that's -F2.
You hit a bug and a design issue. The bug is that -F0 wasn't disabling
fuzz, I just fixed that (commit b1f23f3) and tested that adding -F0
to your command line applied the patch to the file correctly.
The design issue is that toybox patch is streaming: it works like sed
not like vi. It searches through the file for the first place that the
patch applies, and fuzz allows some of the context lines to not match.
All REMOVED lines must always match, and it's got a heuristic where
lines with only whitespace and a single character don't count as context
so it won't fuzz when the context is just curly bracket stacks... but
your hunk has 6 lines of good context, and 5 of them match.
The @from @to line values in unified diffs are advisory, and toybox only
uses them to output offsets from the expected location. Presumably the
other implementation is using the advisory value to prioritize which of
multiple matches to go with, which I can't do without a significant
redesign of the command's plumbing. (It doesn't load in the whole file,
it evaluates and passes through a line at a time, buffering just enough
to evaluate ongoing matches of the hunk it's looking at. It doesn't know
what future matches there might be until it's already evaluated and
written out the earlier lines, so it can't back up and change its mind
about earlier lines it's already processed. Either it the hunk applies
or it doesn't. I could tell it never to apply hunks _before_ the
advisory value, but that's not really an improvement? Offsets are even
more common, fuzz is basically a "near miss" of some other change that
caused an offset. And half of all offsets are negative...)
I've thought about tweaking the fuzz factor heuristics, maybe hunks that
only add lines without removing any should have a lower default fuzz
factor, but that wouldn't have helped you here. You have exactly one
line distinguishing the two locations, without loading the whole file
into the memory and using the @offset values to tie break you can't have
ANY fuzz. (I could also make it so that lines that only add don't get
any default fuzz factor, but half the complaints from people of "the
other one applied it" before I implemented fuzz were add-only hunk
examples. People like to live dangerously...)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The attached patch file does not properly apply to this file. Specifically, one of the changes is applied here. The following arguments were provided to
patch
:-p1 -l -i
.This patch is verified to apply correctly with
GNU patch 2.7.6
.The text was updated successfully, but these errors were encountered: