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

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

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

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

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

[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

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

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

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.

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?

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

-- Hannes

Johannes Sixt wrote:

Thomas Rast schrieb:

quoted

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.

quoted

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

quoted

+	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

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:

quoted

Thomas Rast schrieb:

quoted

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

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

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

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

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

Thomas Rast schrieb:

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.

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

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

-- Hannes

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

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.

quoted

-		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

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

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

Thomas Rast schrieb:

Johannes Sixt wrote:

quoted

# 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

From: Thomas Rast <redacted>

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 <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
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

From: Thomas Rast <redacted>
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 <redacted>
Signed-off-by: Johannes Sixt <redacted>
---

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

Johannes Sixt schrieb:

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

Sorry.

-- Hannes

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

Thomas Rast schrieb:

Johannes Sixt wrote:

quoted

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