Re: [Quilt-dev] [PATCH] quilt drop command

[Jean Delvare]

[Josh Boyer]
> As I said before, I don't particularly care about the name although
> I'm inclined to agree with you.
> 
> The more I think about it, the more I like your "graft" name.  It fits
> what the command does fairly well.  If we went with that, I'd like to
> keep it a separate command though, not an option to "pop".

And the more *I* think about it, the more I like my "incorporate" name.
Maybe I should have come up with a shorter list in the first place,
listing just about any name that crossed my mind was the most certain
way to ensure that no two of us would pick the same ;)

I also tried to think more about the function itself, not just its name.
You might not like the conclusion I came to, but it's probably still
worth writing down here.

Your implementation of the "drop" feature (let's call it that way for
now) is weak. It works for you because you know what you're doing. You
know what patches modify each file, and when you were doing a "cvs
commit" after "quilt pop" and "patch -p1" so far, you knew that the file
you commited wasn't modified by any other currently applied patch.
However, the implementation of the "drop" feature you proposed fails to
check that. This means that it would very easily cause disasters if
improperly used - and you can expect users to misuse it in the absence
of checks to prevent them to do so. Let's imagine the very trivial
scenario below:

* You have two patches applied, A first, B on top of A.

* Both A and B modify the same file.

* You do "quilt drop", which will operate on patch B.

At this point, you cannot do your "cvs commit" because A is still
applied. And I don't think you can pop A either because it would restore
a file which does NOT contain the changes made by B. Am I missing
something? If I'm not, then your implementation of "drop" will only work
in a very limited number of cases, and will cause much trouble in all
the other ones. I don't think we want that.

So I tend to think that this function can only be accepted if either of
two conditions are met:

1* Strict checks are made to ensure that the function can only be used
when it makes sense. For example, you could only be allowed to "drop"
all patches from the top-most applied one down to the very bottom of the
stack. Or checks could be done to ensure that "dropped" patches modify
only files which are not modified by any other applied patches.

2* The function is extended to cover just about any case. This sounds
much more complex, but there probably is a need for this. Seeing the
recent "merging with upstream and adding new files" thread on this list,
it looks like there is a general need for the feature. I do have a need
myself. Each time a new -rc of Linux is released, I upgrade my baseline
to it, and most of the time it requires me to discard a number of
patches which were in my stack and made it in. This isn't exactly the
same as your "prepare for a cvs commit" need, but there definitely are
common points (how do we extract patches out of the series, regardless
of their position?)

Possibly related:

Two functions we might need, and which might help solving the problem,
are promote and demote. These functions would allow patches to be moved
inside the series files. Promote would move a patch down the stack.
Demote would move a patch up the stack. That way, we could promote
patches that have to be applied to the baseline first (either through a
baseline update, or for the purpose of a cvs commit). I'm a bit
surprised that this function doesn't exist yet. Are we simply supposed
to pop all the patches and push them one by one in the new order? In my
experience it isn't a very convenient way, I hope we can do better.
Something in the line of "I want patch B to be applied after patch A" in
an existing series is pretty much what I need.

So all in all I believe that a much more in deep discussion and analysis
should occur about the function you want to add. It IS needed, I fully
agree, but the proper way to implement it is probably way above the
quick hack you proposed, even if that one does work in your particular
conditions.

Thanks,
-- 
Jean Delvare