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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Mon, 16 Jul 2007, Sven Verdoolaege wrote:

> On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:
> > Didn't I mention that it was a severe limitation to think of the sha1 
> > mapping of a 1-to-1 mapping?  Think of it more as a relation.
> 
> The mapping is used in several operations.
> First, there are several things that can happen to a commit
> 
> - it's pruned.  This includes, for me, path pruning, matching
>   and a commit filter returning no SHA1s.

Okay.

> - it's rewritten to another commit that can be considered the
>   "moral equivalent" of that commit.  This occurs when a commit
>   is not pruned, but something else happened to the commit itself or
>   one of its ancestors.  This excludes, for me, the case
>   where a commit filter returns more than one SHA1.

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".

To say "the" replacement "commit", means to mistake the mapping as a 
function, a non-relation.

> - it's replaced by more than one SHA1.  This can only happen
>   in a commit filter.

For the moment, yes.

> There are at least four operations in which this mapping is used:
> 
> - 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? 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.

> - any reference (in refs/) that points to a rewritten or pruned
>   commit is removed and
>     * if the commit was rewritten to a single commit, then it is
>       replaced by this commit
>     * otherwise, there is no moral equivalent single commit, but
>       we want to ensure we can still access the new commits, so
>       I create several references, either to each of the many
>       commits the old commit was rewritten to, or to each of
>       its nearest unpruned ancestors (i.e., the same set as
>       described in the previous operation).

I'd argue that it should be an error if a to-be-rewritten ref (and I still 
strongly disagree with you that all refs should be rewritten) would point 
to multiple commits.  Possibly overridable with "--allow-octopus-refs".  
But the default should be to error out.

> - 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.

> - 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_.

> I understand you want the second operation to only apply to refs 
> explicitly mentioned on the command line.

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.

Ciao,
Dscho