fuzzy patch application

fuzzy patch application

[Nick Desaulniers]

I frequently need to backport patches from the Linux kernel to older
kernel versions (Android Security).  My usual workflow for simple
patches is:

1. try `git am patch.txt`.
2. if that fails try `git apply -v patch.txt` then add commit message
manually.
3. if that fails try `patch -p1 < patch.txt` then add commit message
manually.
4. if that fails, manually fix bug based on patch.

My question is, why does `patch` seem to do a better job at applying
patches than `git am`?  It's almost like the `git` tools don't try to fuzz
the offsets.  Is there a way to tell git to try fuzzing offsets when
applying patches?

From the output of `patch` I ran recently (for a patch that
`git apply` would not apply):

patching file <filename.c>
Hunk #1 succeeded at 113 (offset -1 lines).
Hunk #2 succeeded at 435 (offset -3 lines).
Hunk #3 succeeded at 693 (offset 2 lines).
Hunk #4 succeeded at 1535 (offset -41 lines).
Hunk #5 succeeded at 1551 (offset -41 lines).
Hunk #6 succeeded at 1574 with fuzz 2 (offset -42 lines).
Hunk #7 succeeded at 1614 (offset -42 lines).

In fact, `git apply -v` errors for hunk #6.

I would guess maybe that there's an upper limit on the offset searched?
Also, I'm not sure what the `fuzz 2` part means exactly, but it seems like
`git apply` chokes when fuzzing is needed to properly apply a patch.

http://stackoverflow.com/q/6336440/1027966

-- 
Thanks,
~Nick Desaulniers

Re: fuzzy patch application

[Junio C Hamano]

Nick Desaulniers <ndesaulniers@google.com> writes:

> I frequently need to backport patches from the Linux kernel to older
> kernel versions (Android Security)....
> ...
> My question is, why does `patch` seem to do a better job at applying
> patches than `git am`?  It's almost like the `git` tools don't try to fuzz
> the offsets.

You diagnosed correctly.  We do allow offsets but by default no fuzz
and that is a deliberate design decision made in very early days.
You can pass option to reduce context, but that is not the default.

Re: fuzzy patch application

[Jeff King]

On Fri, Feb 10, 2017 at 11:20:59AM -0800, Nick Desaulniers wrote:

> I frequently need to backport patches from the Linux kernel to older
> kernel versions (Android Security).  My usual workflow for simple
> patches is:
> 
> 1. try `git am patch.txt`.

This is not exactly an answer to your question, but "git am -3" is often
a better solution than trying to fuzz patches. It assumes the patches
are Git patches (and record their origin blobs), and that you have that
blob (which should be true if the patches are based on the normal kernel
history, and you just fetch that history into your repository).

I've found that this often manages to apply patches that "git apply"
will not by itself. And I also find the resulting conflicts to be much
easier to deal with than patch's ".rej" files.

-Peff

Re: fuzzy patch application

[Stefan Beller]

On Fri, Feb 10, 2017 at 12:57 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 10, 2017 at 11:20:59AM -0800, Nick Desaulniers wrote:
>
>> I frequently need to backport patches from the Linux kernel to older
>> kernel versions (Android Security).  My usual workflow for simple
>> patches is:
>>
>> 1. try `git am patch.txt`.
>
> This is not exactly an answer to your question, but "git am -3" is often
> a better solution than trying to fuzz patches. It assumes the patches
> are Git patches (and record their origin blobs), and that you have that
> blob (which should be true if the patches are based on the normal kernel
> history, and you just fetch that history into your repository).
>
> I've found that this often manages to apply patches that "git apply"
> will not by itself. And I also find the resulting conflicts to be much
> easier to deal with than patch's ".rej" files.
>
> -Peff

I have been told this a couple of times before; do we want to make -3
the default (in 2.13 then) ?

Re: fuzzy patch application

[Jeff King]

On Fri, Feb 10, 2017 at 01:37:12PM -0800, Stefan Beller wrote:

> > This is not exactly an answer to your question, but "git am -3" is often
> > a better solution than trying to fuzz patches. It assumes the patches
> > are Git patches (and record their origin blobs), and that you have that
> > blob (which should be true if the patches are based on the normal kernel
> > history, and you just fetch that history into your repository).
> >
> > I've found that this often manages to apply patches that "git apply"
> > will not by itself. And I also find the resulting conflicts to be much
> > easier to deal with than patch's ".rej" files.
> 
> I have been told this a couple of times before; do we want to make -3
> the default (in 2.13 then) ?

I dunno. I always use it, but I'm not sure if there are any downsides,
aside from a little extra processing time. It does have some
incompatibilities with other options. And I think it kicks in rename
detection (but I might be mis-remembering another feature). That could
be surprising, I guess.

The original dates all the way back to 47f0b6d5d (Fall back to three-way
merge when applying a patch., 2005-10-06), but I don't see any rationale
for not making it the default. Junio probably could give a better
answer.

-Peff

Re: fuzzy patch application

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I dunno. I always use it, but I'm not sure if there are any downsides,
> aside from a little extra processing time. It does have some
> incompatibilities with other options. And I think it kicks in rename
> detection (but I might be mis-remembering another feature). That could
> be surprising, I guess.
>
> The original dates all the way back to 47f0b6d5d (Fall back to three-way
> merge when applying a patch., 2005-10-06), but I don't see any rationale
> for not making it the default. Junio probably could give a better
> answer.

Nothing deep.  Just being cautious by not to enable extra frills by
default, making it strictly o-p-t i-n.  As that was a strict fall
back (i.e. there was no "if we are going to fall back, we need to
spend extra cycles to do this extra preparation before we attempt
the regular application"), extra-processing cost was not a concern,
at least back I wrote it the first time.  I do not offhand know if
that still holds in the current one that was rewritten in C.

Making "am -3" default may also scare users who are not exactly
comfortable with reading "git diff" output during a conflicted merge
and resolving a conflict, but other than that there shouldn't be any
downside.

Re: fuzzy patch application

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Making "am -3" default may also scare users who are not exactly
> comfortable with reading "git diff" output during a conflicted merge
> and resolving a conflict, but other than that there shouldn't be any
> downside.

Another obvious downside is that there are those of us who prefer to
run "am" without "-3" first to notice that the patch we expect to
apply cleanly does apply cleanly, which gives us a chance to catch
mistakes.  I personally feel that as long as there is a configuration
that makes -3 a personal default (with --no-3way override from the
command line), it is a better design not to enable "-3" by default
for everybody.  New people can first learn using both forms and then
after they got comfortable with resolving merges, they can choose to
flip the default for themselves.

Re: fuzzy patch application

[Nick Desaulniers]

It's not my call about the defaults, but users can be surprised by
such changes.

For the dangers related to fuzzing, is there more info?  I found
and old post on this from Linus calling fuzzing dangerous but
from what I could tell about my patch that wouldn't apply
without fuzzing, the only difference in bad hunks was
whitespace that had diverged somehow.

On Fri, Feb 10, 2017 at 2:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Making "am -3" default may also scare users who are not exactly
>> comfortable with reading "git diff" output during a conflicted merge
>> and resolving a conflict, but other than that there shouldn't be any
>> downside.
>
> Another obvious downside is that there are those of us who prefer to
> run "am" without "-3" first to notice that the patch we expect to
> apply cleanly does apply cleanly, which gives us a chance to catch
> mistakes.  I personally feel that as long as there is a configuration
> that makes -3 a personal default (with --no-3way override from the
> command line), it is a better design not to enable "-3" by default
> for everybody.  New people can first learn using both forms and then
> after they got comfortable with resolving merges, they can choose to
> flip the default for themselves.



-- 
Thanks,
~Nick Desaulniers

Re: fuzzy patch application

[Junio C Hamano]

Nick Desaulniers <ndesaulniers@google.com> writes:

> For the dangers related to fuzzing, is there more info?  I found
> and old post on this from Linus calling fuzzing dangerous but
> from what I could tell about my patch that wouldn't apply
> without fuzzing, the only difference in bad hunks was
> whitespace that had diverged somehow.

If the "old post" is the one he explains why he chose not to allow
fuzz by default, I think you got all what you need.  Basically, he
wanted you and his users to make sure that the patch they are having
trouble with applying can be due to only insignificant difference
and it is safe to apply with reduced context, instead of blindly
accepting a fuzzy patch application.