[PATCH v4 1/2] filter-branch: stop special-casing $filter_subdir argument
- From
Thomas Rast <trast@student.ethz.ch>
- Date
- Nov 10, 2009, 21:04 UTC
- Message-ID
- <0280836a32983c848bbb0e3b441be256d3c8f4fa.1257885121.git.trast@student.ethz.ch>
- In-Reply-To
- <4AE945D0.5030403@viscovery.net>
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:
Show 6 quoted lines
> # we need "--" only if there are no path arguments in $@
> nonrevs=$(git rev-parse --no-revs "$@") || exit
> dashdash=${nonrevs+"--"}
>
> (including the comment; you can drop the dquotes in the dashdash
> assignment if you think the result is still readable.)Ok. I was briefly scared that this was bash-specific syntax, but dash seems happy so I'll trust your advice.
Show 5 quoted lines
> This is not correct: $filter_subdir undergoes an extra level of > evaluation. You must write it like this: > > eval set -- "$(git rev-parse --sq --no-revs "$@" \ > $dashdash "$filter_subdir")"
Hopefully I finally understand what's going on here. The quoting rules make my head spin.
Show 6 quoted lines
> > - ancestor=$(git rev-list --simplify-merges -1 \ > > - $ref -- "$filter_subdir") > > + ancestor=$(git rev-list --simplify-merges -1 "$ref" "$@") > > You added dquotes around $ref: while not absolutely necessary, I agree > with this change.
It's kind of a sneak fix, but I broke this back in the ancestor rewriting patch... Luckily the refname sanity rules are designed to prevent this from causing any harm.
git-filter-branch.sh | 22 +++++++++++++++------- 1 files changed, 15 insertions(+), 7 deletions(-)
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