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

From: Sven Verdoolaege <skimo@kotnet.org>
Date: Jul 16, 2007, 10:24 UTC
Message-ID: <20070716102407.GL999MdfPADPa@greensroom.kotnet.org>
In-Reply-To: <Pine.LNX.4.64.0707160054340.14781@racer.site>

On Mon, Jul 16, 2007 at 01:38:11AM +0100, Johannes Schindelin wrote:

Show 14 quoted lines

> On Sun, 15 Jul 2007, Sven Verdoolaege wrote:
> > > > +	if (path_pruning &&
> > > > +	    !(commit->object.flags & (TREECHANGE | UNINTERESTING)))
> > > > +		return 1;
> > > 
> > > Why only with "path_pruning"?  Ah yes.  Because otherwise, you would 
> > > assume "A" in "A..B" to be pruned.
> > 
> > TREECHANGE is only set when path pruning is in effect.
> > If I didn't check for path_pruning, then all commits would be
> > considered to have been pruned.  (Or am I missing something?
> > Honestly, I found all that TREECHANGE stuff difficult to follow.)
> 
> AFAICT TREECHANGE means that parents were rewritten.

I think you'll find that if all commits touch a path in the path specifiers then all commits will have TREECHANGE set and so no parents will be rewritten.

Show 6 quoted lines

> > revision.c itself is also riddled with "prune_fn && ".
> > Wouldn't it make sense to invert the meaning of this bit and call
> > it, say, PRUNED, so that the default is off and you would only
> > have to check if the bit was set ?
> 
> You meant the TREECHANGE bit?  No.

Yes.  Why?

> BTW what do you plan to do about my objection to UNINTERESTING, given the 
> example "git rewrite-commits A..B x/y"?

That was based on an apparent misunderstanding of my code that I tried to address above. I did not intend to do what you claim I do and a quick test confirms that my code does indeed not to what you claim it does.

More specifically, the history will not be cut off at A because A is marked UNINTERESTING and is therefore not considered to have been pruned. A commit is considered pruned if it was either explicitly marked as such or if TREECHANGE is not set, but the later check (in is_pruned) is only done on commits that were checked for tree changes.

skimo