Re: [PATCH] filter-branch: rewrite only refs which were not excludedbythe options

From: Johannes Sixt <j.sixt@eudaptics.com>
Date: Jul 24, 2007, 15:03 UTC
Message-ID: <46A614CE.597C7130@eudaptics.com>
In-Reply-To: <Pine.LNX.4.64.0707241518290.14781@racer.site>

Johannes Schindelin wrote:

Show 44 quoted lines

> 
> Hi,
> 
> On Tue, 24 Jul 2007, Johannes Sixt wrote:
> 
> > Johannes Schindelin wrote:
> >
> > > On Tue, 24 Jul 2007, Johannes Sixt wrote:
> > >
> > > > But there's another problem. Consider this history:
> > > >
> > > >    ---X--o--M         <- master
> > > >              \
> > > >           ...-o-...-o <- topic
> > > >
> > > > Then this (rather contrieved) command:
> > > >
> > > >    $ git-filter-branch -n $n master topic --not X
> > > >
> > > > If $n is small enough so that M is never rewritten, then
> > > >
> > > >    git rev-list -1 "$ref" $negatives
> > > >
> > > > still expands to non-empty even for 'master' (= M), which then
> > > > incorrectly ends up in "$tempdir"/heads.
> > >
> > > Aaargh!  Of course!  Since I have to add --topo-order at the end.
> > > Otherwise it makes no sense.
> >
> > No, that was no my point: In my example above, if n=1, `git rev-list -1
> > "$ref" $negatives` evaluates to
> >
> >     $ git rev-list -1 "master" -n 1 ^X
> >
> > which returns M, even though M is not going to be rewritten.
> > --topo-order changes nothing. The problem is that the -n is a relative
> > restriction. --since is turned into --max-age, which is absolute,
> > therefore, the test works as expected with --since.
> 
> So you think we have to say something like
> 
>         git rev-list "$ref" $negatives | grep "$ref" > /dev/null || continue
> 
> ?

No, doesn't help either.

We are talking about a case where there is more than one positive ref. We need not consider the -- sub/ case - it makes things just even more complicated. There are two different rev ranges to be considered:

  # (1) candidate range to be rewritten
  $ git rev-list "$@"

  # (2) test if positive ref is in candidate range
  $ git rev-list $ref $negatives

Usually (without -n) (2) is a subset of (1) because (1) has all positive refs and (2) has only one. And the subset is empty iff the positive ref is not in the rewritten range.

However, if "$@" (and, hence, $negatives) contains the -n limiter, this is no longer true. Example:

 (1)  is: "rev-list -n 1 A B ^C": we get either A or B, but not both;
      lets say A.
 (2A) is: "rev-list -n 1 A ^C": we get A -- good
 (2B) is: "rev-list -n 1 B ^C": we get B -- ouch!

For each positive ref, A, B, we conduct test (2). For A, the test result is correct. But for B it is not. We expect it to tell us that B is not rewritten; but in fact it tells the opposite by returning something instead of nothing.

-- Hannes