[PATCH] filter-branch: Big syntax change; support rewriting multiple refs

[PATCH] filter-branch: Big syntax change; support rewriting multiple refs

[Johannes Schindelin]

We used to take the first non-option argument as the name for the new 
branch.  This syntax is not extensible to support rewriting more than just 
HEAD.

Instead, we now have the following syntax:

	git filter-branch [<filter options>...] [<rev-list options>]

All positive refs given in <rev-list options> are rewritten.  Yes, 
in-place.  If a ref was changed, the original head is stored in 
refs/original/$ref now, for your inspecting pleasure, in addition to the 
reflogs (since it is easier to inspect "git show-ref | grep original" than 
to inspect all the reflogs).

This commit also adds the --force option to remove .git-rewrite/ and all 
refs from refs/original/ before filtering.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Junio, I know that this comes quite late in the game, but I really
	think that the "first arg is new branch name" was a bad syntax.

	Could you please consider taking this patch (or whatever version
	comes out after review ;-) or keeping filter-branch of 1.5.3?  I 
	do not want people to get used to the borked syntax...

	BTW I considered "git log -g --all" as an alternative to 
	inspecting refs/original/, but ATM this die()s if just _one_ of 
	the refs has no logs.  Probably should fix that, too.

 Documentation/git-filter-branch.txt |   51 +++++++------
 git-filter-branch.sh                |  150 +++++++++++++++++++++++++++++-----
 t/t7003-filter-branch.sh            |   41 ++++++----
 3 files changed, 182 insertions(+), 60 deletions(-)

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index eaea82d..915258f 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 	[--index-filter <command>] [--parent-filter <command>]
 	[--msg-filter <command>] [--commit-filter <command>]
 	[--tag-name-filter <command>] [--subdirectory-filter <directory>]
-	[-d <directory>] <new-branch-name> [<rev-list options>...]
+	[-d <directory>] [-f | --force] [<rev-list options>...]
 
 DESCRIPTION
 -----------
@@ -26,10 +26,9 @@ information) will be preserved.
 The command takes the new branch name as a mandatory argument and
 the filters as optional arguments.  If you specify no filters, the
 commits will be recommitted without any changes, which would normally
-have no effect and result in the new branch pointing to the same
-branch as your current branch.  Nevertheless, this may be useful in
-the future for compensating for some git bugs or such, therefore
-such a usage is permitted.
+have no effect.  Nevertheless, this may be useful in the future for
+compensating for some git bugs or such, therefore such a usage is
+permitted.
 
 *WARNING*! The rewritten history will have different object names for all
 the objects and will not converge with the original branch.  You will not
@@ -38,8 +37,9 @@ original branch.  Please do not use this command if you do not know the
 full implications, and avoid using it anyway, if a simple single commit
 would suffice to fix your problem.
 
-Always verify that the rewritten version is correct before disposing
-the original branch.
+Always verify that the rewritten version is correct: The original refs,
+if different from the rewritten ones, will be stored in the namespace
+'refs/original/'.
 
 Note that since this operation is extensively I/O expensive, it might
 be a good idea to redirect the temporary directory off-disk, e.g. on
@@ -142,6 +142,11 @@ definition impossible to preserve signatures at any rate.)
 	does this in the '.git-rewrite/' directory but you can override
 	that choice by this parameter.
 
+-f\|--force::
+	`git filter-branch` refuses to start with an existing temporary
+	directory or when there are already refs starting with
+	'refs/original/', unless forced.
+
 <rev-list-options>::
 	When options are given after the new branch name, they will
 	be passed to gitlink:git-rev-list[1].  Only commits in the resulting
@@ -156,14 +161,14 @@ Suppose you want to remove a file (containing confidential information
 or copyright violation) from all commits:
 
 -------------------------------------------------------
-git filter-branch --tree-filter 'rm filename' newbranch
+git filter-branch --tree-filter 'rm filename' HEAD
 -------------------------------------------------------
 
 A significantly faster version:
 
--------------------------------------------------------------------------------
-git filter-branch --index-filter 'git update-index --remove filename' newbranch
--------------------------------------------------------------------------------
+--------------------------------------------------------------------------
+git filter-branch --index-filter 'git update-index --remove filename' HEAD
+--------------------------------------------------------------------------
 
 Now, you will get the rewritten history saved in the branch 'newbranch'
 (your current branch is left untouched).
@@ -172,25 +177,25 @@ To set a commit (which typically is at the tip of another
 history) to be the parent of the current initial commit, in
 order to paste the other history behind the current history:
 
-------------------------------------------------------------------------
-git filter-branch --parent-filter 'sed "s/^\$/-p <graft-id>/"' newbranch
-------------------------------------------------------------------------
+-------------------------------------------------------------------
+git filter-branch --parent-filter 'sed "s/^\$/-p <graft-id>/"' HEAD
+-------------------------------------------------------------------
 
 (if the parent string is empty - therefore we are dealing with the
 initial commit - add graftcommit as a parent).  Note that this assumes
 history with a single root (that is, no merge without common ancestors
 happened).  If this is not the case, use:
 
--------------------------------------------------------------------------------
+--------------------------------------------------------------------------
 git filter-branch --parent-filter \
-	'cat; test $GIT_COMMIT = <commit-id> && echo "-p <graft-id>"' newbranch
--------------------------------------------------------------------------------
+	'cat; test $GIT_COMMIT = <commit-id> && echo "-p <graft-id>"' HEAD
+--------------------------------------------------------------------------
 
 or even simpler:
 
 -----------------------------------------------
 echo "$commit-id $graft-id" >> .git/info/grafts
-git filter-branch newbranch $graft-id..
+git filter-branch $graft-id..HEAD
 -----------------------------------------------
 
 To remove commits authored by "Darl McBribe" from the history:
@@ -208,7 +213,7 @@ git filter-branch --commit-filter '
 		done;
 	else
 		git commit-tree "$@";
-	fi' newbranch
+	fi' HEAD
 ------------------------------------------------------------------------------
 
 The shift magic first throws away the tree id and then the -p
@@ -238,14 +243,14 @@ A--B-----C
 To rewrite only commits D,E,F,G,H, but leave A, B and C alone, use:
 
 --------------------------------
-git filter-branch ... new-H C..H
+git filter-branch ... C..H
 --------------------------------
 
 To rewrite commits E,F,G,H, use one of these:
 
 ----------------------------------------
-git filter-branch ... new-H C..H --not D
-git filter-branch ... new-H D..H --not C
+git filter-branch ... C..H --not D
+git filter-branch ... D..H --not C
 ----------------------------------------
 
 To move the whole tree into a subdirectory, or remove it from there:
@@ -255,7 +260,7 @@ git filter-branch --index-filter \
 	'git ls-files -s | sed "s-\t-&newsubdir/-" |
 		GIT_INDEX_FILE=$GIT_INDEX_FILE.new \
 			git update-index --index-info &&
-	 mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE' directorymoved
+	 mv $GIT_INDEX_FILE.new $GIT_INDEX_FILE' HEAD
 ---------------------------------------------------------------
 
 
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 0d000ed..0ff3475 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -78,6 +78,8 @@ filter_msg=cat
 filter_commit='git commit-tree "$@"'
 filter_tag_name=
 filter_subdir=
+orig_namespace=refs/original/
+force=
 while case "$#" in 0) usage;; esac
 do
 	case "$1" in
@@ -85,6 +87,11 @@ do
 		shift
 		break
 		;;
+	--force|-f)
+		shift
+		force=t
+		continue
+		;;
 	-*)
 		;;
 	*)
@@ -126,24 +133,43 @@ do
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
 		;;
+	--original)
+		orig_namespace="$OPTARG"
+		;;
 	*)
 		usage
 		;;
 	esac
 done
 
-dstbranch="$1"
-shift
-test -n "$dstbranch" || die "missing branch name"
-git show-ref "refs/heads/$dstbranch" 2> /dev/null &&
-	die "branch $dstbranch already exists"
-
-test ! -e "$tempdir" || die "$tempdir already exists, please remove it"
+case "$force" in
+t)
+	rm -rf "$tempdir"
+;;
+'')
+	test -d "$tempdir" &&
+		die "$tempdir already exists, please remove it"
+esac
 mkdir -p "$tempdir/t" &&
+tempdir="$(cd "$tempdir"; pwd)" &&
 cd "$tempdir/t" &&
 workdir="$(pwd)" ||
 die ""
 
+# Make sure refs/original is empty
+git for-each-ref > "$tempdir"/backup-refs
+while read sha1 type name
+do
+	case "$force,$name" in
+	,$orig_namespace*)
+		die "Namespace $orig_namespace not empty"
+	;;
+	t,$orig_namespace*)
+		git update-ref -d "$name" $sha1
+	;;
+	esac
+done < "$tempdir"/backup-refs
+
 case "$GIT_DIR" in
 /*)
 	;;
@@ -153,6 +179,29 @@ case "$GIT_DIR" in
 esac
 export GIT_DIR GIT_WORK_TREE=.
 
+# These refs should be updated if their heads were rewritten
+
+git rev-parse --revs-only --symbolic "$@" |
+while read ref
+do
+	# normalize ref
+	case "$ref" in
+	HEAD)
+		ref="$(git symbolic-ref "$ref")"
+	;;
+	refs/*)
+	;;
+	*)
+		ref="$(git for-each-ref --format='%(refname)' |
+			grep /"$ref")"
+	esac
+
+	git check-ref-format "$ref" && echo "$ref"
+done > "$tempdir"/heads
+
+test -s "$tempdir"/heads ||
+	die "Which ref do you want to rewrite?"
+
 export GIT_INDEX_FILE="$(pwd)/../index"
 git read-tree || die "Could not seed the index"
 
@@ -174,6 +223,8 @@ commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
 
+# Rewrite the commits
+
 i=0
 while read commit parents; do
 	i=$(($i+1))
@@ -234,22 +285,75 @@ while read commit parents; do
 		$(git write-tree) $parentstr < ../message > ../map/$commit
 done <../revs
 
-src_head=$(tail -n 1 ../revs | sed -e 's/ .*//')
-target_head=$(head -n 1 ../map/$src_head)
-case "$target_head" in
-'')
-	echo Nothing rewritten
+# In case of a subdirectory filter, it is possible that a specified head
+# is not in the set of rewritten commits, because it was pruned by the
+# revision walker.  Fix it by mapping these heads to the next rewritten
+# ancestor(s), i.e. the boundaries in the set of rewritten commits.
+
+# NEEDSWORK: we should sort the unmapped refs topologically first
+while read ref
+do
+	sha1=$(git rev-parse "$ref"^0)
+	test -f "$workdir"/../map/$sha1 && continue
+	# Assign the boundarie(s) in the set of rewritten commits
+	# as the replacement commit(s).
+	# (This would look a bit nicer if --not --stdin worked.)
+	for p in $((cd "$workdir"/../map; ls | sed "s/^/^/") |
+		git rev-list $ref --boundary --stdin |
+		sed -n "s/^-//p")
+	do
+		map $p >> "$workdir"/../map/$sha1
+	done
+done < "$tempdir"/heads
+
+# Finally update the refs
+
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+count=0
+echo
+while read ref
+do
+	# avoid rewriting a ref twice
+	test -f "$orig_namespace$ref" && continue
+
+	sha1=$(git rev-parse "$ref"^0)
+	rewritten=$(map $sha1)
+
+	test $sha1 = "$rewritten" &&
+		warn "WARNING: Ref '$ref' is unchanged" &&
+		continue
+
+	case "$rewritten" in
+	'')
+		echo "Ref '$ref' was deleted"
+		git update-ref -m "filter-branch: delete" -d "$ref" $sha1 ||
+			die "Could not delete $ref"
 	;;
-*)
-	git update-ref refs/heads/"$dstbranch" $target_head ||
-		die "Could not update $dstbranch with $target_head"
-	if [ $(wc -l <../map/$src_head) -gt 1 ]; then
-		echo "WARNING: Your commit filter caused the head commit to expand to several rewritten commits. Only the first such commit was recorded as the current $dstbranch head but you will need to resolve the situation now (probably by manually merging the other commits). These are all the commits:" >&2
-		sed 's/^/	/' ../map/$src_head >&2
-		ret=1
-	fi
+	$_x40)
+		echo "Ref '$ref' was rewritten"
+		git update-ref -m "filter-branch: rewrite" \
+				"$ref" $rewritten $sha1 ||
+			die "Could not rewrite $ref"
 	;;
-esac
+	*)
+		# NEEDSWORK: possibly add -Werror, making this an error
+		warn "WARNING: '$ref' was rewritten into multiple commits:"
+		warn "$rewritten"
+		warn "WARNING: Ref '$ref' points to the first one now."
+		rewritten=$(echo "$rewritten" | head -n 1)
+		git update-ref -m "filter-branch: rewrite to first" \
+				"$ref" $rewritten $sha1 ||
+			die "Could not rewrite $ref"
+	;;
+	esac
+	git update-ref -m "filter-branch: backup" "$orig_namespace$ref" $sha1
+	count=$(($count+1))
+done < "$tempdir"/heads
+
+# TODO: This should possibly go, with the semantics that all positive given
+#       refs are updated, and their original heads stored in refs/original/
+# Filter tags
 
 if [ "$filter_tag_name" ]; then
 	git for-each-ref --format='%(objectname) %(objecttype) %(refname)' refs/tags |
@@ -286,6 +390,8 @@ fi
 
 cd ../..
 rm -rf "$tempdir"
-printf "\nRewritten history saved to the $dstbranch branch\n"
+echo
+test $count -gt 0 && echo "These refs were rewritten:"
+git show-ref | grep ^"$orig_namespace"
 
 exit $ret
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 4ddd656..bc6e2dd 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -30,24 +30,24 @@ test_expect_success 'setup' '
 H=$(git rev-parse H)
 
 test_expect_success 'rewrite identically' '
-	git-filter-branch H2
+	git-filter-branch branch
 '
-
 test_expect_success 'result is really identical' '
-	test $H = $(git rev-parse H2)
+	test $H = $(git rev-parse HEAD)
 '
 
 test_expect_success 'rewrite, renaming a specific file' '
-	git-filter-branch --tree-filter "mv d doh || :" H3
+	git-filter-branch -f --tree-filter "mv d doh || :" HEAD
 '
 
 test_expect_success 'test that the file was renamed' '
-	test d = $(git show H3:doh)
+	test d = $(git show HEAD:doh)
 '
 
-git tag oldD H3~4
+git tag oldD HEAD~4
 test_expect_success 'rewrite one branch, keeping a side branch' '
-	git-filter-branch --tree-filter "mv b boh || :" modD D..oldD
+	git branch modD oldD &&
+	git-filter-branch -f --tree-filter "mv b boh || :" D..modD
 '
 
 test_expect_success 'common ancestor is still common (unchanged)' '
@@ -69,7 +69,8 @@ test_expect_success 'filter subdirectory only' '
 	git rm a &&
 	test_tick &&
 	git commit -m "again not subdir" &&
-	git-filter-branch --subdirectory-filter subdir sub
+	git branch sub &&
+	git-filter-branch -f --subdirectory-filter subdir refs/heads/sub
 '
 
 test_expect_success 'subdirectory filter result looks okay' '
@@ -89,7 +90,8 @@ test_expect_success 'setup and filter history that requires --full-history' '
 	test_tick &&
 	git commit -m "again subdir on master" &&
 	git merge branch &&
-	git-filter-branch --subdirectory-filter subdir sub-master
+	git branch sub-master &&
+	git-filter-branch -f --subdirectory-filter subdir sub-master
 '
 
 test_expect_success 'subdirectory filter result looks okay' '
@@ -100,7 +102,8 @@ test_expect_success 'subdirectory filter result looks okay' '
 '
 
 test_expect_success 'use index-filter to move into a subdirectory' '
-	git-filter-branch --index-filter \
+	git branch directorymoved &&
+	git-filter-branch -f --index-filter \
 		 "git ls-files -s | sed \"s-\\t-&newsubdir/-\" |
 	          GIT_INDEX_FILE=\$GIT_INDEX_FILE.new \
 			git update-index --index-info &&
@@ -108,9 +111,10 @@ test_expect_success 'use index-filter to move into a subdirectory' '
 	test -z "$(git diff HEAD directorymoved:newsubdir)"'
 
 test_expect_success 'stops when msg filter fails' '
-	! git-filter-branch --msg-filter false nonono &&
-	rm -rf .git-rewrite &&
-	! git rev-parse nonono
+	old=$(git rev-parse HEAD) &&
+	! git-filter-branch -f --msg-filter false &&
+	test $old = $(git rev-parse HEAD) &&
+	rm -rf .git-rewrite
 '
 
 test_expect_success 'author information is preserved' '
@@ -118,7 +122,8 @@ test_expect_success 'author information is preserved' '
 	git add i &&
 	test_tick &&
 	GIT_AUTHOR_NAME="B V Uips" git commit -m bvuips &&
-	git-filter-branch --msg-filter "cat; \
+	git branch preserved-author &&
+	git-filter-branch -f --msg-filter "cat; \
 			test \$GIT_COMMIT != $(git rev-parse master) || \
 			echo Hallo" \
 		preserved-author &&
@@ -129,7 +134,8 @@ test_expect_success "remove a certain author's commits" '
 	echo i > i &&
 	test_tick &&
 	git commit -m i i &&
-	git-filter-branch --commit-filter "\
+	git branch removed-author &&
+	git-filter-branch -f --commit-filter "\
 		if [ \"\$GIT_AUTHOR_NAME\" = \"B V Uips\" ];\
 		then\
 			shift;\
@@ -148,4 +154,9 @@ test_expect_success "remove a certain author's commits" '
 	test 0 = $(git rev-list --author="B V Uips" removed-author | wc -l)
 '
 
+test_expect_success 'barf on invalid name' '
+	! git filter-branch -f master xy-problem &&
+	! git filter-branch -f HEAD^
+'
+
 test_done
-- 
1.5.3.rc2.32.g35c5b

Re: [PATCH] filter-branch: Big syntax change; support rewriting multiple refs

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	Junio, I know that this comes quite late in the game, but I really
> 	think that the "first arg is new branch name" was a bad syntax.
>
> 	Could you please consider taking this patch (or whatever version
> 	comes out after review ;-) or keeping filter-branch of 1.5.3?  I 
> 	do not want people to get used to the borked syntax...

Yeah, "No new features after -rc" should not apply to this one.

I was actually going to ask you about it, since this is a
feature we have already advertised to the public, but still is a
new feature, and we'd be better off getting it right in the
first public version.

> 	BTW I considered "git log -g --all" as an alternative to 
> 	inspecting refs/original/, but ATM this die()s if just _one_ of 
> 	the refs has no logs.  Probably should fix that, too.

I do not think refs/original/ is such a hot feature.  What's
wrong with "gitk mine@{1}...mine"?

Re: [PATCH] filter-branch: Big syntax change; support rewriting multiple refs

[Johannes Schindelin]

Hi,

On Mon, 23 Jul 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	Junio, I know that this comes quite late in the game, but I really
> > 	think that the "first arg is new branch name" was a bad syntax.
> >
> > 	Could you please consider taking this patch (or whatever version
> > 	comes out after review ;-) or keeping filter-branch of 1.5.3?  I 
> > 	do not want people to get used to the borked syntax...
> 
> Yeah, "No new features after -rc" should not apply to this one.
> 
> I was actually going to ask you about it, since this is a feature we 
> have already advertised to the public, but still is a new feature, and 
> we'd be better off getting it right in the first public version.

Thanks.

> > 	BTW I considered "git log -g --all" as an alternative to
> > 	inspecting refs/original/, but ATM this die()s if just _one_ of 
> > 	the refs has no logs.  Probably should fix that, too.
> 
> I do not think refs/original/ is such a hot feature.  What's wrong with 
> "gitk mine@{1}...mine"?

If you are saying

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

potentially all refs are rewritten.

To find out which ones actually changed, you can use "git show-ref | grep 
^refs/original/" ATM.

It is not really easy to do it otherwise.  With the patches I sent out 
yesterday,

	$ git log -g --no-walk --all --decorate --abbrev-commit 
	  --pretty=oneline --since=<before-the-last-filter-branch-call>

would be similar, but not as comfortable, would it?

Of course, we could teach filter-branch an option, say --show-changed, 
which will not actually filter branches, but instead look at the reflogs 
itself and show the refs which were recently changed by filter-branch.

But note that you can switch off reflogs.

Ciao,
Dscho

Re: [PATCH] filter-branch: Big syntax change; support rewriting multiplerefs

[Johannes Sixt]

Johannes Schindelin wrote:
> +# These refs should be updated if their heads were rewritten
> +
> +git rev-parse --revs-only --symbolic "$@" |
> +while read ref
> +do
> +       # normalize ref
> +       case "$ref" in
> +       HEAD)
> +               ref="$(git symbolic-ref "$ref")"
> +       ;;
> +       refs/*)
> +       ;;
> +       *)
> +               ref="$(git for-each-ref --format='%(refname)' |
> +                       grep /"$ref")"
> +       esac
> +
> +       git check-ref-format "$ref" && echo "$ref"
> +done > "$tempdir"/heads

This does not work as I'd expected it: I can't successfully say:

   git-filter-branch master

It tells me:

   Which ref do you want to rewrite?

> +# NEEDSWORK: we should sort the unmapped refs topologically first
> +while read ref
> +do
> +       sha1=$(git rev-parse "$ref"^0)
> +       test -f "$workdir"/../map/$sha1 && continue
> +       # Assign the boundarie(s) in the set of rewritten commits
> +       # as the replacement commit(s).
> +       # (This would look a bit nicer if --not --stdin worked.)
> +       for p in $((cd "$workdir"/../map; ls | sed "s/^/^/") |
> +               git rev-list $ref --boundary --stdin |
> +               sed -n "s/^-//p")
> +       do
> +               map $p >> "$workdir"/../map/$sha1
> +       done
> +done < "$tempdir"/heads

This logic seems to be borked, and I don't grok it. I was trying this:

   git-filter-branch -- --since=2007.01.10 \
		refs/heads/topic refs/heads/master

where topic's last change is from before 2007.01.10, i.e. it is not
among the rewritten commits. And I get this:

WARNING: Ref 'refs/heads/master' is unchanged
WARNING: 'refs/heads/topic' was rewritten into multiple commits:
329325526647503382ae7dee41c12fe6b81bbe43
2bdf3349b76f72ceb71755c75a555d60bc7c73aa
ce6c9dfeac5ddbd64da2d9360d6552717df81d1f

The first 2 of these 3 sha1s happen to be merge bases of topic and
master, but none of them appear anywhere in

   git rev-list --parents --boundary --since=2007.07.10 \
		refs/heads/topic refs/heads/master

What's up?

-- Hannes

[PATCH] filter-branch: when dwim'ing a ref, only allow heads and tags

[Johannes Schindelin]

Previously, we matched all refs that had /$ref in them.  And tried to
verify the result as a ref.  If more than one match was found, the result
was not a ref, though.

So only allow tags and heads to be dwim'ed.  If both a tag and a head with
that name exist, it will be ignored again.

Caught by Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 24 Jul 2007, Johannes Sixt wrote:

	> Johannes Schindelin wrote:
	> > +# These refs should be updated if their heads were rewritten
	> > +
	> > +git rev-parse --revs-only --symbolic "$@" |
	> > +while read ref
	> > +do
	> > +       # normalize ref
	> > +       case "$ref" in
	> > +       HEAD)
	> > +               ref="$(git symbolic-ref "$ref")"
	> > +       ;;
	> > +       refs/*)
	> > +       ;;
	> > +       *)
	> > +               ref="$(git for-each-ref --format='%(refname)' |
	> > +                       grep /"$ref")"
	> > +       esac
	> > +
	> > +       git check-ref-format "$ref" && echo "$ref"
	> > +done > "$tempdir"/heads
	> 
	> This does not work as I'd expected it: I can't successfully say:
	> 
	>    git-filter-branch master
	> 
	> It tells me:
	> 
	>    Which ref do you want to rewrite?

	I tried that, and happily got the same.  The explanation: more 
	than one ref matched "/master".  My pattern was extremely borked.

	Thanks.

 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 0ff3475..019a302 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -193,7 +193,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep "refs/\(tags\|heads\)/$ref$")"
 	esac
 
 	git check-ref-format "$ref" && echo "$ref"
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index bc6e2dd..c9a820d 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -159,4 +159,9 @@ test_expect_success 'barf on invalid name' '
 	! git filter-branch -f HEAD^
 '
 
+test_expect_success 'only dwim refs/heads/$ref or refs/tags/$ref' '
+	git update-ref refs/remotes/origin/master HEAD &&
+	git filter-branch -f master
+'
+
 test_done
-- 
1.5.3.rc2.32.g35c5b-dirty

Re: [PATCH] filter-branch: when dwim'ing a ref, only allow heads andtags

[Johannes Sixt]

Johannes Schindelin wrote:
> -                       grep /"$ref")"
> +                       grep "refs/\(tags\|heads\)/$ref$")"

Thanks, but I don't think that this is a portable grep pattern:

$ echo "abc" | grep '\(a\|x\)'
$ echo "abc" | egrep '(a|x)'
abc

But then, this is on some AIX 4.3. How about:

   fgrep -e "refs/heads/$ref" -e "refs/tags/$ref"
or
   grep -e "^refs/heads/$ref" -e "^refs/tags/$ref"

-- Hannes

[PATCH] filter-branch: rewrite only refs which were not excluded by the options

[Johannes Schindelin]

The rev-list options can exclude some -- or all -- refs:

	git filter-branch <some-filter> master ^master

Without this patch, these refs are assumed to be deleted, or worse, they
are rewritten to some bogus merge bases.

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	On Tue, 24 Jul 2007, Johannes Sixt wrote:

	> Johannes Schindelin wrote:
	> 
	> > +# NEEDSWORK: we should sort the unmapped refs topologically first
	> > +while read ref
	> > +do
	> > +       sha1=$(git rev-parse "$ref"^0)
	> > +       test -f "$workdir"/../map/$sha1 && continue
	> > +       # Assign the boundarie(s) in the set of rewritten commits
	> > +       # as the replacement commit(s).
	> > +       # (This would look a bit nicer if --not --stdin worked.)
	> > +       for p in $((cd "$workdir"/../map; ls | sed "s/^/^/") |
	> > +               git rev-list $ref --boundary --stdin |
	> > +               sed -n "s/^-//p")
	> > +       do
	> > +               map $p >> "$workdir"/../map/$sha1
	> > +       done
	> > +done < "$tempdir"/heads
	> 
	> This logic seems to be borked, and I don't grok it. I was trying this:
	> 
	>    git-filter-branch -- --since=2007.01.10 \
	> 		refs/heads/topic refs/heads/master

	Actually, the error lay somewhere else.  If excluded, topic should 
	not have been a rewrite candidate.

	There might be something else looming here: what to do with

		git filter-branch --subdir-filter subdir/ topic master \
			^$(git merge-base topic master)

	when topic has no changes to the subdirectory, but master has?

	I guess the safest would be to 'test -f "$workdir"/../map/$p', and 
	if at least one is not there, leave 'topic' as is.  If at least 
	one is there, however, we want to warn the user that this is not 
	possible.  (Something like: this was an unchanged ancestor: $p1, 
	but that was a changed one: $p2.  Make up your mind what you 
	want.)

 git-filter-branch.sh     |    9 ++++++++-
 t/t7003-filter-branch.sh |    9 +++++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 019a302..49b0f00 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -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
+
+	echo "$ref"
 done > "$tempdir"/heads
 
 test -s "$tempdir"/heads ||
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index c9a820d..667eda7 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -164,4 +164,13 @@ test_expect_success 'only dwim refs/heads/$ref or refs/tags/$ref' '
 	git filter-branch -f master
 '
 
+test_expect_success 'do not update uninteresting refs' '
+	branch=$(git rev-parse branch) &&
+	master=$(git rev-parse master) &&
+	git filter-branch -f --env-filter "GIT_AUTHOR_EMAIL=xy@probl.em" \
+		master ^removed-author branch &&
+	test $branch = $(git rev-parse branch) &&
+	test $master != $(git rev-parse master)
+'
+
 test_done
-- 
1.5.3.rc2.32.g35c5b-dirty

Re: [PATCH] filter-branch: when dwim'ing a ref, only allow heads andtags

[Johannes Schindelin]

Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

> Johannes Schindelin wrote:
> > -                       grep /"$ref")"
> > +                       grep "refs/\(tags\|heads\)/$ref$")"
> 
> Thanks, but I don't think that this is a portable grep pattern:
> 
> $ echo "abc" | grep '\(a\|x\)'
> $ echo "abc" | egrep '(a|x)'
> abc
> 
> But then, this is on some AIX 4.3. How about:
> 
>    fgrep -e "refs/heads/$ref" -e "refs/tags/$ref"
> or
>    grep -e "^refs/heads/$ref" -e "^refs/tags/$ref"

Ah yes, I remember that Junio said some time ago that "\|" is not 
portable.

But to be portable, let's stay with grep, and not use some fancy stuff 
like egrep or fgrep.

Ciao,
Dscho

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

[Johannes Sixt]

Johannes Schindelin wrote:
> 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>

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

-- Hannes

[PATCH] filter-branch: when dwim'ing a ref, only allow heads and tags

[Johannes Schindelin]

Previously, we matched all refs that had /$ref in them.  And tried to
verify the result as a ref.  If more than one match was found, the result
was not a ref, though.

So only allow tags and heads to be dwim'ed.  If both a tag and a head with
that name exist, it will be ignored again.

Caught by Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Heh.  I just realised that I managed to fsck up a one-line patch.  
	D'oh!  Thanks for fixing it, Hannes.

 git-filter-branch.sh     |    2 +-
 t/t7003-filter-branch.sh |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 0ff3475..4fb3abe 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -193,7 +193,7 @@ do
 	;;
 	*)
 		ref="$(git for-each-ref --format='%(refname)' |
-			grep /"$ref")"
+			grep -e "^refs/heads/$ref$" -e "^refs/tags/$ref$")"
 	esac
 
 	git check-ref-format "$ref" && echo "$ref"
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index bc6e2dd..c9a820d 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -159,4 +159,9 @@ test_expect_success 'barf on invalid name' '
 	! git filter-branch -f HEAD^
 '
 
+test_expect_success 'only dwim refs/heads/$ref or refs/tags/$ref' '
+	git update-ref refs/remotes/origin/master HEAD &&
+	git filter-branch -f master
+'
+
 test_done
-- 
1.5.3.rc2.32.g35c5b-dirty

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

[Johannes Schindelin]

Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

> Johannes Schindelin wrote:
> > 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

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

Ciao,
Dscho

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

[Johannes Sixt]

Johannes Schindelin wrote:
> 
> Hi,
> 
> On Tue, 24 Jul 2007, Johannes Sixt wrote:
> 
> > Johannes Schindelin wrote:
> > > 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.

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

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

[Johannes Sixt]

Johannes Schindelin wrote:
> negatives="$(git rev-parse "$@" | while read line
>         do
>                 case "$line" in
>                 $_x40) ;;
>                 *) echo "$line";;
>                 esac
>         done)"

Ah, and you must move the definition of _x40 up a couple of lines.

-- Hannes

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

[Johannes Schindelin]

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

[REVISED PATCH] filter-branch: rewrite only unexcluded refs

[Johannes Schindelin]

The rev-list options can exclude some -- or all -- refs:

	git filter-branch <some-filter> master ^master

Without this patch, these refs are assumed to be deleted, or worse, they
are rewritten to some bogus merge bases.

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.

Thanks go to Johannes Sixt for help on the implementation and for testing.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-filter-branch.sh     |   16 +++++++++++++++-
 t/t7003-filter-branch.sh |    9 +++++++++
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 4fb3abe..3fa2f63 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -181,6 +181,14 @@ export GIT_DIR GIT_WORK_TREE=.
 
 # These refs should be updated if their heads were rewritten
 
+negatives="`git rev-parse "$@" | while read line
+	do
+		case "$line" in
+		$_x40) ;;
+		*) echo "$line";;
+		esac
+	done`"
+
 git rev-parse --revs-only --symbolic "$@" |
 while read ref
 do
@@ -196,7 +204,13 @@ do
 			grep -e "^refs/heads/$ref$" -e "^refs/tags/$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 --topo-order)" && continue
+
+	echo "$ref"
 done > "$tempdir"/heads
 
 test -s "$tempdir"/heads ||
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index c9a820d..667eda7 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -164,4 +164,13 @@ test_expect_success 'only dwim refs/heads/$ref or refs/tags/$ref' '
 	git filter-branch -f master
 '
 
+test_expect_success 'do not update uninteresting refs' '
+	branch=$(git rev-parse branch) &&
+	master=$(git rev-parse master) &&
+	git filter-branch -f --env-filter "GIT_AUTHOR_EMAIL=xy@probl.em" \
+		master ^removed-author branch &&
+	test $branch = $(git rev-parse branch) &&
+	test $master != $(git rev-parse master)
+'
+
 test_done
-- 
1.5.3.rc2.32.g35c5b-dirty

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

[Johannes Schindelin]

Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

> Johannes Schindelin wrote:
> > negatives="$(git rev-parse "$@" | while read line
> >         do
> >                 case "$line" in
> >                 $_x40) ;;
> >                 *) echo "$line";;
> >                 esac
> >         done)"
> 
> Ah, and you must move the definition of _x40 up a couple of lines.

D'oh.  Of course.

When I have more tests, and thought about the problem I hinted at in 
another mail, I'll send a cleaned up patch series.  Promised.  Rebase -i 
is good.

Ciao,
Dscho

---

 git-filter-branch.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 3fa2f63..8107a6c 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -11,6 +11,9 @@
 USAGE="git-filter-branch [-d TEMPDIR] [FILTERS] DESTBRANCH [REV-RANGE]"
 . git-sh-setup
 
+_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
+_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
+
 warn () {
         echo "$*" >&2
 }
@@ -322,8 +325,6 @@ done < "$tempdir"/heads
 
 # Finally update the refs
 
-_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
-_x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 count=0
 echo
 while read ref

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

[Johannes Sixt]

Johannes Schindelin wrote:
> On Tue, 24 Jul 2007, Johannes Sixt wrote:
> > 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?

No. filter-branch is a shell script. I don't have time to waste ;)
It happens in bash 2.05b on Linux.

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

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

Fair enough.

-- Hannes

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

[Johannes Schindelin]

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

?

I really should add a test case for that.

Ciao,
Dscho

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

[Johannes Sixt]

Johannes Schindelin wrote:
> 
> 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

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

[Johannes Schindelin]

Hi,

On Tue, 24 Jul 2007, Johannes Sixt wrote:

> Johannes Schindelin wrote:
> 
> > 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

Okay, but with out any subdirs,

	git rev-list "$@" | grep $(git rev-parse "$ref") > /dev/null || continue

should do exactly what we want.  But in that case, we could just use the 
generated list ../revs to grep for the positive refs.

Darn, this _is_ getting complicated.

Ciao,
Dscho