Re: [Quilt-dev] [patch 1/5] Verbosly fail when trying to push a non existant patch

[Jean Delvare]

Hi Martin,

Le vendredi 18 janvier 2013 à 15:48 +0100, Martin Quinson a écrit :
> On Thu, Jan 17, 2013 at 10:53:42AM +0100, Jean Delvare wrote:
> > This doesn't apply clealy on top of the repository, please rebase.
> 
> See in attachment. 

Thanks.

I don't like it. It changes the behavior of a corner case in a
discussable and inconsistent way, plus the implementation adds
confusion. It can be seen in the test case you contributed:

        $ quilt push -qa
        > Patch patches/missing1.diff does not exist
        > Applying patch patches/missing1.diff

You made the command fail, yet it still claims that the patch was
applied, which it was not. So at the very least the check for patch
existence should be made upfront, _before_ printing "Applying patch %s
\n".

Even then, the behavior change can be discussed. The original bug
reporter and patch contributor claims that a missing patch "usually
indicates there is a problem". However the fact is that quilt itself
will let you create this situation. See:

$ quilt new empty
Patch empty is now on top
$ quilt pop
Patch empty appears to be empty, removing

No patches applied
$ quilt push
Applying patch empty
Patch empty does not exist; applied empty patch

Now at patch empty
$

This did not involve any manual editing of the series file. As long as
"quilt pop" lets you pop a missing patch file, I see no rationale for
"quilt push" failing on missing patch file. The current behavior seems
reasonable to me (although the wording could be improved, missing !=
empty) and more importantly, it is consistent. The messages are clear
enough so the user should notice if the patch wasn't expected to be
missing/empty. The only problem is in automated context where no human
can read the message, maybe this is what you (or Nicolas) were worried
about.

I am fine changing the behavior if you think it would be helpful in
practice, but only if this is done in a consistent way. This means that
"quilt pop" should fail on empty patches if "quilt refresh" wasn't
called to at least create an empty patch file. In other words, empty
patches should be handled with success and missing patches should not
(unless forced.)

What do you think?

-- 
Jean Delvare
Suse L3