Re: [Quilt-dev] [PATCH] handle quilt import -f -p* and update series

[Jean Delvare]

Hi Brandon, Andreas,

On Mon, 12 Feb 2007 21:26:44 -0800, Brandon Philips wrote:
> On 11:42 Wed 31 Jan 2007, Andreas Gruenbacher wrote:
> > On Wednesday 31 January 2007 10:30, Brandon Philips wrote:
> > > Handle the case where the patch that is being replaced with quilt import 
> > > -f
> > > has a different strip level than the one in the series.
> > 
> > this always calls change_db_strip_level. How about the attached version 
> > instead? (Could you please give it a try to see if it breaks something?)
> 
> That patch didn't work for me if the argument had a path.  eg.
> /home/philips/import-fix.diff 
> 
> This new patch fixes all of the issues, hopefully.  :-)

This patch conflicted with the recent implementation of "import -R" by
Randy Dunlap and myself, so I tried to adjust it.

Andreas objected to Brandon's original implementation because it was
calling change_db_strip_level unconditionally. I can't see how this was
true, and Andreas' solution was much more intrusive.

Either way, now that we need to handle the -R argument, things are
different. When using "import -f", we do not only want to conditionally
update the strip level in the series file, we may also need to remove
or add the -R argument. Put in short, we don't exactly need to update
the patch arguments, we need to plain overwrite them with a new set of
arguments. I think that the current set of possible arguments is -p and
-R, but others could be added later. Thus I believe that
change_db_strip_level isn't the right function for what we need to do.
Rather that extending it (it's already quite long) I wrote a new
function, change_db_args, which does exactly this.

I am calling change_db_args unconditionally. In order to avoid this we
would need to compare the -p and -R states of the old and new patches
individually, and only if either needs to be changed, call the
function. We can't just compare the argument strings, as the arguments
might be swapped. This will become increasingly complex if we need to
handle more arguments later, and quite frankly, this doesn't seem to be
worth the effort. I don't expect people to call "import -f" very often,
so I see little point in trying to optimize it.

Here is the patch I have come up with. Comments and testers are welcome.

Index: quilt/import.in
===================================================================
RCS file: /cvsroot/quilt/quilt/quilt/import.in,v
retrieving revision 1.41
diff -u -r1.41 import.in
--- quilt/import.in     21 Feb 2007 20:34:50 -0000      1.41
+++ quilt/import.in     22 Feb 2007 09:59:04 -0000
@@ -193,6 +193,9 @@
                fi
                printf $"Replacing patch %s with new version\n" \
                       "$(print_patch $patch)" >&2
+
+               # Strip level or reverse state might be different
+               change_db_args "$patch" "$patch_args"
        elif [ -e "$dest" ]
        then
                printf $"Importing patch %s\n" "$(print_patch $patch)"
Index: quilt/scripts/patchfns.in
===================================================================
RCS file: /cvsroot/quilt/quilt/quilt/scripts/patchfns.in,v
retrieving revision 1.23
diff -u -r1.23 patchfns.in
--- quilt/scripts/patchfns.in   22 Feb 2007 09:19:25 -0000      1.23
+++ quilt/scripts/patchfns.in   22 Feb 2007 09:59:04 -0000
@@ -222,6 +222,39 @@
        fi
 }
 
+change_db_args()
+{
+       local patch=$1 patch_args=$2
+
+       [ -e $SERIES ] || return 1
+       local tmpfile=$(gen_tempfile)
+       awk '
+       function remove_arg(nr, j) {
+               for (j=nr; j<NF; j++)
+                       $j=$(j+1)
+               NF--
+       }
+       function insert_arg(nr, value, j) {
+               for (j=NF; j>=nr; j--)
+                       $(j+1)=$j
+               $nr=value
+       }
+
+       /^'"$(quote_re $patch)"'([ \t]|$)/ \
+               { while (NF>=2 && $2 !~ /^#/)
+                       remove_arg(2)
+                 if ("'"$patch_args"'" != "")
+                       insert_arg(2, "'"$patch_args"'")
+               }
+               { print }
+       ' $SERIES > $tmpfile
+       if ! cmp $SERIES $tmpfile >/dev/null 2>/dev/null
+       then
+               cat $tmpfile > $SERIES
+       fi
+       rm -f $tmpfile
+}
+
 patch_in_series()
 {
        local patch=$1
Index: test/import.test
===================================================================
RCS file: /cvsroot/quilt/quilt/test/import.test,v
retrieving revision 1.5
diff -u -r1.5 import.test
--- test/import.test    21 Feb 2007 20:34:51 -0000      1.5
+++ test/import.test    22 Feb 2007 09:59:04 -0000
@@ -14,6 +14,12 @@
        $ quilt refresh
        > Refreshed patch %{P}patch1.diff
 
+       $ quilt fork patch1p0.diff
+       > Fork of patch %{P}patch1.diff created as %{P}patch1p0.diff
+
+       $ quilt refresh -p0
+       > Refreshed patch %{P}patch1p0.diff
+
        $ quilt new patchR.diff
        > Patch %{P}patchR.diff is now on top
 
@@ -35,13 +41,14 @@
        > Removing patch patches/patchRp0.diff
        > Restoring f
        >
-       > Removing patch patches/patch1.diff
+       > Removing patch patches/patch1p0.diff
        > Removing f
        >
        > No patches applied
 
        $ mkdir t
        $ mv patches/patch1.diff t/patch1.diff
+       $ mv patches/patch1p0.diff t/patch1p0.diff
        $ mv patches/patchR.diff t/patchR.diff
        $ mv patches/patchRp0.diff t/patchRp0.diff
 
@@ -228,5 +235,43 @@
        >
        > No patches applied
 
+       # test overwiting import with different strip level
+       $ rm -rf patches/ .pc/
+       $ mkdir patches
+       $ quilt import -p0 t/patch1p0.diff -P patch1.diff
+       > Importing patch t/patch1p0.diff (stored as patches/patch1.diff)
+
+       $ cat patches/series
+       > patch1.diff -p0
+
+       $ echo "patch1.diff -p0 # Warning: patch is -p0" > patches/series
+       $ cat patches/series
+       > patch1.diff -p0 # Warning: patch is -p0
+
+       $ quilt import -f t/patch1.diff
+       > Replacing patch patches/patch1.diff with new version
+
+       $ cat patches/series
+       > patch1.diff # Warning: patch is -p0
+
+       # test overwiting import with different options
+       $ rm -rf patches/ .pc/
+       $ mkdir patches
+       $ quilt import -R t/patchR.diff -P patch1.diff
+       > Importing patch t/patchR.diff (stored as patches/patch1.diff)
+
+       $ cat patches/series
+       > patch1.diff -R
+
+       $ echo "patch1.diff -R # Warning: patch is -R" > patches/series
+       $ cat patches/series
+       > patch1.diff -R # Warning: patch is -R
+
+       $ quilt import -f -p0 t/patch1p0.diff -P patch1.diff
+       > Replacing patch patches/patch1.diff with new version
+
+       $ cat patches/series
+       > patch1.diff -p0 # Warning: patch is -R
+
        $ cd ..
        $ rm -rf d


-- 
Jean Delvare