Re: [Quilt-dev] 'quilt refresh' eats valid changelog text

[Andreas Grünbacher]

Am Do., 13. Mai 2021 um 13:26 Uhr schrieb Greg KH <greg@kroah.com>:
> On Thu, May 13, 2021 at 01:18:50PM +0200, Andreas Grünbacher wrote:
> > Am Do., 13. Mai 2021 um 12:30 Uhr schrieb Greg KH <greg@kroah.com>:
> > >
> > > Hi,
> > >
> > > I just got a report where quilt "ate" the changelog text of a commit
> > > that seems to be valid to me.
> > >
> > > If you look at the Linux kernel commit 6a4db2a60306 ("md: md_open
> > > returns -EBUSY when entering racing area"), it has in the body this
> > > text (indented here to show more obviously):
> > >
> > >         For more detail, please refer with Christoph's "split mddev_find" 
> > > patch
> > >         in later commits.
> > >
> > >         *** env ***
> > >         kvm-qemu VM 2C1G with 2 iscsi luns
> > >         kernel should be non-preempt
> > >
> > >         *** script ***
> > >
> > >         about trigger every time with below script
> > >
> > > and it goes on.
> > >
> > > But when imported into quilt, everything after, and including the line
> > > "*** env ***" is cut off in the changelog header.
> > >
> > > I tried to debug this myself, but I really don't understand the awk
> > > regex in 'refresh.in' and why this is needed:
> > >
> > >         /^#? .* \|  *[1-9][0-9]* /  { eat = eat $0 "\n"
> > >
> > > Nor how that triggers the above.
> > >
> > > Any hints on how to resolve this?
> >
> > This is a problem with function patch_header() which treats a "***" as
> > the start of a context diff, like "---" indicates the start of a
> > unified diff. The attached patch should fix the immediate problem, but
> > function patch_body() will need a similar treatment for a proper fix.
> >
> > Andreas
>
> > diff --git a/quilt/scripts/patchfns.in b/quilt/scripts/patchfns.in
> > index c9923e3..196d26d 100644
> > --- a/quilt/scripts/patchfns.in
> > +++ b/quilt/scripts/patchfns.in
> > @@ -841,9 +841,32 @@ cat_to_new_file()
> >  patch_header()
> >  {
> >       awk '
> > -     /^(---|\*\*\*|Index:)[ \t][^ \t]|^diff -/ \
> > -             { exit }
> > -             { print }
> > +     !(MAYBE_CONTEXT || MAYBE_UNIFIED) {
> > +             if (/^\*\*\*[ \t][^ \t]/) {
> > +                     eaten=$0
> > +                     MAYBE_CONTEXT=1
> > +                     next
> > +             }
> > +             if (/^---[ \t][^ \t]/) {
> > +                     eaten=$0
> > +                     MAYBE_UNIFIED=1
> > +                     next
> > +             }
> > +     }
> > +     MAYBE_CONTEXT {
> > +             if (/^---[ \t][^ \t]/)
> > +                     exit
> > +             print eaten
> > +             MAYBE_CONTEXT=0
> > +     }
> > +     MAYBE_UNIFIED {
> > +             if (/^+++[ \t][^ \t]/)
> > +                     exit
> > +             print eaten
> > +             MAYBE_UNIFIED=0
> > +     }
> > +     /^Index:[ \t][^ \t]/ { exit }
> > +     { print }
> >       '
> >  }
> >
>
> Ah nice, I totally missed this regex, sorry about that.
>
> And does anyone still use "non-unified" patches that this needs to be
> handled?

It happens for some obscure distro patches. But note that unified
format has the same problem with "---" lines that context format has
with "***" lines, so we should really handle this better than we do
now.

Andreas