[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Quilt-dev] [PATCH] quilt drop command
From: |
Josh Boyer |
Subject: |
Re: [Quilt-dev] [PATCH] quilt drop command |
Date: |
Wed, 01 Jun 2005 19:50:44 -0500 |
On Wed, 2005-06-01 at 23:52 +0200, Jean Delvare wrote:
> 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.
I'll listen to any conclusion people come to, as long as it makes sense.
And if it makes sense, I'll probably wind up agreeing with it whether it
goes against my original idea or not :).
>
> 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.
Yes, exactly.
> 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.
I agree, the scope of the current command is quite limited. More on why
that came about below.
>
> 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.
That wouldn't be too difficult to add. I believe the refresh command
already has some of that kind of checking in that it won't refresh a
named patch if a newer patch modifies the same file.
>
> 2* The function is extended to cover just about any case. This sounds
> much more complex, but there probably is a need for this.
<snip>
> 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?)
Hm.. see below.
>
> 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.
But that is sort of the problem. The series file doesn't really contain
any notion of patch dependency order. It simply contains a list of
patches and the order they were applied in. Whether or not those
patches are related to one another is beyond the current scope of quilt.
For example, your series file could look like:
d.patch
c.patch
b.patch
a.patch
where b and c are related, but a and d are completely separate from
everything else. So you could promote and demote the patches in any
combination, as long as b is always applied before c. Quilt doesn't
have any way of know that today though, so it's left up to the user.
If we want to start dealing with patch dependency issues in quilt
itself, I think we need to add another level of management. Today we
have a series file that lists a bunch of patches, a.k.a "changesets".
What is needed for dependency tracking is a way to take that concept and
create multiple series files.
So in the above example you would have one series file for a, one for d,
and one for b and c combined. Then the individual series files become
explicit order listings and users would be able to push/pop a whole
series of patches with a single command. A term for the new style of
series file could be "patchset".
This could be highly useful to people using a single tree for multiple
patch sets that don't really have anything to do with each other. For
example, maintaining two separate driver trees.
This is slightly tangential to the original problem, but it may be worth
looking into.
>
> 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.
My quick hack simply evolved from an itch I wanted to scratch, so I'm
not surprised it's lacking. Originally, I had written a "commit"
command that would do the CVS commit for a user too and essentially
commit all applied patches. But the primary object of quilt is patches
not SCM functionality so I tried to make it more general. Now we can
see that it need to be generalized even more.
So lets think about the best way to do this. I'll try to come up with
something within the week. If anyone else has ideas, please post them.
josh