Re: [PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead

2013-01-07 Thread René Scharfe 

Am 07.01.2013 06:16, schrieb Jonathan Nieder:

René Scharfe wrote:


InfoZIP's unzip takes default parameters from the environment variable
UNZIP.  Unset it in the test library and use GIT_UNZIP for specifying
alternate versions of the unzip command instead.

t0024 wasn't even using variable for the actual extraction.  t5000
was, but when setting it to InfoZIP's unzip it would try to extract
from itself (because it treats the contents of $UNZIP as parameters),
which failed of course.


That would only happen if the UNZIP variable was already exported,
right?


We don't want any parameters a user may have been specified influence 
the test.  I'm not sure if someone actually sets that variable for that 
purpose, though.


My main use case is running individual test scripts with an alternative 
unzip binary, and with the patch this works as expected:


$ cd t
$ GIT_UNZIP=/usr/pkg/bin/unzip ./t5000-tar-tree.sh


The patch makes sense and takes care of all uses of ${UNZIP} I can
find, and it even makes the quoting consistent so a person can put
their copy of unzip under "/Program Files".  For what it's worth,

Reviewed-by: Jonathan Nieder 


Thanks!

René

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead

2013-01-06 Thread Jonathan Nieder 
René Scharfe wrote:

> InfoZIP's unzip takes default parameters from the environment variable
> UNZIP.  Unset it in the test library and use GIT_UNZIP for specifying
> alternate versions of the unzip command instead.
>
> t0024 wasn't even using variable for the actual extraction.  t5000
> was, but when setting it to InfoZIP's unzip it would try to extract
> from itself (because it treats the contents of $UNZIP as parameters),
> which failed of course.

That would only happen if the UNZIP variable was already exported,
right?

The patch makes sense and takes care of all uses of ${UNZIP} I can
find, and it even makes the quoting consistent so a person can put
their copy of unzip under "/Program Files".  For what it's worth,

Reviewed-by: Jonathan Nieder 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead

2013-01-06 Thread René Scharfe 
InfoZIP's unzip takes default parameters from the environment variable
UNZIP.  Unset it in the test library and use GIT_UNZIP for specifying
alternate versions of the unzip command instead.

t0024 wasn't even using variable for the actual extraction.  t5000
was, but when setting it to InfoZIP's unzip it would try to extract
from itself (because it treats the contents of $UNZIP as parameters),
which failed of course.

Signed-off-by: Rene Scharfe 
---
 t/t0024-crlf-archive.sh |  6 +++---
 t/t5000-tar-tree.sh | 10 +-
 t/test-lib.sh   |  2 ++
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index ec6c1b3..080fe5c 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -3,7 +3,7 @@
 test_description='respect crlf in git archive'
 
 . ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
+GIT_UNZIP=${GIT_UNZIP:-unzip}
 
 test_expect_success setup '
 
@@ -26,7 +26,7 @@ test_expect_success 'tar archive' '
 
 '
 
-"$UNZIP" -v >/dev/null 2>&1
+"$GIT_UNZIP" -v >/dev/null 2>&1
 if [ $? -eq 127 ]; then
say "Skipping ZIP test, because unzip was not found"
 else
@@ -37,7 +37,7 @@ test_expect_success UNZIP 'zip archive' '
 
git archive --format=zip HEAD >test.zip &&
 
-   ( mkdir unzipped && cd unzipped && unzip ../test.zip ) &&
+   ( mkdir unzipped && cd unzipped && "$GIT_UNZIP" ../test.zip ) &&
 
test_cmp sample unzipped/sample
 
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..1f7593d 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,7 @@ commit id embedding:
 '
 
 . ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
+GIT_UNZIP=${GIT_UNZIP:-unzip}
 GZIP=${GZIP:-gzip}
 GUNZIP=${GUNZIP:-gzip -d}
 
@@ -37,9 +37,9 @@ check_zip() {
dir=$1
dir_with_prefix=$dir/$2
 
-   test_expect_success UNZIP " extract ZIP archive" "
-   (mkdir $dir && cd $dir && $UNZIP ../$zipfile)
-   "
+   test_expect_success UNZIP " extract ZIP archive" '
+   (mkdir $dir && cd $dir && "$GIT_UNZIP" ../$zipfile)
+   '
 
test_expect_success UNZIP " validate filenames" "
(cd ${dir_with_prefix}a && find .) | sort >$listfile &&
@@ -201,7 +201,7 @@ test_expect_success \
   test_cmp a/substfile2 g/prefix/a/substfile2
 '
 
-$UNZIP -v >/dev/null 2>&1
+"$GIT_UNZIP" -v >/dev/null 2>&1
 if [ $? -eq 127 ]; then
say "Skipping ZIP tests, because unzip was not found"
 else
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8a12cbb..d8ec408 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -85,6 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
.*_TEST
PROVE
VALGRIND
+   UNZIP
PERF_AGGREGATING_LATER
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
@@ -128,6 +129,7 @@ fi
 unset CDPATH
 
 unset GREP_OPTIONS
+unset UNZIP
 
 case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
 1|2|true)
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html