[RFC,1/1] support/scripts/apply-patches.sh: set the maximum fuzz factor to 1
diff mbox series

Message ID 20240502090939.748610-1-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • [RFC,1/1] support/scripts/apply-patches.sh: set the maximum fuzz factor to 1
Related show

Commit Message

Dario Binacchi May 2, 2024, 9:09 a.m. UTC 
This patch was created as a fix to a problem that occurred during the
compilation of QEMU:

>>> qemu 8.1.1 Patching

Applying 0001-tests-fp-disable-fp-bench-build-by-default.patch using patch:
patching file tests/fp/meson.build
Hunk #1 succeeded at 138 with fuzz 2 (offset -502 lines).

Applying 0002-softmmu-qemu-seccomp.c-add-missing-header-for-CLONE_.patch using patch:
patching file softmmu/qemu-seccomp.c

Applying 0004-tracing-install-trace-events-file-only-if-necessary.patch using patch:
patching file trace/meson.build

With the bump to version 8.1.1, the patch that disabled the compilation
of the fp-bench test does not report any errors, even though the patch
itself is no longer applicable. The only noticeable message is:

"Hunk #1 succeeded at 138 with fuzz 2 (offset -502 lines)."

As reported by the patch man page:

"With context diffs, and to a lesser extent with normal diffs, patch can
detect when the line numbers mentioned in the patch are incorrect, and
attempts to find the correct place to apply each hunk of the patch.
As a first guess, it takes the line number mentioned for the hunk, plus
or minus any offset used in applying the previous hunk. If that is not
the correct place, patch scans both forwards and backwards for a set of
lines matching the context given in the hunk. First patch looks for a
place where all lines of the context match. If no such place is found,
and it's a context diff, and the maximum fuzz factor is set to 1 or more,
then another scan takes place ignoring the first and last line of
context. If that fails, and the maximum fuzz factor is set to 2 or more,
the first two and last two lines of context are ignored, and another
scan is made. The default maximum fuzz factor is 2.

If the hunk is installed at a different line from the line number
specified in the diff, you are told the offset. A single large offset
may indicate that a hunk was installed in the wrong place. You are also
told if a fuzz factor was used to make the match, in which case you
should also be slightly suspicious."

By setting the maximum fuzz factor to 1, we reduce the possibility that
patches which cannot be applied are incorrectly reported as valid.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---
 support/scripts/apply-patches.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Yann E. MORIN May 5, 2024, 3:16 p.m. UTC | #1 
Dario, All,

On 2024-05-02 11:09 +0200, Dario Binacchi spake thusly:
[--SNIP--]
> By setting the maximum fuzz factor to 1, we reduce the possibility that
> patches which cannot be applied are incorrectly reported as valid.

I think it is a good idea overall.

Did you assess how many of our patches thus no longer apply with this
change?

Note that, except for the huge amount it would require, I would pretty
much be in favour of being even stricter, and refuse any fuzz at all,
thus effectively requiring that patches actually be properly refreshed
for a version bump.

Regards,
Yann E. MORIN.

> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>  support/scripts/apply-patches.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 6da83f6826e9..999384cbbd5b 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -114,7 +114,7 @@ function apply_patch {
>          exit 1
>      fi
>      echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
> -    ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
> +    ${uncomp} "${path}/$patch" | patch -F1 -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
>      if [ $? != 0 ] ; then
>          echo "Patch failed!  Please fix ${patch}!"
>          exit 1
> -- 
> 2.43.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Dario Binacchi May 7, 2024, 7:05 p.m. UTC | #2 
Hi Yann,

On Sun, May 5, 2024 at 5:16 PM Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
>
> Dario, All,
>
> On 2024-05-02 11:09 +0200, Dario Binacchi spake thusly:
> [--SNIP--]
> > By setting the maximum fuzz factor to 1, we reduce the possibility that
> > patches which cannot be applied are incorrectly reported as valid.
>
> I think it is a good idea overall.
>
> Did you assess how many of our patches thus no longer apply with this
> change?

No, but I have started testing about it.

>
> Note that, except for the huge amount it would require, I would pretty
> much be in favour of being even stricter, and refuse any fuzz at all,
> thus effectively requiring that patches actually be properly refreshed
> for a version bump.

I agree with you. I had also thought about it, but I didn't want to make a too
aggressive patch for the first version.

I am running tests with -F0 and -F1 to see the number of patches that
fail in both cases. I am using the `legal-info` target, which stops at applying
 patches without compiling the packages. It will take me some time, but as
soon as I have the results, I will update you.

Thanks and regards,
Dario

>
> Regards,
> Yann E. MORIN.
>
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > ---
> >  support/scripts/apply-patches.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> > index 6da83f6826e9..999384cbbd5b 100755
> > --- a/support/scripts/apply-patches.sh
> > +++ b/support/scripts/apply-patches.sh
> > @@ -114,7 +114,7 @@ function apply_patch {
> >          exit 1
> >      fi
> >      echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
> > -    ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
> > +    ${uncomp} "${path}/$patch" | patch -F1 -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
> >      if [ $? != 0 ] ; then
> >          echo "Patch failed!  Please fix ${patch}!"
> >          exit 1
> > --
> > 2.43.0
> >
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'

Patch
diff mbox series 

diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
index 6da83f6826e9..999384cbbd5b 100755
--- a/support/scripts/apply-patches.sh
+++ b/support/scripts/apply-patches.sh
@@ -114,7 +114,7 @@  function apply_patch {
         exit 1
     fi
     echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
-    ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
+    ${uncomp} "${path}/$patch" | patch -F1 -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
     if [ $? != 0 ] ; then
         echo "Patch failed!  Please fix ${patch}!"
         exit 1