Re: [Quilt-dev] [patch] import -R (reverse)

[Jean Delvare]

Hi Randy,

On Sun, 18 Feb 2007 23:01:55 +0100, Jean Delvare wrote:
> On Sun, 18 Feb 2007 22:50:05 +0100, Jean Delvare wrote:
> > On Sun, 3 Dec 2006 21:39:16 -0800, Randy Dunlap wrote:
> > > 
> > > I needed 'quilt import -R' recently and noticed that it is
> > > listed in the TODO file.
> > 
> > This is an interesting feature I have often been missing myself, it
> > would indeed be nice to implement it.
> > 
> > > This is likely missing something, so please tell me what it is.
> > > It works, but I had to specify -p1 also, instead of it being
> > > used as the normal default.  Anyone know why?
> > 
> > Not sure, maybe because of the issues below.
> 
> In fact, no, it's a bug in the function patch_args that is called by
> "quilt push". It adds the default strip level (-p1) only if no option
> have been set for the patch, rather than if no -p option has been set.
> This is usually the same, except when you add -R, then it breaks as you
> found yourself.
> 
> There are two ways to fix it:
> 1* If import -R is used and no -p value is given, add -p1 to the patch
> options. That's the easiest.
> 2* Actually fix function patch_args to add -p1 to the patch options if
> no -p option is found, rather than if the option string is empty. This
> will be slightly more difficult, but still doable, and cleaner too IMHO.

I took the second approach as it was cleaner. Then I fixed the problems
I had found in your original patch, the code isn't really nice but it
works. And I also had to update change_db_strip_level to remove -R from
the patch args list, as refreshing a patch resets its "reverse" nature.

Here's what I've come up with, which appears to work for me. Can you
please give it a try and confirm it works OK for you too? (CVS patch,
apply with -p0.)

Index: TODO
===================================================================
RCS file: /cvsroot/quilt/quilt/TODO,v
retrieving revision 1.29
diff -u -p -r1.29 TODO
--- TODO        15 Feb 2006 08:54:21 -0000      1.29
+++ TODO        19 Feb 2007 08:53:18 -0000
@@ -90,8 +90,6 @@ quilt import:
        - Add option to replace the currently applied patch with a new
          one, by backing out the topmost patch first.
 
-       - Add an -R option for reverse-applying a patch.
-
 quilt setup:
 
        - Also recognize other uses of tar and patch (with options in the
Index: quilt/import.in
===================================================================
RCS file: /cvsroot/quilt/quilt/quilt/import.in,v
retrieving revision 1.40
diff -u -p -r1.40 import.in
--- quilt/import.in     1 Jun 2006 10:01:21 -0000       1.40
+++ quilt/import.in     19 Feb 2007 08:53:18 -0000
@@ -19,7 +19,7 @@ fi
 
 usage()
 {
-       printf $"Usage: quilt import [-p num] [-P patch] [-f] [-d {o|a|n}] 
patchfile ...\n"
+       printf $"Usage: quilt import [-p num] [-R] [-P patch] [-f] [-d {o|a|n}] 
patchfile ...\n"
        if [ x$1 = x-h ]
        then
                printf $"
@@ -29,6 +29,9 @@ current top patch, and must be pushed af
 -p num
        Number of directory levels to strip when applying (default=1)
 
+-R
+       Apply patch in reverse.
+
 -P patch
        Patch filename to use inside quilt. This option can only be
        used when importing a single patch.
@@ -93,7 +96,7 @@ die()
        exit $status
 }
 
-options=`getopt -o P:d:fp:h -- "$@"`
+options=`getopt -o P:d:fp:Rh -- "$@"`
 
 if [ $? -ne 0 ]
 then
@@ -111,6 +114,9 @@ do
        -p)
                opt_strip=$2
                shift 2 ;;
+       -R)
+               opt_reverse=1
+               shift ;;
        -d)
                case "$2" in
                        o|n|a) opt_desc=$2 ;;
@@ -135,6 +141,15 @@ then
 fi
 
 [ -n "$opt_strip" ] && patch_args="-p$opt_strip"
+if [ -n "$opt_reverse" ]
+then
+       if [ -n "$patch_args" ]
+       then
+               patch_args="$patch_args -R"
+       else
+               patch_args="-R"
+       fi
+fi
 
 for patch_file in "$@"
 do
Index: quilt/scripts/patchfns.in
===================================================================
RCS file: /cvsroot/quilt/quilt/quilt/scripts/patchfns.in,v
retrieving revision 1.21
diff -u -p -r1.21 patchfns.in
--- quilt/scripts/patchfns.in   31 Jan 2007 19:42:51 -0000      1.21
+++ quilt/scripts/patchfns.in   19 Feb 2007 08:53:18 -0000
@@ -136,10 +136,13 @@ patch_args()
                awk '
                        {sub(/(^|[ \t]+)#.*/, "") }
                $1 == "'"$patch"'" \
-                       { if (NF >= 2)
-                               for (i=2; i <= NF; i++)
-                                       print $i
-                         else
+                       { p_printed=0
+                         for (i=2; i <= NF; i++) {
+                               print $i
+                               if ($i ~ /^-p/)
+                                       p_printed=1
+                         }
+                         if (!p_printed)
                                print "-p1" ;
                          exit
                        }
@@ -161,6 +164,7 @@ patch_strip_level()
        echo "1"
 }
 
+# Also remove -R if present.
 change_db_strip_level()
 {
        local level=$1 patch=$2
@@ -184,6 +188,13 @@ change_db_strip_level()
                                }
                          if (i > NF && "'"$level"'" != "")
                                 $i="'"$level"'"
+                         for (i=2; i<=NF; i++)
+                               if ($i == "-R") {
+                                       for (j=i+1; j<=NF; j++)
+                                               $(j-1)=$j
+                                       NF--
+                                       break
+                               }
                        }
                        { print }
                ' $SERIES > $tmpfile
Index: test/import.test
===================================================================
RCS file: /cvsroot/quilt/quilt/test/import.test,v
retrieving revision 1.4
diff -u -p -r1.4 import.test
--- test/import.test    21 Jan 2006 21:44:38 -0000      1.4
+++ test/import.test    19 Feb 2007 08:53:18 -0000
@@ -14,7 +14,27 @@
        $ quilt refresh
        > Refreshed patch %{P}patch1.diff
 
-       $ quilt pop
+       $ quilt new patchR.diff
+       > Patch %{P}patchR.diff is now on top
+
+       $ quilt add f
+       > File f added to patch %{P}patchR.diff
+
+       $ rm -f f
+
+       $ quilt refresh
+       > Refreshed patch %{P}patchR.diff
+
+       $ quilt fork patchRp0.diff
+       > Fork of patch %{P}patchR.diff created as %{P}patchRp0.diff
+
+       $ quilt refresh -p0
+       > Refreshed patch %{P}patchRp0.diff
+
+       $ quilt pop -a
+       > Removing patch patches/patchRp0.diff
+       > Restoring f
+       >
        > Removing patch patches/patch1.diff
        > Removing f
        >
@@ -22,6 +42,8 @@
 
        $ mkdir t
        $ mv patches/patch1.diff t/patch1.diff
+       $ mv patches/patchR.diff t/patchR.diff
+       $ mv patches/patchRp0.diff t/patchRp0.diff
 
        # test importing into an empty series
        $ rm -rf patches/ .pc/
@@ -137,5 +159,69 @@
        $ quilt delete patch1.diff
        > Patch patch1.diff is not in series
 
+       # test importing a reverse patch
+       $ rm -rf patches/ .pc/
+       $ mkdir patches
+       $ quilt import -R t/patchR.diff
+       > Importing patch t/patchR.diff (stored as patches/patchR.diff)
+
+       $ quilt push
+       > Applying patch patches/patchR.diff
+       >~ patching file `?f'?
+       >
+       > Now at patch patches/patchR.diff
+
+       $ cat f
+       > f
+
+       $ grep patchR patches/series
+       > patchR.diff -R
+
+       # refreshing a reverse patch un-reverses it
+       $ quilt refresh
+       > Refreshed patch %{P}patchR.diff
+
+       $ grep patchR patches/series
+       > patchR.diff
+
+       $ touch f
+       $ quilt pop
+       > Removing patch patches/patchR.diff
+       > Removing f
+       >
+       > No patches applied
+
+       # test importing a reverse patch with strip level
+       $ rm -rf patches/ .pc/
+       $ mkdir patches
+       $ quilt import -R -p0 t/patchRp0.diff
+       > Importing patch t/patchRp0.diff (stored as patches/patchRp0.diff)
+
+       $ quilt push
+       > Applying patch patches/patchRp0.diff
+       >~ patching file `?f'?
+       >
+       > Now at patch patches/patchRp0.diff
+
+       $ cat f
+       > f
+
+       $ grep patchR patches/series
+       > patchRp0.diff -p0 -R
+
+       # refreshing a reverse patch un-reverses it
+       $ quilt refresh
+       > Refreshed patch %{P}patchRp0.diff
+
+       $ grep patchR patches/series
+       > patchRp0.diff -p0
+
+       $ touch f
+       $ quilt pop
+       > Removing patch patches/patchRp0.diff
+       > Removing f
+       >
+       > No patches applied
+
        $ cd ..
        $ rm -rf d

-- 
Jean Delvare