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

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

[Thomas Rast]

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(-)

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

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

[Thomas Rast]

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(-)

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

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

[Thomas Rast]

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

[Same as v1.]

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(-)

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

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

[Thomas Rast]

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(-)

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

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

[Johannes Sixt]

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

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

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

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

[Thomas Rast]

Johannes Sixt wrote:
> 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.

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

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

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

[Johannes Sixt]

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.

> 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

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

[Thomas Rast]

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(-)

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

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

[Thomas Rast]

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(-)

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

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

[Johannes Sixt]

Thomas Rast schrieb:
> @@ -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

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

[Johannes Sixt]

Thomas Rast schrieb:
> 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".

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

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

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

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

[Thomas Rast]

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

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

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

> > -		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(-)

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

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

[Thomas Rast]

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(-)

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

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

[Johannes Sixt]

Thomas Rast schrieb:
> 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:

> 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

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

[Johannes Sixt]

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.

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

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

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

[Johannes Sixt]

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(-)

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

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

[Junio C Hamano]

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

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

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

[Thomas Rast]

Junio C Hamano wrote:
> 
> 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

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

[Johannes Sixt]

Johannes Sixt schrieb:
> --- 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

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

[Thomas Rast]

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

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

[Johannes Sixt]

Thomas Rast schrieb:
> 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

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

[Junio C Hamano]

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.