threads / patch / 21313

patch, 2 partsfilter-branch: nearest-ancestor rewriting outside subdir filter

Subject: [PATCH 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

## tl;dr

22 messages between Oct 21, 2009 and Nov 11, 2009. Diffs are folded; open one to read it.

replies: 21people: 3as markdown or json

Thomas Rast· Oct 21, 2009, 18:16 UTC · lore

[PATCH 1/2] filter-branch: stop special-casing $filter_subdir argument

Handling $filter_subdir in the usual way requires a separate case at every use, because the variable is empty when unused. Furthermore, the case for --subdirectory-filter supplies its own --, so the user cannot provide one himself (though there is also very little point in doing so).

Instead, tack the $filter_subdir onto $@ in the right place automatically, and only use a -- if it was not already provided by the user.

We set non_ref_args again after changing "$@"; the next patch wants to use it again afterwards, so we better not leave a stale value in there.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This is preparatory for the next patch; introducing another 'case' along the lines of the existing one annoyed me, so I went for this instead. I would greatly appreciate extra eyes on my use of 'eval'. I originally expected this to work without eval, but apparently this is how one does it. Quoting rules in the shell are annoying.

Incidentally, the last hunk sneak fixes a previously unquoted use of $ref that is my fault from back in a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12).

 git-filter-branch.sh |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

Show changes to git-filter-branch.sh +21 −7

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..3890c22 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
 # map old->new commit ids for rewriting parents
 mkdir ../map || die "Could not create map/ directory"
 
+non_ref_args=$(git rev-parse --no-revs --sq "$@")
+dashdash=--
+for arg in "$non_ref_args"; do
+	if test arg = --; then
+		dashdash=
+		break
+	fi
+done
+
 case "$filter_subdir" in
 "")
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@"
+	filter_subdir_sq=
 	;;
 *)
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@" -- "$filter_subdir"
-esac > ../revs || die "Could not get the commits"
+	filter_subdir_sq=$(git rev-parse --sq-quote "$filter_subdir")
+esac
+
+eval "set -- \"\$@\" $dashdash $filter_subdir_sq"
+non_ref_args=$(git rev-parse --no-revs --sq "$@")
+
+git rev-list --reverse --topo-order --default HEAD \
+	--parents --simplify-merges "$@" \
+	> ../revs || die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -356,8 +370,8 @@ then
 	do
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
-		ancestor=$(git rev-list --simplify-merges -1 \
-				$ref -- "$filter_subdir")
+		ancestor=$(eval "git rev-list --simplify-merges " \
+				"-1 \"$ref\" $non_ref_args")
 		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi

-- 
1.6.5.1.139.g12527

Thomas Rast· Oct 21, 2009, 18:16 UTC · re: Thomas Rast · lore

Since a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12) git-filter-branch has done nearest-ancestor rewriting when using a --subdirectory-filter.

However, that rewriting strategy is also a useful building block in other tasks. For example, if you want to split out a subset of files from your history, you would typically call

  git filter-branch <refs> -- -- <files>

But this fails for all refs that do not point directly to a commit that affects <files>, because their referenced commit will not be rewritten and the ref remains untouched.

The code was already there for the --subdirectory-filter case, so just introduce an option that enables it independently.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This came up on IRC the other day (see a pattern?), when someone wanted to split out the history for a single file, and first had to point all relevant refs at the corresponding nearest relevant ancestor.

I think we could even make this option the default if it wasn't for backwards compatibility; after all, what use is rewriting the commits if you do not get a ref pointing to them?

 Documentation/git-filter-branch.txt |   13 ++++++++++++-
 git-filter-branch.sh                |    7 ++++++-
 t/t7003-filter-branch.sh            |   18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

Show changes to 3 files +36 −2

Documentation/git-filter-branch.txt, git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 2b40bab..394a77a 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -159,7 +159,18 @@ to other tags will be rewritten to point to the underlying commit.
 --subdirectory-filter <directory>::
 	Only look at the history which touches the given subdirectory.
 	The result will contain that directory (and only that) as its
-	project root.
+	project root.  Implies --remap-to-ancestor.
+
+--remap-to-ancestor::
+	Rewrite refs to the nearest rewritten ancestor instead of
+	ignoring them.
++
+Normally, positive refs on the command line are only changed if the
+commit they point to was rewritten.  However, you can limit the extent
+of this rewriting by using linkgit:rev-list[1] arguments, e.g., path
+limiters.  Refs pointing to such excluded commits would then normally
+be ignored.  With this option, they are instead rewritten to point at
+the nearest ancestor that was not excluded.
 
 --prune-empty::
 	Some kind of filters will generate empty commits, that left the tree
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 3890c22..d18f82d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -125,6 +125,7 @@ filter_subdir=
 orig_namespace=refs/original/
 force=
 prune_empty=
+remap_to_ancestor=
 while :
 do
 	case "$1" in
@@ -182,10 +183,14 @@ do
 		;;
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
+		remap_to_ancestor=t
 		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
 		;;
+	--remap-to-ancestor)
+		remap_to_ancestor=t
+		;;
 	*)
 		usage
 		;;
@@ -364,7 +369,7 @@ done <../revs
 # revision walker.  Fix it by mapping these heads to the unique nearest
 # ancestor that survived the pruning.
 
-if test "$filter_subdir"
+if test "$remap_to_ancestor" = t
 then
 	while read ref
 	do
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 329c851..9503875 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -288,4 +288,22 @@ test_expect_success 'Prune empty commits' '
 	test_cmp expect actual
 '
 
+test_expect_success '--remap-to-ancestor with filename filters' '
+	git checkout master &&
+	git reset --hard A &&
+	test_commit add-foo foo 1 &&
+	git branch moved-foo &&
+	test_commit add-bar bar a &&
+	git branch invariant &&
+	orig_invariant=$(git rev-parse invariant) &&
+	git branch moved-bar &&
+	test_commit change-foo foo 2 &&
+	git filter-branch -f --remap-to-ancestor \
+		moved-foo moved-bar A..master \
+		-- -- foo &&
+	test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
+	test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
+	test $orig_invariant = $(git rev-parse invariant)
+'
+
 test_done

-- 
1.6.5.1.139.g12527

Thomas Rast· Oct 21, 2009, 18:28 UTC · re: Thomas Rast · lore

[PATCH v2 1/2] filter-branch: stop special-casing $filter_subdir argument

Instead, tack the $filter_subdir onto $@ in the right place automatically, and only use a -- if it was not already provided by the user.

We set non_ref_args again after changing "$@"; the next patch wants to use it again afterwards, so we better not leave a stale value in there.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

[Same as v1.]

Incidentally, the last hunk sneak fixes a previously unquoted use of $ref that is my fault from back in a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12).

 git-filter-branch.sh |   28 +++++++++++++++++++++-------
 1 files changed, 21 insertions(+), 7 deletions(-)

Show changes to git-filter-branch.sh +21 −7

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..3890c22 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
 # map old->new commit ids for rewriting parents
 mkdir ../map || die "Could not create map/ directory"
 
+non_ref_args=$(git rev-parse --no-revs --sq "$@")
+dashdash=--
+for arg in "$non_ref_args"; do
+	if test arg = --; then
+		dashdash=
+		break
+	fi
+done
+
 case "$filter_subdir" in
 "")
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@"
+	filter_subdir_sq=
 	;;
 *)
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@" -- "$filter_subdir"
-esac > ../revs || die "Could not get the commits"
+	filter_subdir_sq=$(git rev-parse --sq-quote "$filter_subdir")
+esac
+
+eval "set -- \"\$@\" $dashdash $filter_subdir_sq"
+non_ref_args=$(git rev-parse --no-revs --sq "$@")
+
+git rev-list --reverse --topo-order --default HEAD \
+	--parents --simplify-merges "$@" \
+	> ../revs || die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -356,8 +370,8 @@ then
 	do
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
-		ancestor=$(git rev-list --simplify-merges -1 \
-				$ref -- "$filter_subdir")
+		ancestor=$(eval "git rev-list --simplify-merges " \
+				"-1 \"$ref\" $non_ref_args")
 		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi

-- 
1.6.5.1.142.g4bac9

Thomas Rast· Oct 21, 2009, 18:28 UTC · re: Thomas Rast · lore

[PATCH v2 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Since a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12) git-filter-branch has done nearest-ancestor rewriting when using a --subdirectory-filter.

However, that rewriting strategy is also a useful building block in other tasks. For example, if you want to split out a subset of files from your history, you would typically call

  git filter-branch -- <refs> -- <files>

But this fails for all refs that do not point directly to a commit that affects <files>, because their referenced commit will not be rewritten and the ref remains untouched.

The code was already there for the --subdirectory-filter case, so just introduce an option that enables it independently.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Evidently I shouldn't send any patches after dinner, or before, for that matter (but for lack of wifi in the restaurant, *during* dinner is not an option either). Or at least I shouldn't re-read them after sending :-(

v1 had a completely misplaced option parsing for the new option. Very embarrassing. Really.

I also sneak fixed the commit message above; you only need two -- if you want rev-list options, e.g.,

  git filter-branch -- --all -- README

in which case the first one of course needs to appear before the first <refs> argument.

 Documentation/git-filter-branch.txt |   13 ++++++++++++-
 git-filter-branch.sh                |    9 ++++++++-
 t/t7003-filter-branch.sh            |   18 ++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

Show changes to 3 files +38 −2

Documentation/git-filter-branch.txt, git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 2b40bab..394a77a 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -159,7 +159,18 @@ to other tags will be rewritten to point to the underlying commit.
 --subdirectory-filter <directory>::
 	Only look at the history which touches the given subdirectory.
 	The result will contain that directory (and only that) as its
-	project root.
+	project root.  Implies --remap-to-ancestor.
+
+--remap-to-ancestor::
+	Rewrite refs to the nearest rewritten ancestor instead of
+	ignoring them.
++
+Normally, positive refs on the command line are only changed if the
+commit they point to was rewritten.  However, you can limit the extent
+of this rewriting by using linkgit:rev-list[1] arguments, e.g., path
+limiters.  Refs pointing to such excluded commits would then normally
+be ignored.  With this option, they are instead rewritten to point at
+the nearest ancestor that was not excluded.
 
 --prune-empty::
 	Some kind of filters will generate empty commits, that left the tree
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 3890c22..be36db4 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -125,6 +125,7 @@ filter_subdir=
 orig_namespace=refs/original/
 force=
 prune_empty=
+remap_to_ancestor=
 while :
 do
 	case "$1" in
@@ -137,6 +138,11 @@ do
 		force=t
 		continue
 		;;
+	--remap-to-ancestor)
+		shift
+		remap_to_ancestor=t
+		continue
+		;;
 	--prune-empty)
 		shift
 		prune_empty=t
@@ -182,6 +188,7 @@ do
 		;;
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
+		remap_to_ancestor=t
 		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
@@ -364,7 +371,7 @@ done <../revs
 # revision walker.  Fix it by mapping these heads to the unique nearest
 # ancestor that survived the pruning.
 
-if test "$filter_subdir"
+if test "$remap_to_ancestor" = t
 then
 	while read ref
 	do
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 329c851..9503875 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -288,4 +288,22 @@ test_expect_success 'Prune empty commits' '
 	test_cmp expect actual
 '
 
+test_expect_success '--remap-to-ancestor with filename filters' '
+	git checkout master &&
+	git reset --hard A &&
+	test_commit add-foo foo 1 &&
+	git branch moved-foo &&
+	test_commit add-bar bar a &&
+	git branch invariant &&
+	orig_invariant=$(git rev-parse invariant) &&
+	git branch moved-bar &&
+	test_commit change-foo foo 2 &&
+	git filter-branch -f --remap-to-ancestor \
+		moved-foo moved-bar A..master \
+		-- -- foo &&
+	test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
+	test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
+	test $orig_invariant = $(git rev-parse invariant)
+'
+
 test_done

-- 
1.6.5.1.142.g4bac9

Johannes Sixt· Oct 22, 2009, 06:06 UTC · re: Thomas Rast · lore

Re: [PATCH v2 1/2] filter-branch: stop special-casing $filter_subdir argument

Thomas Rast schrieb:

Show 9 quoted lines

> Handling $filter_subdir in the usual way requires a separate case at
> every use, because the variable is empty when unused.  Furthermore,
> the case for --subdirectory-filter supplies its own --, so the user
> cannot provide one himself (though there is also very little point in
> doing so).
> 
> Instead, tack the $filter_subdir onto $@ in the right place
> automatically, and only use a -- if it was not already provided by the
> user.

I understand that this is a preparatory patch, but you seem to argue that even without the follow-up patch there is a problem. But from your explanation I do not understand what it is. An example invocation that shows the problem would be very helpful.

Show 7 quoted lines

> @@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
>  # map old->new commit ids for rewriting parents
>  mkdir ../map || die "Could not create map/ directory"
>  
> +non_ref_args=$(git rev-parse --no-revs --sq "$@")
> +dashdash=--
> +for arg in "$non_ref_args"; do

At this point $non_ref_args might contain one or more IFS-separatable words, but if you say "$non_ref_args" here, this loop will be entered exactly once. But even if you drop the dquotes, the --sq quoting that you requested from rev-parse bought you nothing.

> +	if test arg = --; then

Did you mean $arg here? Even then this test will succeed only if $non_ref_args contains exactly one word and that word is '--'. Is that what you mean?

Show 38 quoted lines

> +		dashdash=
> +		break
> +	fi
> +done
> +
>  case "$filter_subdir" in
>  "")
> -	git rev-list --reverse --topo-order --default HEAD \
> -		--parents --simplify-merges "$@"
> +	filter_subdir_sq=
>  	;;
>  *)
> -	git rev-list --reverse --topo-order --default HEAD \
> -		--parents --simplify-merges "$@" -- "$filter_subdir"
> -esac > ../revs || die "Could not get the commits"
> +	filter_subdir_sq=$(git rev-parse --sq-quote "$filter_subdir")
> +esac
> +
> +eval "set -- \"\$@\" $dashdash $filter_subdir_sq"
> +non_ref_args=$(git rev-parse --no-revs --sq "$@")
> +
> +git rev-list --reverse --topo-order --default HEAD \
> +	--parents --simplify-merges "$@" \
> +	> ../revs || die "Could not get the commits"
>  commits=$(wc -l <../revs | tr -d " ")
>  
>  test $commits -eq 0 && die "Found nothing to rewrite"
> @@ -356,8 +370,8 @@ then
>  	do
>  		sha1=$(git rev-parse "$ref"^0)
>  		test -f "$workdir"/../map/$sha1 && continue
> -		ancestor=$(git rev-list --simplify-merges -1 \
> -				$ref -- "$filter_subdir")
> +		ancestor=$(eval "git rev-list --simplify-merges " \
> +				"-1 \"$ref\" $non_ref_args")
>  		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
>  	done < "$tempdir"/heads
>  fi

This looks so convoluted; there must be a simpler way to achieve your goal.

-- Hannes

Thomas Rast· Oct 22, 2009, 08:05 UTC · re: Johannes Sixt · lore

Re: [PATCH v2 1/2] filter-branch: stop special-casing $filter_subdir argument

Johannes Sixt wrote:

Show 11 quoted lines

> Thomas Rast schrieb:
> > Handling $filter_subdir in the usual way requires a separate case at
> > every use, because the variable is empty when unused.  Furthermore,
> > the case for --subdirectory-filter supplies its own --, so the user
> > cannot provide one himself (though there is also very little point in
> > doing so).
> 
> I understand that this is a preparatory patch, but you seem to argue that
> even without the follow-up patch there is a problem. But from your
> explanation I do not understand what it is. An example invocation that
> shows the problem would be very helpful.

Well, I just observed while writing the patch that you cannot say

  git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

because the --subdirectory-filter supplies its own -- to the rev-list invocation, i.e., it calls

  git rev-list --all -- subdir/file -- subdir

which filters for a file called --.

I doubt anyone ever needed this operation though, and it can easily be done in two separate filtering steps.

Show 12 quoted lines

> > @@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
> >  # map old->new commit ids for rewriting parents
> >  mkdir ../map || die "Could not create map/ directory"
> >  
> > +non_ref_args=$(git rev-parse --no-revs --sq "$@")
> > +dashdash=--
> > +for arg in "$non_ref_args"; do
> 
> At this point $non_ref_args might contain one or more IFS-separatable
> words, but if you say "$non_ref_args" here, this loop will be entered
> exactly once. But even if you drop the dquotes, the --sq quoting that you
> requested from rev-parse bought you nothing.

Hrm. Ok, so the ".." were clearly in mistake, but why could I remove the --sq? Doesn't the shell expand the arguments provided by $non_ref_args if I use it without quotes nor --sq, so that it might accidentally expand paths or such?

Show 5 quoted lines

> > +	if test arg = --; then
> 
> Did you mean $arg here? Even then this test will succeed only if
> $non_ref_args contains exactly one word and that word is '--'. Is that
> what you mean?

No, it should find a -- argument to see if we need to supply our own before the $filter_subdir.

> This looks so convoluted; there must be a simpler way to achieve your goal.

I'll try with more 'case's later... maybe I can at least avoid the eval.

Thanks for your comments!

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Johannes Sixt· Oct 22, 2009, 08:31 UTC · re: Thomas Rast · lore

Re: [PATCH v2 1/2] filter-branch: stop special-casing $filter_subdir argument

Thomas Rast schrieb:

> Well, I just observed while writing the patch that you cannot say
> 
>   git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

Please include this in the commit message.

Show 18 quoted lines

> Johannes Sixt wrote:
>> Thomas Rast schrieb:
>>> @@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
>>>  # map old->new commit ids for rewriting parents
>>>  mkdir ../map || die "Could not create map/ directory"
>>>  
>>> +non_ref_args=$(git rev-parse --no-revs --sq "$@")
>>> +dashdash=--
>>> +for arg in "$non_ref_args"; do
>> At this point $non_ref_args might contain one or more IFS-separatable
>> words, but if you say "$non_ref_args" here, this loop will be entered
>> exactly once. But even if you drop the dquotes, the --sq quoting that you
>> requested from rev-parse bought you nothing.
> 
> Hrm.  Ok, so the ".." were clearly in mistake, but why could I remove
> the --sq?  Doesn't the shell expand the arguments provided by
> $non_ref_args if I use it without quotes nor --sq, so that it might
> accidentally expand paths or such?

When the shell expands $variable (outside quotes), it does not apply quotes anymore, but only word-splits using $IFS. In your code, the words would contain literal single-quotes, and paths with spaces would still be split into words.

Wouldn't it be sufficient to just check whether any non-rev arguments are present, and to suppress '--' if there are, like:

dashdash= test -z "$(git rev-parse --no-revs "$@")" && dashdash=--

OK, this still leaves you with the problem that you want to separate non-rev arguments from rev arguments. Right?

For this I suggest that you extract revs into a regular variable (because the SHA1s can be word-split in a predictable way), and that you leave the non-rev arguments in $@:

revs=$(git rev-parse --revs "$@") # don't know if this works eval set -- "$(git rev-parse --no-revs --sq "$@")" # dquotes?

or so... This way you 'eval' should not be needed in later code.

-- Hannes

Thomas Rast· Oct 28, 2009, 22:59 UTC · re: Johannes Sixt · lore

[PATCH v3 1/2] filter-branch: stop special-casing $filter_subdir argument

Handling $filter_subdir in the usual way requires a separate case at every use, because the variable is empty when unused.

Furthermore, the case for --subdirectory-filter supplies its own --, so the user cannot provide one himself, so the following was impossible:

  git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

To keep the argument handling sane, we filter $@ to contain only the non-revision arguments, and store all revisions in $ref_args. The $ref_args are easy to handle since only the SHA1s are needed; the actual branch names have already been stored in $tempdir/heads at this point.

An extra separating -- is only required if the user did not provide any non-revision arguments, as the latter disambiguate the $filter_subdir following after them (or fail earlier because they are ambiguous themselves).

Thanks to Johannes Sixt for suggesting this solution.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Johannes Sixt wrote:

> When the shell expands $variable (outside quotes), it does not apply
> quotes anymore, but only word-splits using $IFS. In your code, the words
> would contain literal single-quotes, and paths with spaces would still be
> split into words.

If there's a good reason for these weird rules, I'm still missing it...

But your suggestion works very nicely.

 git-filter-branch.sh |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

Show changes to git-filter-branch.sh +14 −7

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..da23b99 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -257,15 +257,23 @@ git read-tree || die "Could not seed the index"
 # map old->new commit ids for rewriting parents
 mkdir ../map || die "Could not create map/ directory"
 
+dashdash=
+test -z "$(git rev-parse --no-revs "$@")" && dashdash=--
+ref_args=$(git rev-parse --revs-only "$@")
+
 case "$filter_subdir" in
 "")
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@"
+	eval set -- "$(git rev-parse --sq --no-revs "$@")"
 	;;
 *)
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@" -- "$filter_subdir"
-esac > ../revs || die "Could not get the commits"
+	eval set -- "$(git rev-parse --sq --no-revs "$@")" \
+	    $dashdash "$filter_subdir"
+	;;
+esac
+
+git rev-list --reverse --topo-order --default HEAD \
+	--parents --simplify-merges $ref_args "$@" \
+	> ../revs || die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -356,8 +364,7 @@ then
 	do
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
-		ancestor=$(git rev-list --simplify-merges -1 \
-				$ref -- "$filter_subdir")
+		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")
 		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi

-- 
1.6.5.1.161.g3b9c0

Thomas Rast· Oct 28, 2009, 22:59 UTC · re: Thomas Rast · lore

[PATCH v3 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Since a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12) git-filter-branch has done nearest-ancestor rewriting when using a --subdirectory-filter.

However, that rewriting strategy is also a useful building block in other tasks. For example, if you want to split out a subset of files from your history, you would typically call

  git filter-branch -- <refs> -- <files>

But this fails for all refs that do not point directly to a commit that affects <files>, because their referenced commit will not be rewritten and the ref remains untouched.

The code was already there for the --subdirectory-filter case, so just introduce an option that enables it independently.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Same as v2.

 Documentation/git-filter-branch.txt |   13 ++++++++++++-
 git-filter-branch.sh                |    9 ++++++++-
 t/t7003-filter-branch.sh            |   18 ++++++++++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)

Show changes to 3 files +38 −2

Documentation/git-filter-branch.txt, git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 2b40bab..394a77a 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -159,7 +159,18 @@ to other tags will be rewritten to point to the underlying commit.
 --subdirectory-filter <directory>::
 	Only look at the history which touches the given subdirectory.
 	The result will contain that directory (and only that) as its
-	project root.
+	project root.  Implies --remap-to-ancestor.
+
+--remap-to-ancestor::
+	Rewrite refs to the nearest rewritten ancestor instead of
+	ignoring them.
++
+Normally, positive refs on the command line are only changed if the
+commit they point to was rewritten.  However, you can limit the extent
+of this rewriting by using linkgit:rev-list[1] arguments, e.g., path
+limiters.  Refs pointing to such excluded commits would then normally
+be ignored.  With this option, they are instead rewritten to point at
+the nearest ancestor that was not excluded.
 
 --prune-empty::
 	Some kind of filters will generate empty commits, that left the tree
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index da23b99..ad2bc6f 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -125,6 +125,7 @@ filter_subdir=
 orig_namespace=refs/original/
 force=
 prune_empty=
+remap_to_ancestor=
 while :
 do
 	case "$1" in
@@ -137,6 +138,11 @@ do
 		force=t
 		continue
 		;;
+	--remap-to-ancestor)
+		shift
+		remap_to_ancestor=t
+		continue
+		;;
 	--prune-empty)
 		shift
 		prune_empty=t
@@ -182,6 +188,7 @@ do
 		;;
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
+		remap_to_ancestor=t
 		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
@@ -358,7 +365,7 @@ done <../revs
 # revision walker.  Fix it by mapping these heads to the unique nearest
 # ancestor that survived the pruning.
 
-if test "$filter_subdir"
+if test "$remap_to_ancestor" = t
 then
 	while read ref
 	do
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 329c851..9503875 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -288,4 +288,22 @@ test_expect_success 'Prune empty commits' '
 	test_cmp expect actual
 '
 
+test_expect_success '--remap-to-ancestor with filename filters' '
+	git checkout master &&
+	git reset --hard A &&
+	test_commit add-foo foo 1 &&
+	git branch moved-foo &&
+	test_commit add-bar bar a &&
+	git branch invariant &&
+	orig_invariant=$(git rev-parse invariant) &&
+	git branch moved-bar &&
+	test_commit change-foo foo 2 &&
+	git filter-branch -f --remap-to-ancestor \
+		moved-foo moved-bar A..master \
+		-- -- foo &&
+	test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
+	test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
+	test $orig_invariant = $(git rev-parse invariant)
+'
+
 test_done

-- 
1.6.5.1.161.g3b9c0

Johannes Sixt· Oct 29, 2009, 07:38 UTC · re: Thomas Rast · lore

Re: [PATCH v3 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Thomas Rast schrieb:

Show 7 quoted lines

> @@ -358,7 +365,7 @@ done <../revs
>  # revision walker.  Fix it by mapping these heads to the unique nearest
>  # ancestor that survived the pruning.
>  
> -if test "$filter_subdir"
> +if test "$remap_to_ancestor" = t
>  then

The comment whose last lines are in the context of this hunk talks about "subdirectory filter". You may want to revise it.

-- Hannes

Johannes Sixt· Oct 29, 2009, 07:35 UTC · re: Thomas Rast · lore

Re: [PATCH v3 1/2] filter-branch: stop special-casing $filter_subdir argument

Thomas Rast schrieb:

Show 10 quoted lines

> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index a480d6f..da23b99 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -257,15 +257,23 @@ git read-tree || die "Could not seed the index"
>  # map old->new commit ids for rewriting parents
>  mkdir ../map || die "Could not create map/ directory"
>  
> +dashdash=
> +test -z "$(git rev-parse --no-revs "$@")" && dashdash=--

Hmm, if the user runs

   $ git filter-branch does-not-exist

then this would be the first call to rev-parse that fails with "fatal: ambiguous argument 'does-not-exist': unknown revision or path...", but this code wouldn't catch the failure. I suggest this instead:

# we need "--" only if there are no path arguments in $@ nonrevs=$(git rev-parse --no-revs "$@") || exit dashdash=${nonrevs+"--"}

(including the comment; you can drop the dquotes in the dashdash assignment if you think the result is still readable.)

I was to suggest to move these lines into the case arm below that is the only user of $dashdash. But since this is a good error check of the user-supplied arguments, I now prefer to keep the lines outside the case statement.

> +ref_args=$(git rev-parse --revs-only "$@")

"ref_args"? Shouldn't it be "rev_args"? It's about "revisions", not "references".

Show 12 quoted lines

>  case "$filter_subdir" in
>  "")
> -	git rev-list --reverse --topo-order --default HEAD \
> -		--parents --simplify-merges "$@"
> +	eval set -- "$(git rev-parse --sq --no-revs "$@")"
>  	;;
>  *)
> -	git rev-list --reverse --topo-order --default HEAD \
> -		--parents --simplify-merges "$@" -- "$filter_subdir"
> -esac > ../revs || die "Could not get the commits"
> +	eval set -- "$(git rev-parse --sq --no-revs "$@")" \
> +	    $dashdash "$filter_subdir"

This is not correct: $filter_subdir undergoes an extra level of evaluation. You must write it like this:

	eval set -- "$(git rev-parse --sq --no-revs "$@" \
		$dashdash "$filter_subdir")"

Show 6 quoted lines

> +	;;
> +esac
> +
> +git rev-list --reverse --topo-order --default HEAD \
> +	--parents --simplify-merges $ref_args "$@" \
> +	> ../revs || die "Could not get the commits"

Personally, I would prefer to write this as

git rev-list --reverse --topo-order --default HEAD \
	--parents --simplify-merges $ref_args "$@" > ../revs ||
	die "Could not get the commits"

to avoid one backslash.

Show 7 quoted lines

> @@ -356,8 +364,7 @@ then
>  	do
>  		sha1=$(git rev-parse "$ref"^0)
>  		test -f "$workdir"/../map/$sha1 && continue
> -		ancestor=$(git rev-list --simplify-merges -1 \
> -				$ref -- "$filter_subdir")
> +		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")

You added dquotes around $ref: while not absolutely necessary, I agree with this change.

>  		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
>  	done < "$tempdir"/heads
>  fi

-- Hannes

Thomas Rast· Nov 10, 2009, 21:04 UTC · re: Johannes Sixt · lore

[PATCH v4 1/2] filter-branch: stop special-casing $filter_subdir argument

Handling $filter_subdir in the usual way requires a separate case at every use, because the variable is empty when unused.

Furthermore, the case for --subdirectory-filter supplies its own --, so the user cannot provide one himself, so the following was impossible:

  git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

Thanks to Johannes Sixt for suggesting this solution.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Thanks once again for your reviews. It's been a while since I last looked into this, but after some staring, I think you're right on all counts.

Johannes Sixt wrote:

Show 6 quoted lines

> # we need "--" only if there are no path arguments in $@
> nonrevs=$(git rev-parse --no-revs "$@") || exit
> dashdash=${nonrevs+"--"}
> 
> (including the comment; you can drop the dquotes in the dashdash
> assignment if you think the result is still readable.)

Ok. I was briefly scared that this was bash-specific syntax, but dash seems happy so I'll trust your advice.

Show 5 quoted lines

> This is not correct: $filter_subdir undergoes an extra level of
> evaluation. You must write it like this:
> 
> 	eval set -- "$(git rev-parse --sq --no-revs "$@" \
> 		$dashdash "$filter_subdir")"

Hopefully I finally understand what's going on here. The quoting rules make my head spin.

Show 6 quoted lines

> > -		ancestor=$(git rev-list --simplify-merges -1 \
> > -				$ref -- "$filter_subdir")
> > +		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")
> 
> You added dquotes around $ref: while not absolutely necessary, I agree
> with this change.

It's kind of a sneak fix, but I broke this back in the ancestor rewriting patch... Luckily the refname sanity rules are designed to prevent this from causing any harm.

 git-filter-branch.sh |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

Show changes to git-filter-branch.sh +15 −7

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..96b2182 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -257,15 +257,24 @@ git read-tree || die "Could not seed the index"
 # map old->new commit ids for rewriting parents
 mkdir ../map || die "Could not create map/ directory"
 
+# we need "--" only if there are no path arguments in $@
+nonrevs=$(git rev-parse --no-revs "$@") || exit
+dashdash=${nonrevs+"--"}
+rev_args=$(git rev-parse --revs-only "$@")
+
 case "$filter_subdir" in
 "")
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@"
+	eval set -- "$(git rev-parse --sq --no-revs "$@")"
 	;;
 *)
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@" -- "$filter_subdir"
-esac > ../revs || die "Could not get the commits"
+	eval set -- "$(git rev-parse --sq --no-revs "$@" $dashdash \
+		"$filter_subdir")"
+	;;
+esac
+
+git rev-list --reverse --topo-order --default HEAD \
+	--parents --simplify-merges $rev_args "$@" > ../revs ||
+	die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")
 
 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -356,8 +365,7 @@ then
 	do
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
-		ancestor=$(git rev-list --simplify-merges -1 \
-				$ref -- "$filter_subdir")
+		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")
 		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi

-- 
1.6.5.2.365.ge9237

Thomas Rast· Nov 10, 2009, 21:04 UTC · re: Thomas Rast · lore

[PATCH v4 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Since a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12) git-filter-branch has done nearest-ancestor rewriting when using a --subdirectory-filter.

However, that rewriting strategy is also a useful building block in other tasks. For example, if you want to split out a subset of files from your history, you would typically call

  git filter-branch -- <refs> -- <files>

But this fails for all refs that do not point directly to a commit that affects <files>, because their referenced commit will not be rewritten and the ref remains untouched.

The code was already there for the --subdirectory-filter case, so just introduce an option that enables it independently.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Compared to v3 this only changes the comment, as suggested by Johannes Sixt.

 Documentation/git-filter-branch.txt |   13 ++++++++++++-
 git-filter-branch.sh                |   18 +++++++++++++-----
 t/t7003-filter-branch.sh            |   18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 6 deletions(-)

Show changes to 3 files +43 −6

Documentation/git-filter-branch.txt, git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 2b40bab..394a77a 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -159,7 +159,18 @@ to other tags will be rewritten to point to the underlying commit.
 --subdirectory-filter <directory>::
 	Only look at the history which touches the given subdirectory.
 	The result will contain that directory (and only that) as its
-	project root.
+	project root.  Implies --remap-to-ancestor.
+
+--remap-to-ancestor::
+	Rewrite refs to the nearest rewritten ancestor instead of
+	ignoring them.
++
+Normally, positive refs on the command line are only changed if the
+commit they point to was rewritten.  However, you can limit the extent
+of this rewriting by using linkgit:rev-list[1] arguments, e.g., path
+limiters.  Refs pointing to such excluded commits would then normally
+be ignored.  With this option, they are instead rewritten to point at
+the nearest ancestor that was not excluded.
 
 --prune-empty::
 	Some kind of filters will generate empty commits, that left the tree
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 96b2182..a5a0352 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -125,6 +125,7 @@ filter_subdir=
 orig_namespace=refs/original/
 force=
 prune_empty=
+remap_to_ancestor=
 while :
 do
 	case "$1" in
@@ -137,6 +138,11 @@ do
 		force=t
 		continue
 		;;
+	--remap-to-ancestor)
+		shift
+		remap_to_ancestor=t
+		continue
+		;;
 	--prune-empty)
 		shift
 		prune_empty=t
@@ -182,6 +188,7 @@ do
 		;;
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
+		remap_to_ancestor=t
 		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
@@ -354,12 +361,13 @@ while read commit parents; do
 			die "could not write rewritten commit"
 done <../revs
 
-# 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 unique nearest
-# ancestor that survived the pruning.
+# If we are filtering for paths, as in the 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.
+# Ancestor remapping fixes this by mapping these heads to the unique
+# nearest ancestor that survived the pruning.
 
-if test "$filter_subdir"
+if test "$remap_to_ancestor" = t
 then
 	while read ref
 	do
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 329c851..9503875 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -288,4 +288,22 @@ test_expect_success 'Prune empty commits' '
 	test_cmp expect actual
 '
 
+test_expect_success '--remap-to-ancestor with filename filters' '
+	git checkout master &&
+	git reset --hard A &&
+	test_commit add-foo foo 1 &&
+	git branch moved-foo &&
+	test_commit add-bar bar a &&
+	git branch invariant &&
+	orig_invariant=$(git rev-parse invariant) &&
+	git branch moved-bar &&
+	test_commit change-foo foo 2 &&
+	git filter-branch -f --remap-to-ancestor \
+		moved-foo moved-bar A..master \
+		-- -- foo &&
+	test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
+	test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
+	test $orig_invariant = $(git rev-parse invariant)
+'
+
 test_done

-- 
1.6.5.2.365.ge9237

Johannes Sixt· Nov 11, 2009, 08:30 UTC · re: Thomas Rast · lore

Re: [PATCH v4 1/2] filter-branch: stop special-casing $filter_subdir argument

Thomas Rast schrieb:

Show 10 quoted lines

> Johannes Sixt wrote:
>> # we need "--" only if there are no path arguments in $@
>> nonrevs=$(git rev-parse --no-revs "$@") || exit
>> dashdash=${nonrevs+"--"}
>>
>> (including the comment; you can drop the dquotes in the dashdash
>> assignment if you think the result is still readable.)
> 
> Ok.  I was briefly scared that this was bash-specific syntax, but dash
> seems happy so I'll trust your advice.

You shouldn't. I'm wrong ;-)

In the snippet above, dashdash will always be set to "--" because a mere '+' in the variable expansion only tests whether the variable ('nonrevs') is unset, but it is always set. Even ${nonrevs:+"--"} is wrong, and your earlier 'test -z' invocation was the correct way to set dashdash.

But...

I did some testing, and now I have to wonder about the use-case that you present in the commit message:

Show 5 quoted lines

> Furthermore, the case for --subdirectory-filter supplies its own --,
> so the user cannot provide one himself, so the following was
> impossible:
>
>   git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

If you try in git.git itself:

  $ git rev-list --simplify-merges --all -- \
	Documentation/git-commit.txt | wc -l
  84
  $ git rev-list --simplify-merges --all -- \
	Documentation/git-commit.txt Documentation | wc -l
  4624

I thought that the intention to give an extra path argument is to reduce the number of commits that remain in the rewritten history. But by giving --subdirectory-filter, the path filter actually loosened, and many more commits are rewritten.

Since your intention to write this patch is actually to implement --remap-to-ancestor, I suggest that we defer the question whether the above use-case makes sense, and only rewrite this particular paragraph in the commit message to point out the real bug:

----- Furthermore, --subdirectory-filter supplies its own '--', and if the user provided one himself, such as in

  git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

an extra '--' was used as path filter in the call to git-rev-list that determines the commits that shall be rewritten. -----

I'll submit a replacement patch.

-- Hannes

Johannes Sixt· Nov 11, 2009, 08:53 UTC · re: Johannes Sixt · lore

[PATCH v5 1/2] filter-branch: stop special-casing $filter_subdir argument

From: Thomas Rast <trast@student.ethz.ch>

Handling $filter_subdir in the usual way requires a separate case at every use, because the variable is empty when unused.

Furthermore, --subdirectory-filter supplies its own '--', and if the user provided one himself, such as in

  git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

	an extra '--' was used as path filter in the call to git-rev-list that
determines the commits that shall be rewritten.

Thanks to Johannes Sixt for suggesting this solution.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Johannes Sixt schrieb:

> I'll submit a replacement patch.

Here it is. The interdiff to your version is merely

Show changes to git-filter-branch.sh +16 −9

--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -266,7 +266,7 @@

 # we need "--" only if there are no path arguments in $@
 nonrevs=$(git rev-parse --no-revs "$@") || exit
-dashdash=${nonrevs+"--"}
+test -z "$nonrevs" && dashdash=-- || dashdash=
 rev_args=$(git rev-parse --revs-only "$@")

 case "$filter_subdir" in

and I changed the commit message in the way I announced it.

-- Hannes

 git-filter-branch.sh |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index a480d6f..ed3db7d 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -257,15 +257,24 @@ git read-tree || die "Could not seed the index"
 # map old->new commit ids for rewriting parents
 mkdir ../map || die "Could not create map/ directory"

+# we need "--" only if there are no path arguments in $@
+nonrevs=$(git rev-parse --no-revs "$@") || exit
+test -z "$nonrevs" && dashdash=-- || dashdash=
+rev_args=$(git rev-parse --revs-only "$@")
+
 case "$filter_subdir" in
 "")
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@"
+	eval set -- "$(git rev-parse --sq --no-revs "$@")"
 	;;
 *)
-	git rev-list --reverse --topo-order --default HEAD \
-		--parents --simplify-merges "$@" -- "$filter_subdir"
-esac > ../revs || die "Could not get the commits"
+	eval set -- "$(git rev-parse --sq --no-revs "$@" $dashdash \
+		"$filter_subdir")"
+	;;
+esac
+
+git rev-list --reverse --topo-order --default HEAD \
+	--parents --simplify-merges $rev_args "$@" > ../revs ||
+	die "Could not get the commits"
 commits=$(wc -l <../revs | tr -d " ")

 test $commits -eq 0 && die "Found nothing to rewrite"
@@ -356,8 +365,7 @@ then
 	do
 		sha1=$(git rev-parse "$ref"^0)
 		test -f "$workdir"/../map/$sha1 && continue
-		ancestor=$(git rev-list --simplify-merges -1 \
-				$ref -- "$filter_subdir")
+		ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@")
 		test "$ancestor" && echo $(map $ancestor) >> "$workdir"/../map/$sha1
 	done < "$tempdir"/heads
 fi

-- 
1.6.5.rc2.47.g49402

Johannes Sixt· Nov 11, 2009, 08:55 UTC · re: Johannes Sixt · lore

[PATCH v5 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

From: Thomas Rast <trast@student.ethz.ch>
Date: Tue, 10 Nov 2009 22:04:51 +0100

Since a0e4639 (filter-branch: fix ref rewriting with --subdirectory-filter, 2008-08-12) git-filter-branch has done nearest-ancestor rewriting when using a --subdirectory-filter.

However, that rewriting strategy is also a useful building block in other tasks. For example, if you want to split out a subset of files from your history, you would typically call

  git filter-branch -- <refs> -- <files>

But this fails for all refs that do not point directly to a commit that affects <files>, because their referenced commit will not be rewritten and the ref remains untouched.

The code was already there for the --subdirectory-filter case, so just introduce an option that enables it independently.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---

Only resent for Junio's convenience. It is identical to your v4, but with my SOB added.

-- Hannes

 Documentation/git-filter-branch.txt |   13 ++++++++++++-
 git-filter-branch.sh                |   18 +++++++++++++-----
 t/t7003-filter-branch.sh            |   18 ++++++++++++++++++
 3 files changed, 43 insertions(+), 6 deletions(-)

Show changes to 3 files +43 −6

Documentation/git-filter-branch.txt, git-filter-branch.sh, t/t7003-filter-branch.sh

diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index 2b40bab..394a77a 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -159,7 +159,18 @@ to other tags will be rewritten to point to the underlying commit.
 --subdirectory-filter <directory>::
 	Only look at the history which touches the given subdirectory.
 	The result will contain that directory (and only that) as its
-	project root.
+	project root.  Implies --remap-to-ancestor.
+
+--remap-to-ancestor::
+	Rewrite refs to the nearest rewritten ancestor instead of
+	ignoring them.
++
+Normally, positive refs on the command line are only changed if the
+commit they point to was rewritten.  However, you can limit the extent
+of this rewriting by using linkgit:rev-list[1] arguments, e.g., path
+limiters.  Refs pointing to such excluded commits would then normally
+be ignored.  With this option, they are instead rewritten to point at
+the nearest ancestor that was not excluded.

 --prune-empty::
 	Some kind of filters will generate empty commits, that left the tree
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index ed3db7d..6b8b6a4 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -125,6 +125,7 @@ filter_subdir=
 orig_namespace=refs/original/
 force=
 prune_empty=
+remap_to_ancestor=
 while :
 do
 	case "$1" in
@@ -137,6 +138,11 @@ do
 		force=t
 		continue
 		;;
+	--remap-to-ancestor)
+		shift
+		remap_to_ancestor=t
+		continue
+		;;
 	--prune-empty)
 		shift
 		prune_empty=t
@@ -182,6 +188,7 @@ do
 		;;
 	--subdirectory-filter)
 		filter_subdir="$OPTARG"
+		remap_to_ancestor=t
 		;;
 	--original)
 		orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
@@ -354,12 +361,13 @@ while read commit parents; do
 			die "could not write rewritten commit"
 done <../revs

-# 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 unique nearest
-# ancestor that survived the pruning.
+# If we are filtering for paths, as in the 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.
+# Ancestor remapping fixes this by mapping these heads to the unique
+# nearest ancestor that survived the pruning.

-if test "$filter_subdir"
+if test "$remap_to_ancestor" = t
 then
 	while read ref
 	do
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 329c851..9503875 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -288,4 +288,22 @@ test_expect_success 'Prune empty commits' '
 	test_cmp expect actual
 '

+test_expect_success '--remap-to-ancestor with filename filters' '
+	git checkout master &&
+	git reset --hard A &&
+	test_commit add-foo foo 1 &&
+	git branch moved-foo &&
+	test_commit add-bar bar a &&
+	git branch invariant &&
+	orig_invariant=$(git rev-parse invariant) &&
+	git branch moved-bar &&
+	test_commit change-foo foo 2 &&
+	git filter-branch -f --remap-to-ancestor \
+		moved-foo moved-bar A..master \
+		-- -- foo &&
+	test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
+	test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
+	test $orig_invariant = $(git rev-parse invariant)
+'
+
 test_done

-- 
1.6.5.rc2.47.g49402

Junio C Hamano· Nov 11, 2009, 18:22 UTC · re: Johannes Sixt · lore

Re: [PATCH v5 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 15 quoted lines

> However, that rewriting strategy is also a useful building block in
> other tasks.  For example, if you want to split out a subset of files
> from your history, you would typically call
>
>   git filter-branch -- <refs> -- <files>
>
> But this fails for all refs that do not point directly to a commit
> that affects <files>, because their referenced commit will not be
> rewritten and the ref remains untouched.
>
> The code was already there for the --subdirectory-filter case, so just
> introduce an option that enables it independently.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Up to this point, nothing mentions the name of the new option. Please write your log for a person who needs to write the release notes by looking at "git shortlog" output ;-)

I am wondering if this even needs an option to trigger. Shouldn't you want this behaviour always when you give any pathspec?

What are the sane reasons to leave the rewritten ref to point at the old commit, essentially making the rewritten history unreachable?

Thomas Rast· Nov 11, 2009, 18:36 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 2/2] filter-branch: nearest-ancestor rewriting outside subdir filter

Junio C Hamano wrote:

Show 10 quoted lines

> 
> Up to this point, nothing mentions the name of the new option.  Please
> write your log for a person who needs to write the release notes by
> looking at "git shortlog" output ;-)
> 
> I am wondering if this even needs an option to trigger.  Shouldn't you
> want this behaviour always when you give any pathspec?
> 
> What are the sane reasons to leave the rewritten ref to point at the old
> commit, essentially making the rewritten history unreachable?

Fear of hysterical raisins, nothing else. I cannot really see a use-case of the original behaviour, and since it does not change the behaviour in the case of an unfiltered rewrite, it should be safe to always be enabled.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Johannes Sixt· Nov 11, 2009, 08:58 UTC · re: Johannes Sixt · lore

Re: [PATCH v5 1/2] filter-branch: stop special-casing $filter_subdir argument

Johannes Sixt schrieb:

Show 11 quoted lines

> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -266,7 +266,7 @@
> 
>  # we need "--" only if there are no path arguments in $@
>  nonrevs=$(git rev-parse --no-revs "$@") || exit
> -dashdash=${nonrevs+"--"}
> +test -z "$nonrevs" && dashdash=-- || dashdash=
>  rev_args=$(git rev-parse --revs-only "$@")
> 
>  case "$filter_subdir" in

Arrgh! I should have indented these lines so that they do not count as patch text.

Sorry.

-- Hannes

Thomas Rast· Nov 11, 2009, 10:24 UTC · re: Johannes Sixt · lore

Re: [PATCH v5 1/2] filter-branch: stop special-casing $filter_subdir argument

Johannes Sixt wrote in the other reply:

> I thought that the intention to give an extra path argument is to reduce
> the number of commits that remain in the rewritten history. But by giving
> --subdirectory-filter, the path filter actually loosened, and many more
> commits are rewritten.

Right, I had a thinko there, the path filter adds up as an "or", so filtering for paths outside the subdir loosens it (and filtering for more paths inside doesn't make a difference).

> Since your intention to write this patch is actually to implement
> --remap-to-ancestor, I suggest that we defer the question whether the
> above use-case makes sense, and only rewrite this particular paragraph in
> the commit message to point out the real bug:

Agreed.

Johannes Sixt wrote:

Show 7 quoted lines

> Furthermore, --subdirectory-filter supplies its own '--', and if the user
> provided one himself, such as in
> 
>   git filter-branch --subdirectory-filter subdir -- --all -- subdir/file
> 
> 	an extra '--' was used as path filter in the call to git-rev-list that
> determines the commits that shall be rewritten.

There's some stray space here that should probably also be removed.

> Here it is. The interdiff to your version is merely

[...]

>  # we need "--" only if there are no path arguments in $@
>  nonrevs=$(git rev-parse --no-revs "$@") || exit
> -dashdash=${nonrevs+"--"}
> +test -z "$nonrevs" && dashdash=-- || dashdash=

Ack.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Johannes Sixt· Nov 11, 2009, 12:10 UTC · re: Thomas Rast · lore

Re: [PATCH v5 1/2] filter-branch: stop special-casing $filter_subdir argument

Thomas Rast schrieb:

Show 10 quoted lines

> Johannes Sixt wrote:
>> Furthermore, --subdirectory-filter supplies its own '--', and if the user
>> provided one himself, such as in
>>
>>   git filter-branch --subdirectory-filter subdir -- --all -- subdir/file
>>
>> 	an extra '--' was used as path filter in the call to git-rev-list that
>> determines the commits that shall be rewritten.
> 
> There's some stray space here that should probably also be removed.

Oh, right. Junio, you can pick up the 2-patch series from

  git://repo.or.cz/git/mingw/j6t.git filter-branch

-- Hannes

Junio C Hamano· Nov 11, 2009, 18:00 UTC · re: Johannes Sixt · lore

Re: [PATCH v4 1/2] filter-branch: stop special-casing $filter_subdir argument

Johannes Sixt <j.sixt@viscovery.net> writes:

> In the snippet above, dashdash will always be set to "--" because a mere
> '+' in the variable expansion only tests whether the variable ('nonrevs')
> is unset, but it is always set. Even ${nonrevs:+"--"} is wrong, and your
> earlier 'test -z' invocation was the correct way to set dashdash.

I do not mind "test -z", so this is just for information, but you could use variable substitution with a colon, i.e. ${nonrevs:+"--"}.

IIRC, the colon-form ("do this if unset or empty") wasn't understood by ancient BSDs but these days it is safe to use.