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

Johannes Schindelin wrote:

Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

quoted

Johannes Schindelin wrote:

quoted

So really exclude excluded refs from being rewritten.  This also allows
you to safely call

        git filter-branch <some-filter> --all <rev-list options>

to rewrite _all_ branches and tags.

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.

quoted

quoted

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

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.

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

-- Hannes