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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

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

> > > > @@ -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?

> 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

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux