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@xxxxxxxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html