Re: [Quilt-dev] [PATCH 2/2] pop: Harmonize `patch --force` usage between push and pop.

[Jean Delvare]

Hi Benjamin,

Le vendredi 30 novembre 2012 à 16:43 -0500, Benjamin Poirier a écrit :
> push:apply_patch() uses --force by default but pop:check_for_pending_changes()
> does 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.

There was a rationale for this asymmetry: when a patch is applied (with
push), it is possible that it doesn't apply cleanly. However when
popping a patch, either it has been refreshed at some point in time
after having been pushed, and it will apply cleanly on top of the
previous version of each file (exactly what quilt pop does); or it did
not apply cleanly and wasn't refreshed yet, in which
case .pc/$patch~refresh has been created and quilt pop will either
refuse to run, or will skip the verification altogether (if forced.)

So the patch command in quilt pop was simply not supposed to ever fail,
which is why no --force parameter is passed to it. There are two
exceptions to this rule though: patches with a header but no actual
contents, and internal errors such as the read-only file issue you've
just hit and reported.

While I believe the read-only error would be better handled differently
(primarily in "patch" itself, and possibly also with a new option to
backup-files -r that would ensure all restored files can be written to -
that could as well be the default behavior when -k is passed), I have no
objection to adding a force parameter to this patch command to avoid
similar situations in the future.

I would also drop the test and the error message. As I said before, the
status code returned by patch isn't accurate enough for automatic
diagnostic of the actual cause of the problem. If the command failed
then the following file comparison is the only way to know if changes
are about to be lost or not.

> ---
>  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


-- 
Jean Delvare
Suse L3