[Quilt-dev] RFC: quilt pop and read-only files

[Benjamin Poirier]

Hello,

I believe I've run into some issues with quilt pop and read-only files:
        $ mkdir /tmp/quilt-test && cd /tmp/quilt-test
        $ echo "step 1" > file
        $ chmod a-w file
        $ quilt new demo
        Patch patches/demo is now on top
        $ quilt add file
        File file added to patch patches/demo
        $ echo "step 2" > file
        $ quilt refresh
        Refreshed patch patches/demo
        $ echo "blabla" | quilt header -r
        Replaced header of patch patches/demo
        $ quilt pop
        Patch patches/demo does not remove cleanly (refresh it or enforce with 
-f)

The first problem is here. This message is misleading, in fact we are in the
case "Failed to patch temporary files", the output from patch was:
"File file is read-only; refusing to patch
1 out of 1 hunk ignored -- saving rejects to file file.rej"

Nevertheless, we can proceed with the suggestions:
        $ quilt refresh
        Patch patches/demo is unchanged
        $ quilt pop
        Patch patches/demo does not remove cleanly (refresh it or enforce with 
-f)

One way to back out of this situation is to force the pop:
        $ quilt pop -f
        Removing patch patches/demo
        Restoring file

        No patches applied

That's the second problem, I find that solution somewhat inelegant and
counter-intuitive. Usually, "-f" leaves me with the impression that
changes will be discarded.

In the example above "file" is edited manually but the issue is the same
when working with quilt import/push. The handling of the --force option
to `patch` is reversed between push and pop. By default, `quilt push`
leads to using `patch -f` which can work on read only files whereas
`quilt pop` doesn't pass "-f" to patch in "check_for_pending_changes".

Two conditions are required for the problem to manifest itself:
1) the .timestamp for a patch becomes outdated but the patch body is unchanged
(because of quilt header above).
2) the patch touches a file which was originally read-only.

What does the quilt developer community think about this issue? Is "pop
-f" warranted and accepted or should pop's behavior be altered?

These problems can be cured by the two small patches that follow. I'll
be happy to submit them properly if that proves to be desired.

Subject: [PATCH 1/2] RFC: pop: Correctly report pop failure cause.

Because of the extra condition that was present, in the case where patching
the original file(s) fails, the situation was incorrectly reported as "Patch
[...] does not remove cleanly" whereas it should be "Failed to patch temporary
files".

Anyone knows the rationale for that condition?
---
 quilt/pop.in |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/quilt/pop.in b/quilt/pop.in
index efacf09..8b69f64 100644
--- a/quilt/pop.in
+++ b/quilt/pop.in
@@ -111,12 +111,9 @@ check_for_pending_changes()
                             --no-backup-if-mismatch -E \
                             >/dev/null 2>/dev/null
                then
-                       if ! [ -e $QUILT_PC/$patch ]
-                       then
-                               printf $"Failed to patch temporary files\n" >&2
-                               rm -rf $workdir
-                               return 1
-                       fi
+                       printf $"Failed to patch temporary files\n" >&2
+                       rm -rf $workdir
+                       return 1
                fi
        fi
 
-- 

Subject: [PATCH 2/2] RFC: pop: Harmonize `patch --force` usage between push
 and pop.

push:apply_patch() uses --force by default but pop:check_for_pending_changes()
did not. This leads to a problem when trying to pop a patch that changes files
which were originally read-only: patch refuses to touch them.
---
 quilt/pop.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/quilt/pop.in b/quilt/pop.in
index 8b69f64..63367dc 100644
--- a/quilt/pop.in
+++ b/quilt/pop.in
@@ -108,7 +108,7 @@ check_for_pending_changes()
                if ! cat_file $patch_file \
                     | patch -d $workdir $QUILT_PATCH_OPTS \
                             $(patch_args $patch) \
-                            --no-backup-if-mismatch -E \
+                            --no-backup-if-mismatch -E --force \
                             >/dev/null 2>/dev/null
                then
                        printf $"Failed to patch temporary files\n" >&2
--