Re: [PATCH 6/6] Add git-rewrite-commits

From: Sven Verdoolaege <skimo@kotnet.org>
Date: Jul 19, 2007, 12:40 UTC
Message-ID: <20070719124053.GC999MdfPADPa@greensroom.kotnet.org>
In-Reply-To: <Pine.LNX.4.64.0707181205260.14781@racer.site>

On Wed, Jul 18, 2007 at 12:17:03PM +0100, Johannes Schindelin wrote:

> Okay.  For me it does not at all exclude that.  If I want to replace a 
> commit by no commit, I write a commit-filter which does not return 
> anything.  If I return more than one SHA1s, I damned well want all of 
> those be the replacement "commit".

So how about you telling me what it _means_ for one commit to be replaced by more than one commit or at least giving me an example?

Show 10 quoted lines

> > - if the parents of a commit have been rewritten to one or more
> >   commits, then they are replaced by the new commits.
> 
> Yes, that is the primary use for the mapping.
> 
> >   If any parent has been pruned, then it is replaced by
> >   the result of applying this operation on _its_ parents.
> 
> Why?  This is overy complicated.  If a commit has been pruned, why does 
> the mapping not point to the _non-pruned_ parent?

It may not have any non-pruned parents and for the pruned ones, we wouldn't want to lose the relation with the non-pruned ancestors.

Show 7 quoted lines

> IOW if you have 
> something like this:
> 
> 	A - B - C - D - E - F
> 
> and all commits except A and F are pruned, the mapping for A, B, C, D and 
> E should _all_ point to the (possibly rewritten) A.

I'm not sure what you mean by "mapping" here, but the operation described above would make all of B, C, D, E and F have (the possibly rewritten) A as single parent (and parenthood was all I was talking about above).

Show 6 quoted lines

> > - a SHA1 of a commit that appears in a commit message is replaced
> >   by the rewritten commit iff it was rewritten to a single commit.
> >   That is, if the commit was pruned or rewritten (through a commit
> >   filter to more than one commit), then the SHA1 is left alone.
> 
> Both this behaviour and the one you described in your reply are wrong.

So tell me what you would do then and why that would make sense.

Show 11 quoted lines

> > - the mapping available to filters
> >     * if the commit was pruned, an empty file is created
> >     * otherwise a file is created containing all rewritten SHA1s
> 
> As I stated above: it is utterly wrong to create an empty mapping for a 
> commit that was pruned.  It does not take long to think of an example:
> 
> 	A - B - C - D
> 
> Now, A and D get pruned.  Do you want the whole branch to vanish?  _Hell, 
> no_.

Define "vanish" and, again, tell me what you would do.

> You have to at least give the users a chance to grasp what they are doing.  
> And if that means to change the semantics to something saner, then so be 
> it.

Let's get things straight. I've added the map files and the possibility for a commit filter to return more than one commit because you asked me to. I've tried to make sense of it, but if you think the behavior I defined is not what it is supposed to be, then it is up to _you_ to tell me what you think it should be instead of letting me guess.

I think I'll just remove the possibility for the commit filter to return more than one SHA1 (or maybe even no SHA1s). filter-branch doesn't seem to allow either of those either.

skimo