On Tue, May 10, 2016 at 01:20:54AM -0400, Eric Sunshine wrote:
> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
> index c058aa4..525e6d3 100755
> --- a/t/t1500-rev-parse.sh
> +++ b/t/t1500-rev-parse.sh
> @@ -7,11 +7,13 @@ test_description='test git rev-parse'
> test_rev_parse () {
> dir=
> bare=
> + env=
> while :
> do
> case "$1" in
> -C) dir="-C $2"; shift; shift ;;
> -b) bare="$2"; shift; shift ;;
> + -g) env="GIT_DIR=$2; export GIT_DIR"; shift; shift ;;
This will expand $2 inside $env, which is later eval'd. So funny things
happen if there are spaces or metacharacters. It looks like you only use
it with short relative paths ("../repo.git", etc), which is OK, but this
would probably break badly if we ever used absolute paths.
I don't know if it's worth worrying about or not. The usual solution is
something like:
env_git_dir=$2
env='GIT_DIR=$env_git_dir; export GIT_DIR'
...
eval "$env"
> @@ -36,6 +38,8 @@ test_rev_parse () {
> do
> expect="$1"
> test_expect_success "$name: $o" '
> + test_when_finished "sane_unset GIT_DIR" &&
> + eval $env &&
I was surprised not to see quoting around $env here, but it probably
doesn't matter (I think it may affect how some whitespace is treated,
but the contents of $env are pretty tame).
This will set up the sane_unset regardless of whether $env does
anything. Would it make more sense to stick the test_when_finished
inside $env? You could use regular unset then, too, since you know the
variable would be set.
-Peff
--
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