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

[Martin Quinson]

Agreed, this fix is a bit odd, actually. But I have absolutely no time
to devote to quilt within the next month(es). If you don't like the
change, don't integrate it that's fine. 

I marked the URL of your rejection arguments in the metadata of the
patch, and I'll think of how to improve (or drop it in debian too)
during the next round of discussion for patch inclusion.

Please don't delay the release of the next version because of me.

Bye, Mt.

On Wed, Jan 23, 2013 at 11:32:13AM +0100, Jean Delvare wrote:
> 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
> 

-- 
> Tu connais les cartes mères Gigabyte ?
Ces gens qui croient que quand on est informaticien, on connait tout sur les
pièces détachées. Je suis pas mécanicien moi, je suis pilote.