Re: [PATCH] filter-branch: rewrite only refs which were not excludedbythe options
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 24, 2007, 13:41 UTC
- Message-ID
- <Pine.LNX.4.64.0707241435290.14781@racer.site>
- In-Reply-To
- <46A5FF69.F5D75C9E@eudaptics.com>
Hi,
On Tue, 24 Jul 2007, Johannes Sixt wrote:
Show 19 quoted lines
> Johannes Schindelin wrote: > > > > On Tue, 24 Jul 2007, Johannes Sixt wrote: > > > > > BTW, '--all' in the argument list of filter-branch works only if it > > > is preceded by '--': > > > > > > git filter-branch <some-filter> -- --all <rev-list options> > > > > Hmm. Maybe we should reconsider the logic. I.e. instead of > > > > *) > > usage > > > > *) > > break > > That is not enough: This case block comes after a test that checks that > one additional argument is present, which would not be true anymore.
Right.
Show 59 quoted lines
> > > > @@ -181,6 +181,7 @@ export GIT_DIR GIT_WORK_TREE=.
> > > >
> > > > # These refs should be updated if their heads were rewritten
> > > >
> > > > +negatives="$(git rev-parse --revs-only "$@" | grep "^\^")"
> > > > git rev-parse --revs-only --symbolic "$@" |
> > > > while read ref
> > > > do
> > > > @@ -196,7 +197,13 @@ do
> > > > grep "refs/\(tags\|heads\)/$ref$")"
> > > > esac
> > > >
> > > > - git check-ref-format "$ref" && echo "$ref"
> > > > + # make sure we have a valid ref
> > > > + git check-ref-format "$ref" || continue
> > > > +
> > > > + # if the ref has been excluded by the other options, skip it
> > > > + test -z "$(git rev-list -1 "$ref" $negatives)" && continue
> > >
> > > Does this catch my use-case with --since? I think not, because:
> > >
> > > $ git rev-parse --revs-only --since=2007.01.01 master topic
> > > --max-age=1167606000
> > > 257061f3323dc0162f731d934f0870e919211fdf
> > > 3405729b94a654df8afbb9a1e13a4cf49a1c351c
> > >
> > > There are no negatives. Does it help to filter the non-positives?
> > >
> > > negatives=$(git rev-parse --revs-only "$@" | egrep -v '^[0-9a-f]{40}$')
> > >
> > > (Except the the '{40}' part is not portable. Hmpf.)
> >
> > To keep the "--since=..." we have to lose the "--revs-only"...
> > Darn. I thought that "--since=" was expanded by rev-parse. FWIW this
> > might work:
> >
> > negatives="$(git rev-parse "$@" | while read line
> > do
> > case "$line" in
> > $_x40) ;;
> > *) echo "$line";;
> > esac
> > done)"
> >
> > Can you please test? I am off for lunch.
>
> This worked:
>
> negatives=`git rev-parse --revs-only "$@" | while read line
> do
> case "$line" in
> $_x40) ;;
> *) echo "$line";;
> esac
> done`
>
> i.e. the closing parenthesis in the case arms together with the opening
> $( made for a syntax error. The --revs-only did not hurt in my tests,
> but you may have other reasons to remove it.Funny. AFAIR something similar worked here, all the time. But I believe you... you're on MinGW, right?
Show 16 quoted lines
> 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.
> I think the decision whether a positive ref should be rewritten should > be postponed until the rewrite has completed. Because then we know for > certain which revs were treated and can pick the matching refs. We only > lose the check for the error "Which ref do you want to rewrite?"
No, that is not enough:
A - B - C
B touches the subdirectory sub/.
git filter-branch C -- sub/
will not rewrite C.
Besides, I really like the check for the error "Which ref do you want to rewrite today?" early, instead of working for a long time, only to say "there was nothing to rewrite, you idiot, you should have chosen the arguments more carefully".
Ciao, Dscho