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

[Jean Delvare]

On Tue, 20 Feb 2007 09:21:50 +0100, Jean Delvare wrote:
> I expect some more fixes to be required though. For example I suspect
> we don't handle "quilt import -f" properly if the original patch was
> reverted and the new one isn't, and vice-versa. Pretty much the same
> problem as with the strip level, discussed here:
> http://lists.gnu.org/archive/html/quilt-dev/2007-01/msg00011.html
> 
> I also think that "-R" or "-p" appearing in a comment after a patch
> name in the series.conf file would be the victim of side effects when
> we update the patch arguments. This case isn't covered by the test
> suite as far as I can tell.

I've extended the test suite and had to improve change_db_strip_level
to preserve all the comments in the series file. It seems to work as
intended, however the awk code in change_db_strip_level has grown quite
a bit. I have no idea how to make it smaller, I'm not even sure it's
possible, given that we need to:
* Update the -p argument, which in practice might mean removing it or
  inserting it
* Remove the -R argument if present
* Preserve the trailing comment if present
So unless someone sees a better way to achieve all that, I'm going to
apply the following patch, most probably tomorrow morning (+0100). Then
I'll try to fix the bug reported by Brandon Philips, as odds are, the
posted patch will probably need to be adjusted to the changes I am
doing (or vice-versa.)

Testers are welcome.

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        20 Feb 2007 18:17:39 -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     20 Feb 2007 18:17:39 -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   20 Feb 2007 18:17:40 -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
@@ -176,14 +180,33 @@ change_db_strip_level()
        then
                local tmpfile=$(gen_tempfile)
                awk '
+               function remove_arg(nr, j) {
+                       for (j=nr; j<NF; j++)
+                               $j=$(j+1)
+                       NF--
+               }
                /^'"$(quote_re $patch)"'([ \t]|$)/ \
-                       { for(i=2; i<=NF; i++)
+                       { p_printed=0
+                         for (i=2; i<=NF; i++) {
+                               if ($i ~ /^#/) break
                                if ($i ~ /^-p/) {
-                                       $i="'"$level"'"
-                                       break
+                                       if ("'"$level"'" == "")
+                                               remove_arg(i--)
+                                       else
+                                               $i="'"$level"'"
+                                       p_printed=1
+                                       continue
+                               }
+                               if ($i == "-R") {
+                                       remove_arg(i--)
+                                       continue
                                }
-                         if (i > NF && "'"$level"'" != "")
-                                $i="'"$level"'"
+                         }
+                         if (!p_printed && "'"$level"'" != "") {
+                               for (j=NF; j>i; j--)
+                                       $(j+1)=$j
+                               $i="'"$level"'"
+                         }
                        }
                        { 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    20 Feb 2007 18:17:40 -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,74 @@
        $ 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
+
+       $ cat patches/series
+       > patchR.diff -R
+
+       # test that comments are preserved
+       $ echo "# Test of -R import" > patches/series
+       $ echo "patchR.diff -R # This patch was originally -R -p1" >> 
patches/series
+
+       # refreshing a reverse patch un-reverses it
+       $ quilt refresh
+       > Refreshed patch %{P}patchR.diff
+
+       $ cat patches/series
+       > # Test of -R import
+       > patchR.diff # This patch was originally -R -p1
+
+       $ 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
+
+       $ cat patches/series
+       > patchRp0.diff -p0 -R
+
+       # refreshing a reverse patch un-reverses it
+       $ quilt refresh
+       > Refreshed patch %{P}patchRp0.diff
+
+       $ cat 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