{"thread":{"id":"32453","subject":"[LHF] making t5000 \"tar xf\" tests more lenient","startedAt":"2012-12-24T20:49:10Z","lastAt":"2013-01-10T07:36:29Z","messageCount":23,"participants":["Junio C Hamano","René Scharfe","Jonathan Nieder","Matt Kraai"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"205479","messageId":"7vwqw7mb09.fsf@alter.siamese.dyndns.org","threadId":"32453","inReplyTo":null,"subject":"[LHF] making t5000 \"tar xf\" tests more lenient","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-24T20:49:10Z","receivedAt":"2012-12-24T20:49:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I've been running testsuite on a few platforms that are unfamiliar\nto me, and was bitten by BSD implementation of tar that do not grok\nthe extended pax headers. I've already fixed one in t9502 [*1*]\nwhere we produce a tarball with \"git archive\" and then try to\nvalidate it with the platform implementation of \"tar\" so that the\ntest won't barf when it sees the extended pax header.\n\nt5000 is littered with similar tests that break unnecessarily with\nBSD implementations. I think what it wants to test are:\n\n - \"archive\" produces tarball, and \"tar\" can extract from it. If\n your \"tar\" does not understand pax header, you may get it as an\n extra file that shouldn't be there, but that should not cause the\n test to fail---in real life, people without a pax-aware \"tar\"\n will just ignore and remove the header and can go on.\n\n - \"get-tar-commmit-id\" can inspect a tarball produced by \"archive\"\n to recover the object name of the commit even on a platform\n without a pax-aware \"tar\".\n\nPerhaps t5000 can be restructured like so:\n\n - create a tarball with the commit-id and test with\n \"get-tar-commit-id\" to validate it; also create a tarball out of\n a tree to make sure it does not have commit-id and check with\n \"get-tar-commit-id\". Do this on any and all platform, even on\n the ones without a pax-aware \"tar\".\n\n - check platform implementation of \"tar\" early to see if extracting\n a simple output from \"git archive\" results in an extra pax header\n file. If so, remember this fact and produce any and all tarballs\n used in the remainder of the test by forcing ^{tree}.\n\nso that people on platforms without pax-aware \"tar\" do not have to\ninstall GNU tar only to pass this test.\n\nIt would be a good exercise during the holiday week for somebody on\nBSD (it seems NetBSD is more troublesome than OpenBSD) to come up\nwith a patch to help users on these platforms.\n\nThanks.\n\n\n[Footnote]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/211803\n"},{"id":"206038","messageId":"50E8722B.8010408@lsrfire.ath.cx","threadId":"32453","inReplyTo":"7vwqw7mb09.fsf@alter.siamese.dyndns.org","subject":"Re: [LHF] making t5000 \"tar xf\" tests more lenient","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-05T18:34:19Z","receivedAt":"2013-01-05T18:34:19Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.12.2012 21:49, schrieb Junio C Hamano:\n> I've been running testsuite on a few platforms that are unfamiliar\n> to me, and was bitten by BSD implementation of tar that do not grok\n> the extended pax headers. I've already fixed one in t9502 [*1*]\n> where we produce a tarball with \"git archive\" and then try to\n> validate it with the platform implementation of \"tar\" so that the\n> test won't barf when it sees the extended pax header.\n> \n> t5000 is littered with similar tests that break unnecessarily with\n> BSD implementations. I think what it wants to test are:\n> \n> - \"archive\" produces tarball, and \"tar\" can extract from it. If\n> your \"tar\" does not understand pax header, you may get it as an\n> extra file that shouldn't be there, but that should not cause the\n> test to fail---in real life, people without a pax-aware \"tar\"\n> will just ignore and remove the header and can go on.\n> \n> - \"get-tar-commmit-id\" can inspect a tarball produced by \"archive\"\n> to recover the object name of the commit even on a platform\n> without a pax-aware \"tar\".\n> \n> Perhaps t5000 can be restructured like so:\n> \n> - create a tarball with the commit-id and test with\n> \"get-tar-commit-id\" to validate it; also create a tarball out of\n> a tree to make sure it does not have commit-id and check with\n> \"get-tar-commit-id\". Do this on any and all platform, even on\n> the ones without a pax-aware \"tar\".\n> \n> - check platform implementation of \"tar\" early to see if extracting\n> a simple output from \"git archive\" results in an extra pax header\n> file. If so, remember this fact and produce any and all tarballs\n> used in the remainder of the test by forcing ^{tree}.\n> \n> so that people on platforms without pax-aware \"tar\" do not have to\n> install GNU tar only to pass this test.\n> \n> It would be a good exercise during the holiday week for somebody on\n> BSD (it seems NetBSD is more troublesome than OpenBSD) to come up\n> with a patch to help users on these platforms.\n\nI got around to building a virtual machine with NetBSD 6.0.1, which\ninvolved a bit of cursing because networking only seems to work when I\nturn off ACPI and SMP -- and running tests is a lot more pleasant with\nmore than one CPU.\n\nAnyway, I don't think the pax headers are to blame here. The patch below\nfixes the tar failures for me, but I'm not sure why. There must be\nsomething special about some (not all!) directory entries with trailing\nslashes after directory names.\n\nSeveral ZIP tests still fail, however, because NetBSD's unzip doesn't\nseem to support (our flavour of) symlinks and streamed files.\n\nI'll take a deeper look into it over the weekend.\n\nRené\n\n\n---\n archive-tar.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 0ba3f25..fd2d3e8 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -215,6 +215,8 @@ static int write_tar_entry(struct archiver_args *args,\n \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\t*header.typeflag = TYPEFLAG_DIR;\n \t\tmode = (mode | 0777) & ~tar_umask;\n+\t\tif (pathlen && path[pathlen - 1] == '/')\n+\t\t\tpathlen--;\n \t} else if (S_ISLNK(mode)) {\n \t\t*header.typeflag = TYPEFLAG_LNK;\n \t\tmode |= 0777;\n-- \n1.7.12\n"},{"id":"206041","messageId":"7vwqvrpg9o.fsf@alter.siamese.dyndns.org","threadId":"32453","inReplyTo":"50E8722B.8010408@lsrfire.ath.cx","subject":"Re: [LHF] making t5000 \"tar xf\" tests more lenient","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-05T19:43:31Z","receivedAt":"2013-01-05T19:43:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Anyway, I don't think the pax headers are to blame here. The patch below\n> fixes the tar failures for me, but I'm not sure why. There must be\n> something special about some (not all!) directory entries with trailing\n> slashes after directory names.\n>\n> Several ZIP tests still fail, however, because NetBSD's unzip doesn't\n> seem to support (our flavour of) symlinks and streamed files.\n\nJust FYI, I found that unzip that comes with base distro and the one\nyou can add via pkg_add (or pkgin) are different, and the latter\nseemed to behave better.\n"},{"id":"206043","messageId":"7vsj6fpeys.fsf@alter.siamese.dyndns.org","threadId":"32453","inReplyTo":"50E8722B.8010408@lsrfire.ath.cx","subject":"Re: [LHF] making t5000 \"tar xf\" tests more lenient","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-05T20:11:39Z","receivedAt":"2013-01-05T20:11:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Anyway, I don't think the pax headers are to blame here.\n\nHmph, I am reasonably sure I saw a test that created an archive from\na commit (hence with pax header), asked platform tar to either list\nthe contents or actually extracted to the filesystem, and tried to\nensure nothing but the paths in the repository existed in the\narchive. When the platform tar implementation treated the pax\nheader as an extra file, such a test sees something not in the\nrepository and fails.\n\nThis was on (and I am still on) NetBSD 6.0 not 6.0.1, by the way.\n"},{"id":"206065","messageId":"50E8AE12.8040102@lsrfire.ath.cx","threadId":"32453","inReplyTo":"7vwqw7mb09.fsf@alter.siamese.dyndns.org","subject":"[PATCH] archive-tar: split long paths more carefully","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-05T22:49:54Z","receivedAt":"2013-01-05T22:49:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The name field of a tar header has a size of 100 characters. This limit\nwas extended long ago in a backward compatible way by providing the\nadditional prefix field, which can hold 155 additional characters. The\nactual path is constructed at extraction time by concatenating the prefix\nfield, a slash and the name field.\n\nget_path_prefix() is used to determine which slash in the path is used as\nthe cutting point and thus which part of it is placed into the field\nprefix and which into the field name. It tries to cram as much into the\nprefix field as possible. (And only if we can't fit a path into the\nprovided 255 characters we use a pax extended header to store it.)\n\nIf a path is longer than 100 but shorter than 156 characters and ends\nwith a slash (i.e. is for a directory) then get_path_prefix() puts the\nwhole path in the prefix field and leaves the name field empty. GNU tar\nreconstructs the path without complaint, but the tar included with\nNetBSD 6 does not: It reports the header to be invalid.\n\nFor compatibility with this version of tar, make sure to never leave the\nname field empty. In order to do that, trim the trailing slash from the\npart considered as possible prefix, if it exists -- that way the last\npath component (or more, but not less) will end up in the name field.\n\nSigned-off-by: Rene Scharfe \n---\n archive-tar.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 0ba3f25..923daf5 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -153,6 +153,8 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)\n static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)\n {\n \tsize_t i = pathlen;\n+\tif (i > 1 && path[i - 1] == '/')\n+\t\ti--;\n \tif (i > maxlen)\n \t\ti = maxlen;\n \tdo {\n-- \n1.7.12\n"},{"id":"206066","messageId":"50E8AE19.5030700@lsrfire.ath.cx","threadId":"32453","inReplyTo":"7vsj6fpeys.fsf@alter.siamese.dyndns.org","subject":"Re: [LHF] making t5000 \"tar xf\" tests more lenient","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-05T22:50:01Z","receivedAt":"2013-01-05T22:50:01Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 05.01.2013 21:11, schrieb Junio C Hamano:\n> René Scharfe writes:\n>\n>> Anyway, I don't think the pax headers are to blame here.\n>\n> Hmph, I am reasonably sure I saw a test that created an archive from\n> a commit (hence with pax header), asked platform tar to either list\n> the contents or actually extracted to the filesystem, and tried to\n> ensure nothing but the paths in the repository existed in the\n> archive. When the platform tar implementation treated the pax\n> header as an extra file, such a test sees something not in the\n> repository and fails.\n\nt5000 avoids that issue by comparing only the contents of a \nsubdirectory. The script could do with a little cleanup, in any case. \nMoving ZIP testing into its own file, more explicit pax header file \nhandling, speaking file and directory names, and modern coding style \nwould all be nice. :)\n\nRené\n"},{"id":"206074","messageId":"20130105233106.GA5686@elie.Belkin","threadId":"32453","inReplyTo":"50E8AE12.8040102@lsrfire.ath.cx","subject":"Re: [PATCH] archive-tar: split long paths more carefully","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-05T23:31:06Z","receivedAt":"2013-01-05T23:31:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"René Scharfe wrote:\n\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -153,6 +153,8 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)\n> static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)\n> {\n> \tsize_t i = pathlen;\n> +\tif (i > 1 && path[i - 1] == '/')\n> +\t\ti--;\n\nBeautiful.\n"},{"id":"206113","messageId":"7v7gnqpzrz.fsf@alter.siamese.dyndns.org","threadId":"32453","inReplyTo":"50E8AE12.8040102@lsrfire.ath.cx","subject":"Re: [PATCH] archive-tar: split long paths more carefully","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T06:54:24Z","receivedAt":"2013-01-06T06:54:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> The name field of a tar header has a size of 100 characters. This limit\n> was extended long ago in a backward compatible way by providing the\n> additional prefix field, which can hold 155 additional characters. The\n> actual path is constructed at extraction time by concatenating the prefix\n> field, a slash and the name field.\n>\n> get_path_prefix() is used to determine which slash in the path is used as\n> the cutting point and thus which part of it is placed into the field\n> prefix and which into the field name. It tries to cram as much into the\n> prefix field as possible. (And only if we can't fit a path into the\n> provided 255 characters we use a pax extended header to store it.)\n>\n> If a path is longer than 100 but shorter than 156 characters and ends\n> with a slash (i.e. is for a directory) then get_path_prefix() puts the\n> whole path in the prefix field and leaves the name field empty. GNU tar\n> reconstructs the path without complaint, but the tar included with\n> NetBSD 6 does not: It reports the header to be invalid.\n>\n> For compatibility with this version of tar, make sure to never leave the\n> name field empty. In order to do that, trim the trailing slash from the\n> part considered as possible prefix, if it exists -- that way the last\n> path component (or more, but not less) will end up in the name field.\n\nNicely explained; thanks.\n\nMakes me wonder what we should do for a file inside a directory\nwhose name is 10 bytes long, and whose filename is 120 bytes long,\nthough.\n\nSounds like people on NetBSD are SOL due to the 155+'/'+100 in such\na case and there is nothing we can do, I guess.\n\n> Signed-off-by: Rene Scharfe \n> ---\n> archive-tar.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/archive-tar.c b/archive-tar.c\n> index 0ba3f25..923daf5 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -153,6 +153,8 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)\n> static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)\n> {\n> \tsize_t i = pathlen;\n> +\tif (i > 1 && path[i - 1] == '/')\n> +\t\ti--;\n> \tif (i > maxlen)\n> \t\ti = maxlen;\n> \tdo {\n"},{"id":"206140","messageId":"50E99659.5090702@lsrfire.ath.cx","threadId":"32453","inReplyTo":"7vwqw7mb09.fsf@alter.siamese.dyndns.org","subject":"[PATCH] archive-zip: write uncompressed size into header even with streaming","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T15:20:57Z","receivedAt":"2013-01-06T15:20:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"We record the uncompressed and compressed sizes and the CRC of streamed\nfiles as zero in the local header of the file. The actual values are\nrecorded in an extra data descriptor after the file content, and in the\nusual ZIP directory entry at the end of the archive.\n\nWhile we know the compressed size and the CRC only after we processed\nthe contents, we actually know the uncompressed size right from the\nstart. And for files that we store uncompressed we also already know\ntheir final size.\n\nDo it like InfoZIP's zip and recored the known values, even though they\ncan be reconstructed using the ZIP directory and the data descriptors\nalone. InfoZIP's unzip worked fine before, but NetBSD's version\nactually depends on these fields.\n\nThe uncompressed size is already set by sha1_object_info(). We just\nneed to initialize the compressed size to zero or the uncompressed size\ndepending on the compression method (0 means storing). The CRC was\npropertly initialized already.\n\nSigned-off-by: Rene Scharfe \n---\n archive-zip.c | 7 ++-----\n 1 file changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 55f66b4..d3aef53 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -240,7 +240,7 @@ static int write_zip_entry(struct archiver_args *args,\n \t\t\t(mode & 0111) ? ((mode) << 16) : 0;\n \t\tif (S_ISREG(mode) && args->compression_level != 0 && size > 0)\n \t\t\tmethod = 8;\n-\t\tcompressed_size = size;\n+\t\tcompressed_size = (method == 0) ? size : 0;\n \n \t\tif (S_ISREG(mode) && type == OBJ_BLOB && !args->convert &&\n \t\t size > big_file_threshold) {\n@@ -313,10 +313,7 @@ static int write_zip_entry(struct archiver_args *args,\n \tcopy_le16(header.compression_method, method);\n \tcopy_le16(header.mtime, zip_time);\n \tcopy_le16(header.mdate, zip_date);\n-\tif (flags & ZIP_STREAM)\n-\t\tset_zip_header_data_desc(&header, 0, 0, 0);\n-\telse\n-\t\tset_zip_header_data_desc(&header, size, compressed_size, crc);\n+\tset_zip_header_data_desc(&header, size, compressed_size, crc);\n \tcopy_le16(header.filename_length, pathlen);\n \tcopy_le16(header.extra_length, ZIP_EXTRA_MTIME_SIZE);\n \twrite_or_die(1, &header, ZIP_LOCAL_HEADER_SIZE);\n-- \n1.7.12\n"},{"id":"206158","messageId":"50E9B82D.50005@lsrfire.ath.cx","threadId":"32453","inReplyTo":"7vwqw7mb09.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/4] ZIP test fixes","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T17:45:17Z","receivedAt":"2013-01-06T17:45:17Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Fix a bug in two scripts that call unzip, use the opportunity for a small\ncleanup, move all ZIP related tests out of t5000 and finally skip testing\nsymlinks if unzip doesn't support them.\n\nThe first one allows running t5000 with the unzip from pkgsrc manually on\nNetBSD, which succeeds. The last one -- together with the archive-zip\nstreaming patch I sent earlier today -- makes the ZIP tests succeed on\nthat platform out of the box.\n\nRené\n\n\n t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead\n t0024, t5000: use test_lazy_prereq for UNZIP\n t5000, t5002: move ZIP tests into their own script\n t5002: check if unzip supports symlinks\n\n t/t0024-crlf-archive.sh | 16 +++---\n t/t5000-tar-tree.sh | 71 -----------------------\n t/t5002-archive-zip.sh | 131 +++++++++++++++++++++++++++++++++++++++++++\n t/t5002/infozip-symlinks.zip | Bin 0 -> 328 bytes\n t/test-lib.sh | 2 +\n 5 files changed, 140 insertions(+), 80 deletions(-)\n create mode 100755 t/t5002-archive-zip.sh\n create mode 100644 t/t5002/infozip-symlinks.zip\n\n-- \n1.7.12\n"},{"id":"206159","messageId":"50E9B8CD.2010209@lsrfire.ath.cx","threadId":"32453","inReplyTo":"50E9B82D.50005@lsrfire.ath.cx","subject":"[PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T17:47:57Z","receivedAt":"2013-01-06T17:47:57Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"InfoZIP's unzip takes default parameters from the environment variable\nUNZIP. Unset it in the test library and use GIT_UNZIP for specifying\nalternate versions of the unzip command instead.\n\nt0024 wasn't even using variable for the actual extraction. t5000\nwas, but when setting it to InfoZIP's unzip it would try to extract\nfrom itself (because it treats the contents of $UNZIP as parameters),\nwhich failed of course.\n\nSigned-off-by: Rene Scharfe \n---\n t/t0024-crlf-archive.sh | 6 +++---\n t/t5000-tar-tree.sh | 10 +++++-----\n t/test-lib.sh | 2 ++\n 3 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh\nindex ec6c1b3..080fe5c 100755\n--- a/t/t0024-crlf-archive.sh\n+++ b/t/t0024-crlf-archive.sh\n@@ -3,7 +3,7 @@\n test_description='respect crlf in git archive'\n \n . ./test-lib.sh\n-UNZIP=${UNZIP:-unzip}\n+GIT_UNZIP=${GIT_UNZIP:-unzip}\n \n test_expect_success setup '\n \n@@ -26,7 +26,7 @@ test_expect_success 'tar archive' '\n \n '\n \n-\"$UNZIP\" -v >/dev/null 2>&1\n+\"$GIT_UNZIP\" -v >/dev/null 2>&1\n if [ $? -eq 127 ]; then\n \tsay \"Skipping ZIP test, because unzip was not found\"\n else\n@@ -37,7 +37,7 @@ test_expect_success UNZIP 'zip archive' '\n \n \tgit archive --format=zip HEAD >test.zip &&\n \n-\t( mkdir unzipped && cd unzipped && unzip ../test.zip ) &&\n+\t( mkdir unzipped && cd unzipped && \"$GIT_UNZIP\" ../test.zip ) &&\n \n \ttest_cmp sample unzipped/sample\n \ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex ecf00ed..1f7593d 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -25,7 +25,7 @@ commit id embedding:\n '\n \n . ./test-lib.sh\n-UNZIP=${UNZIP:-unzip}\n+GIT_UNZIP=${GIT_UNZIP:-unzip}\n GZIP=${GZIP:-gzip}\n GUNZIP=${GUNZIP:-gzip -d}\n \n@@ -37,9 +37,9 @@ check_zip() {\n \tdir=$1\n \tdir_with_prefix=$dir/$2\n \n-\ttest_expect_success UNZIP \" extract ZIP archive\" \"\n-\t\t(mkdir $dir && cd $dir && $UNZIP ../$zipfile)\n-\t\"\n+\ttest_expect_success UNZIP \" extract ZIP archive\" '\n+\t\t(mkdir $dir && cd $dir && \"$GIT_UNZIP\" ../$zipfile)\n+\t'\n \n \ttest_expect_success UNZIP \" validate filenames\" \"\n \t\t(cd ${dir_with_prefix}a && find .) | sort >$listfile &&\n@@ -201,7 +201,7 @@ test_expect_success \\\n test_cmp a/substfile2 g/prefix/a/substfile2\n '\n \n-$UNZIP -v >/dev/null 2>&1\n+\"$GIT_UNZIP\" -v >/dev/null 2>&1\n if [ $? -eq 127 ]; then\n \tsay \"Skipping ZIP tests, because unzip was not found\"\n else\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8a12cbb..d8ec408 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -85,6 +85,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n \t\t.*_TEST\n \t\tPROVE\n \t\tVALGRIND\n+\t\tUNZIP\n \t\tPERF_AGGREGATING_LATER\n \t));\n \tmy @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);\n@@ -128,6 +129,7 @@ fi\n unset CDPATH\n \n unset GREP_OPTIONS\n+unset UNZIP\n \n case $(echo $GIT_TRACE |tr \"[A-Z]\" \"[a-z]\") in\n 1|2|true)\n-- \n1.7.12\n"},{"id":"206160","messageId":"50E9B90C.2060200@lsrfire.ath.cx","threadId":"32453","inReplyTo":"50E9B82D.50005@lsrfire.ath.cx","subject":"[PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T17:49:00Z","receivedAt":"2013-01-06T17:49:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This change makes the code smaller and we can put it at the top of\nthe script, its rightful place as setup code.\n\nSigned-off-by: Rene Scharfe \n---\n t/t0024-crlf-archive.sh | 12 +++++-------\n t/t5000-tar-tree.sh | 12 +++++-------\n 2 files changed, 10 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh\nindex 080fe5c..ba397bb 100755\n--- a/t/t0024-crlf-archive.sh\n+++ b/t/t0024-crlf-archive.sh\n@@ -5,6 +5,11 @@ test_description='respect crlf in git archive'\n . ./test-lib.sh\n GIT_UNZIP=${GIT_UNZIP:-unzip}\n \n+test_lazy_prereq UNZIP '\n+\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n+\ttest $? -ne 127\n+'\n+\n test_expect_success setup '\n \n \tgit config core.autocrlf true &&\n@@ -26,13 +31,6 @@ test_expect_success 'tar archive' '\n \n '\n \n-\"$GIT_UNZIP\" -v >/dev/null 2>&1\n-if [ $? -eq 127 ]; then\n-\tsay \"Skipping ZIP test, because unzip was not found\"\n-else\n-\ttest_set_prereq UNZIP\n-fi\n-\n test_expect_success UNZIP 'zip archive' '\n \n \tgit archive --format=zip HEAD >test.zip &&\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 1f7593d..6702157 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -31,6 +31,11 @@ GUNZIP=${GUNZIP:-gzip -d}\n \n SUBSTFORMAT=%H%n\n \n+test_lazy_prereq UNZIP '\n+\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n+\ttest $? -ne 127\n+'\n+\n check_zip() {\n \tzipfile=$1.zip\n \tlistfile=$1.lst\n@@ -201,13 +206,6 @@ test_expect_success \\\n test_cmp a/substfile2 g/prefix/a/substfile2\n '\n \n-\"$GIT_UNZIP\" -v >/dev/null 2>&1\n-if [ $? -eq 127 ]; then\n-\tsay \"Skipping ZIP tests, because unzip was not found\"\n-else\n-\ttest_set_prereq UNZIP\n-fi\n-\n test_expect_success \\\n 'git archive --format=zip' \\\n 'git archive --format=zip HEAD >d.zip'\n-- \n1.7.12\n"},{"id":"206161","messageId":"50E9B9A6.5070400@lsrfire.ath.cx","threadId":"32453","inReplyTo":"50E9B82D.50005@lsrfire.ath.cx","subject":"[PATCH 3/4] t5000, t5002: move ZIP tests into their own script","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T17:51:34Z","receivedAt":"2013-01-06T17:51:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This makes ZIP specific tweaks easier.\n\nSigned-off-by: Rene Scharfe \n---\n t/t5000-tar-tree.sh | 69 ----------------------------\n t/t5002-archive-zip.sh | 119 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 119 insertions(+), 69 deletions(-)\n create mode 100755 t/t5002-archive-zip.sh\n\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 6702157..e7c240f 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -25,37 +25,11 @@ commit id embedding:\n '\n \n . ./test-lib.sh\n-GIT_UNZIP=${GIT_UNZIP:-unzip}\n GZIP=${GZIP:-gzip}\n GUNZIP=${GUNZIP:-gzip -d}\n \n SUBSTFORMAT=%H%n\n \n-test_lazy_prereq UNZIP '\n-\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n-\ttest $? -ne 127\n-'\n-\n-check_zip() {\n-\tzipfile=$1.zip\n-\tlistfile=$1.lst\n-\tdir=$1\n-\tdir_with_prefix=$dir/$2\n-\n-\ttest_expect_success UNZIP \" extract ZIP archive\" '\n-\t\t(mkdir $dir && cd $dir && \"$GIT_UNZIP\" ../$zipfile)\n-\t'\n-\n-\ttest_expect_success UNZIP \" validate filenames\" \"\n-\t\t(cd ${dir_with_prefix}a && find .) | sort >$listfile &&\n-\t\ttest_cmp a.lst $listfile\n-\t\"\n-\n-\ttest_expect_success UNZIP \" validate file contents\" \"\n-\t\tdiff -r a ${dir_with_prefix}a\n-\t\"\n-}\n-\n test_expect_success \\\n 'populate workdir' \\\n 'mkdir a b c &&\n@@ -206,55 +180,12 @@ test_expect_success \\\n test_cmp a/substfile2 g/prefix/a/substfile2\n '\n \n-test_expect_success \\\n- 'git archive --format=zip' \\\n- 'git archive --format=zip HEAD >d.zip'\n-\n-check_zip d\n-\n-test_expect_success \\\n- 'git archive --format=zip in a bare repo' \\\n- '(cd bare.git && git archive --format=zip HEAD) >d1.zip'\n-\n-test_expect_success \\\n- 'git archive --format=zip vs. the same in a bare repo' \\\n- 'test_cmp d.zip d1.zip'\n-\n-test_expect_success 'git archive --format=zip with --output' \\\n- 'git archive --format=zip --output=d2.zip HEAD &&\n- test_cmp d.zip d2.zip'\n-\n-test_expect_success 'git archive with --output, inferring format' '\n-\tgit archive --output=d3.zip HEAD &&\n-\ttest_cmp d.zip d3.zip\n-'\n-\n test_expect_success 'git archive with --output, override inferred format' '\n \tgit archive --format=tar --output=d4.zip HEAD &&\n \ttest_cmp b.tar d4.zip\n '\n \n test_expect_success \\\n- 'git archive --format=zip with prefix' \\\n- 'git archive --format=zip --prefix=prefix/ HEAD >e.zip'\n-\n-check_zip e prefix/\n-\n-test_expect_success 'git archive -0 --format=zip on large files' '\n-\ttest_config core.bigfilethreshold 1 &&\n-\tgit archive -0 --format=zip HEAD >large.zip\n-'\n-\n-check_zip large\n-\n-test_expect_success 'git archive --format=zip on large files' '\n-\ttest_config core.bigfilethreshold 1 &&\n-\tgit archive --format=zip HEAD >large-compressed.zip\n-'\n-\n-check_zip large-compressed\n-\n-test_expect_success \\\n 'git archive --list outside of a git repo' \\\n 'GIT_DIR=some/non-existing/directory git archive --list'\n \ndiff --git a/t/t5002-archive-zip.sh b/t/t5002-archive-zip.sh\nnew file mode 100755\nindex 0000000..ac9c6d4\n--- /dev/null\n+++ b/t/t5002-archive-zip.sh\n@@ -0,0 +1,119 @@\n+#!/bin/sh\n+\n+test_description='git archive --format=zip test'\n+\n+. ./test-lib.sh\n+GIT_UNZIP=${GIT_UNZIP:-unzip}\n+\n+SUBSTFORMAT=%H%n\n+\n+test_lazy_prereq UNZIP '\n+\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n+\ttest $? -ne 127\n+'\n+\n+check_zip() {\n+\tzipfile=$1.zip\n+\tlistfile=$1.lst\n+\tdir=$1\n+\tdir_with_prefix=$dir/$2\n+\n+\ttest_expect_success UNZIP \" extract ZIP archive\" '\n+\t\t(mkdir $dir && cd $dir && \"$GIT_UNZIP\" ../$zipfile)\n+\t'\n+\n+\ttest_expect_success UNZIP \" validate filenames\" \"\n+\t\t(cd ${dir_with_prefix}a && find .) | sort >$listfile &&\n+\t\ttest_cmp a.lst $listfile\n+\t\"\n+\n+\ttest_expect_success UNZIP \" validate file contents\" \"\n+\t\tdiff -r a ${dir_with_prefix}a\n+\t\"\n+}\n+\n+test_expect_success \\\n+ 'populate workdir' \\\n+ 'mkdir a b c &&\n+ echo simple textfile >a/a &&\n+ mkdir a/bin &&\n+ cp /bin/sh a/bin &&\n+ printf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >a/substfile1 &&\n+ printf \"A not substituted O\" >a/substfile2 &&\n+ if test_have_prereq SYMLINKS; then\n+\tln -s a a/l1\n+ else\n+\tprintf %s a > a/l1\n+ fi &&\n+ (p=long_path_to_a_file && cd a &&\n+ for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&\n+ echo text >file_with_long_path) &&\n+ (cd a && find .) | sort >a.lst'\n+\n+test_expect_success \\\n+ 'add ignored file' \\\n+ 'echo ignore me >a/ignored &&\n+ echo ignored export-ignore >.git/info/attributes'\n+\n+test_expect_success \\\n+ 'add files to repository' \\\n+ 'find a -type f | xargs git update-index --add &&\n+ find a -type l | xargs git update-index --add &&\n+ treeid=`git write-tree` &&\n+ echo $treeid >treeid &&\n+ git update-ref HEAD $(TZ=GMT GIT_COMMITTER_DATE=\"2005-05-27 22:00:00\" \\\n+ git commit-tree $treeid d.zip'\n+\n+check_zip d\n+\n+test_expect_success \\\n+ 'git archive --format=zip in a bare repo' \\\n+ '(cd bare.git && git archive --format=zip HEAD) >d1.zip'\n+\n+test_expect_success \\\n+ 'git archive --format=zip vs. the same in a bare repo' \\\n+ 'test_cmp d.zip d1.zip'\n+\n+test_expect_success 'git archive --format=zip with --output' \\\n+ 'git archive --format=zip --output=d2.zip HEAD &&\n+ test_cmp d.zip d2.zip'\n+\n+test_expect_success 'git archive with --output, inferring format' '\n+\tgit archive --output=d3.zip HEAD &&\n+\ttest_cmp d.zip d3.zip\n+'\n+\n+test_expect_success \\\n+ 'git archive --format=zip with prefix' \\\n+ 'git archive --format=zip --prefix=prefix/ HEAD >e.zip'\n+\n+check_zip e prefix/\n+\n+test_expect_success 'git archive -0 --format=zip on large files' '\n+\ttest_config core.bigfilethreshold 1 &&\n+\tgit archive -0 --format=zip HEAD >large.zip\n+'\n+\n+check_zip large\n+\n+test_expect_success 'git archive --format=zip on large files' '\n+\ttest_config core.bigfilethreshold 1 &&\n+\tgit archive --format=zip HEAD >large-compressed.zip\n+'\n+\n+check_zip large-compressed\n+\n+test_done\n-- \n1.7.12\n"},{"id":"206162","messageId":"50E9BB8B.9020101@lsrfire.ath.cx","threadId":"32453","inReplyTo":"50E9B82D.50005@lsrfire.ath.cx","subject":"[PATCH 4/4] t5002: check if unzip supports symlinks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T17:59:39Z","receivedAt":"2013-01-06T17:59:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Only add a symlink to the repository if both the filesystem and\nunzip support symlinks. To check the latter, add a ZIP file\ncontaining a symlink, created like this with InfoZIP zip 3.0:\n\n\t$ echo sample text >textfile\n\t$ ln -s textfile symlink\n\t$ zip -y infozip-symlinks.zip textfile symlink\n\nIf we can extract it successfully, we add a symlink to the test\nrepository for git archive --format=zip, or otherwise skip that\nstep. Users can see the skipped test and perhaps run it again\nwith a different unzip version.\n\nSigned-off-by: Rene Scharfe \n---\n t/t5002-archive-zip.sh | 26 +++++++++++++++++++-------\n t/t5002/infozip-symlinks.zip | Bin 0 -> 328 bytes\n 2 files changed, 19 insertions(+), 7 deletions(-)\n create mode 100644 t/t5002/infozip-symlinks.zip\n\ndiff --git a/t/t5002-archive-zip.sh b/t/t5002-archive-zip.sh\nindex ac9c6d4..d35aa24 100755\n--- a/t/t5002-archive-zip.sh\n+++ b/t/t5002-archive-zip.sh\n@@ -12,6 +12,15 @@ test_lazy_prereq UNZIP '\n \ttest $? -ne 127\n '\n \n+test_lazy_prereq UNZIP_SYMLINKS '\n+\t(\n+\t\tmkdir unzip-symlinks &&\n+\t\tcd unzip-symlinks &&\n+\t\t\"$GIT_UNZIP\" \"$TEST_DIRECTORY\"/t5002/infozip-symlinks.zip &&\n+\t\ttest -h symlink\n+\t)\n+'\n+\n check_zip() {\n \tzipfile=$1.zip\n \tlistfile=$1.lst\n@@ -40,15 +49,18 @@ test_expect_success \\\n cp /bin/sh a/bin &&\n printf \"A\\$Format:%s\\$O\" \"$SUBSTFORMAT\" >a/substfile1 &&\n printf \"A not substituted O\" >a/substfile2 &&\n- if test_have_prereq SYMLINKS; then\n-\tln -s a a/l1\n- else\n-\tprintf %s a > a/l1\n- fi &&\n (p=long_path_to_a_file && cd a &&\n for depth in 1 2 3 4 5; do mkdir $p && cd $p; done &&\n- echo text >file_with_long_path) &&\n- (cd a && find .) | sort >a.lst'\n+ echo text >file_with_long_path)\n+'\n+\n+test_expect_success SYMLINKS,UNZIP_SYMLINKS 'add symlink' '\n+\tln -s a a/symlink_to_a\n+'\n+\n+test_expect_success 'prepare file list' '\n+\t(cd a && find .) | sort >a.lst\n+'\n \n test_expect_success \\\n 'add ignored file' \\\ndiff --git a/t/t5002/infozip-symlinks.zip b/t/t5002/infozip-symlinks.zip\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..065728c631cf1f7ab20a045a83abc3e08455eeba\nGIT binary patch\nliteral 328\nzcmWIWW@h1H0D(ty)tzkeJdg4K*&xipAj43ST2YdgnUfkC!pXp_F7Y}5gi9;985mh!\nzFf%Z)qyW_wC*~I9q$+@vas|Lmdj&M@9kbsh4zNiK4D3MDiYs$-GV`**hM5Bm0%0`6\nzU={{=Gcw6B<8qh;&`<^jMj&3&2x7r>g@&*~oQY;CvT2wOgO~;~=j}p2APILS&@e1c\nU4De=U11V+#!r4H2I*7vn0CeC%rvLx|\n\nliteral 0\nHcmV?d00001\n\n-- \n1.7.12\n"},{"id":"206164","messageId":"20130106180621.GA16494@ftbfs.org","threadId":"32453","inReplyTo":"50E9B90C.2060200@lsrfire.ath.cx","subject":"Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-01-06T18:06:21Z","receivedAt":"2013-01-06T18:06:21Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:\n> This change makes the code smaller and we can put it at the top of\n> the script, its rightful place as setup code.\n\nWould it be better to add the setting of GIT_UNZIP and\ntest_lazy_prereq to test-lib.sh so they aren't duplicated in both\nt0024-crlf-archive.sh and t5000-tar-tree.sh, something like the\nfollowing (modulo UNZIP/GIT_UNZIP)?\n\n-- \nMatt Kraai\nhttps://ftbfs.org/kraai\n\ndiff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh\nindex ec6c1b3..084f33c 100755\n--- a/t/t0024-crlf-archive.sh\n+++ b/t/t0024-crlf-archive.sh\n@@ -3,7 +3,6 @@\n test_description='respect crlf in git archive'\n \n . ./test-lib.sh\n-UNZIP=${UNZIP:-unzip}\n \n test_expect_success setup '\n \n@@ -26,13 +25,6 @@ test_expect_success 'tar archive' '\n \n '\n \n-\"$UNZIP\" -v >/dev/null 2>&1\n-if [ $? -eq 127 ]; then\n-\tsay \"Skipping ZIP test, because unzip was not found\"\n-else\n-\ttest_set_prereq UNZIP\n-fi\n-\n test_expect_success UNZIP 'zip archive' '\n \n \tgit archive --format=zip HEAD >test.zip &&\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex ecf00ed..85b64ae 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -25,7 +25,6 @@ commit id embedding:\n '\n \n . ./test-lib.sh\n-UNZIP=${UNZIP:-unzip}\n GZIP=${GZIP:-gzip}\n GUNZIP=${GUNZIP:-gzip -d}\n \n@@ -201,13 +200,6 @@ test_expect_success \\\n test_cmp a/substfile2 g/prefix/a/substfile2\n '\n \n-$UNZIP -v >/dev/null 2>&1\n-if [ $? -eq 127 ]; then\n-\tsay \"Skipping ZIP tests, because unzip was not found\"\n-else\n-\ttest_set_prereq UNZIP\n-fi\n-\n test_expect_success \\\n 'git archive --format=zip' \\\n 'git archive --format=zip HEAD >d.zip'\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8a12cbb..4ceabad 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -752,6 +752,13 @@ test_lazy_prereq AUTOIDENT '\n \tgit var GIT_AUTHOR_IDENT\n '\n \n+UNZIP=${UNZIP:-unzip}\n+\n+test_lazy_prereq UNZIP '\n+\t\"$UNZIP\" -v >/dev/null 2>&1\n+\ttest $? -ne 127\n+'\n+\n # When the tests are run as root, permission tests will report that\n # things are writable when they shouldn't be.\n test -w / || test_set_prereq SANITY\n"},{"id":"206180","messageId":"50E9F3B4.2080103@lsrfire.ath.cx","threadId":"32453","inReplyTo":"20130106180621.GA16494@ftbfs.org","subject":"Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-06T21:59:16Z","receivedAt":"2013-01-06T21:59:16Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 06.01.2013 19:06, schrieb Matt Kraai:\n> On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:\n>> This change makes the code smaller and we can put it at the top of\n>> the script, its rightful place as setup code.\n>\n> Would it be better to add the setting of GIT_UNZIP and\n> test_lazy_prereq to test-lib.sh so they aren't duplicated in both\n> t0024-crlf-archive.sh and t5000-tar-tree.sh, something like the\n> following (modulo UNZIP/GIT_UNZIP)?\n\nWe could do that in a follow-up patch, but I'm not sure it's worth it \nfor the two use cases.\n\nRené\n"},{"id":"206208","messageId":"20130107051609.GB27909@elie.Belkin","threadId":"32453","inReplyTo":"50E9B8CD.2010209@lsrfire.ath.cx","subject":"Re: [PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-07T05:16:09Z","receivedAt":"2013-01-07T05:16:09Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"René Scharfe wrote:\n\n> InfoZIP's unzip takes default parameters from the environment variable\n> UNZIP. Unset it in the test library and use GIT_UNZIP for specifying\n> alternate versions of the unzip command instead.\n>\n> t0024 wasn't even using variable for the actual extraction. t5000\n> was, but when setting it to InfoZIP's unzip it would try to extract\n> from itself (because it treats the contents of $UNZIP as parameters),\n> which failed of course.\n\nThat would only happen if the UNZIP variable was already exported,\nright?\n\nThe patch makes sense and takes care of all uses of ${UNZIP} I can\nfind, and it even makes the quoting consistent so a person can put\ntheir copy of unzip under \"/Program Files\". For what it's worth,\n\nReviewed-by: Jonathan Nieder \n"},{"id":"206217","messageId":"20130107084509.GH27909@elie.Belkin","threadId":"32453","inReplyTo":"50E9B90C.2060200@lsrfire.ath.cx","subject":"Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-07T08:45:10Z","receivedAt":"2013-01-07T08:45:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"René Scharfe wrote:\n\n> --- a/t/t0024-crlf-archive.sh\n> +++ b/t/t0024-crlf-archive.sh\n> @@ -5,6 +5,11 @@ test_description='respect crlf in git archive'\n> . ./test-lib.sh\n> GIT_UNZIP=${GIT_UNZIP:-unzip}\n> \n> +test_lazy_prereq UNZIP '\n> +\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n> +\ttest $? -ne 127\n\nMicronit: now that this is part of a test, there is no more need to\nsilence its output. The \"unzip -v\" output could be useful to people\ndebugging with \"t0024-crlf-archive.sh -v -i\".\n\nWith or without that change, this is a nice cleanup and obviously\ncorrect, so\nReviewed-by: Jonathan Nieder \n"},{"id":"206218","messageId":"20130107085206.GI27909@elie.Belkin","threadId":"32453","inReplyTo":"50E9BB8B.9020101@lsrfire.ath.cx","subject":"Re: [PATCH 4/4] t5002: check if unzip supports symlinks","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-07T08:52:06Z","receivedAt":"2013-01-07T08:52:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"René Scharfe wrote:\n\n> Only add a symlink to the repository if both the filesystem and\n> unzip support symlinks. To check the latter, add a ZIP file\n> containing a symlink, created like this with InfoZIP zip 3.0:\n>\n>\t$ echo sample text >textfile\n>\t$ ln -s textfile symlink\n>\t$ zip -y infozip-symlinks.zip textfile symlink\n\nHm. Do some implementations of \"unzip\" not support symlinks, or is\nthe problem that some systems build Info-ZIP without the SYMLINKS\noption?\n"},{"id":"206232","messageId":"50EAF703.7070806@lsrfire.ath.cx","threadId":"32453","inReplyTo":"20130107051609.GB27909@elie.Belkin","subject":"Re: [PATCH 1/4] t0024, t5000: clear variable UNZIP, use GIT_UNZIP instead","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-07T16:25:39Z","receivedAt":"2013-01-07T16:25:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.01.2013 06:16, schrieb Jonathan Nieder:\n> René Scharfe wrote:\n>\n>> InfoZIP's unzip takes default parameters from the environment variable\n>> UNZIP. Unset it in the test library and use GIT_UNZIP for specifying\n>> alternate versions of the unzip command instead.\n>>\n>> t0024 wasn't even using variable for the actual extraction. t5000\n>> was, but when setting it to InfoZIP's unzip it would try to extract\n>> from itself (because it treats the contents of $UNZIP as parameters),\n>> which failed of course.\n>\n> That would only happen if the UNZIP variable was already exported,\n> right?\n\nWe don't want any parameters a user may have been specified influence \nthe test. I'm not sure if someone actually sets that variable for that \npurpose, though.\n\nMy main use case is running individual test scripts with an alternative \nunzip binary, and with the patch this works as expected:\n\n\t$ cd t\n\t$ GIT_UNZIP=/usr/pkg/bin/unzip ./t5000-tar-tree.sh\n\n> The patch makes sense and takes care of all uses of ${UNZIP} I can\n> find, and it even makes the quoting consistent so a person can put\n> their copy of unzip under \"/Program Files\". For what it's worth,\n>\n> Reviewed-by: Jonathan Nieder \n\nThanks!\n\nRené\n"},{"id":"206233","messageId":"50EAF7A1.3090602@lsrfire.ath.cx","threadId":"32453","inReplyTo":"20130107084509.GH27909@elie.Belkin","subject":"Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-07T16:28:17Z","receivedAt":"2013-01-07T16:28:17Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.01.2013 09:45, schrieb Jonathan Nieder:\n> René Scharfe wrote:\n>\n>> --- a/t/t0024-crlf-archive.sh\n>> +++ b/t/t0024-crlf-archive.sh\n>> @@ -5,6 +5,11 @@ test_description='respect crlf in git archive'\n>> . ./test-lib.sh\n>> GIT_UNZIP=${GIT_UNZIP:-unzip}\n>>\n>> +test_lazy_prereq UNZIP '\n>> +\t\"$GIT_UNZIP\" -v >/dev/null 2>&1\n>> +\ttest $? -ne 127\n>\n> Micronit: now that this is part of a test, there is no more need to\n> silence its output. The \"unzip -v\" output could be useful to people\n> debugging with \"t0024-crlf-archive.sh -v -i\".\n\nOh, yes, good point.\n\n> With or without that change, this is a nice cleanup and obviously\n> correct, so\n> Reviewed-by: Jonathan Nieder \n\nThanks,\nRené\n"},{"id":"206234","messageId":"50EAFCD7.9090008@lsrfire.ath.cx","threadId":"32453","inReplyTo":"20130107085206.GI27909@elie.Belkin","subject":"Re: [PATCH 4/4] t5002: check if unzip supports symlinks","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-01-07T16:50:31Z","receivedAt":"2013-01-07T16:50:31Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 07.01.2013 09:52, schrieb Jonathan Nieder:\n> René Scharfe wrote:\n>\n>> Only add a symlink to the repository if both the filesystem and\n>> unzip support symlinks. To check the latter, add a ZIP file\n>> containing a symlink, created like this with InfoZIP zip 3.0:\n>>\n>> \t$ echo sample text >textfile\n>> \t$ ln -s textfile symlink\n>> \t$ zip -y infozip-symlinks.zip textfile symlink\n>\n> Hm. Do some implementations of \"unzip\" not support symlinks, or is\n> the problem that some systems build Info-ZIP without the SYMLINKS\n> option?\n\nThe unzip supplied with NetBSD 6.0.1, which is based on libarchive, \ndoesn't support symlinks. It creates a file with the link target path \nas its only content for such entries.\n\nI assume that Info-ZIP is compiled with the SYMLINKS option on all \nplatforms whose default filesystem supports symbolic links. Except on \nWindows perhaps, where it's complicated.\n\nFor the test script there is no difference: If we don't have a tool to \nverify symlinks in archives, we better skip that part.\n\nRené\n"},{"id":"206443","messageId":"20130110073629.GC5121@elie.Belkin","threadId":"32453","inReplyTo":"50EAFCD7.9090008@lsrfire.ath.cx","subject":"Re: [PATCH 4/4] t5002: check if unzip supports symlinks","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-10T07:36:29Z","receivedAt":"2013-01-10T07:36:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"René Scharfe wrote:\n> Am 07.01.2013 09:52, schrieb Jonathan Nieder:\n\n>> Hm. Do some implementations of \"unzip\" not support symlinks, or is\n>> the problem that some systems build Info-ZIP without the SYMLINKS\n>> option?\n>\n> The unzip supplied with NetBSD 6.0.1, which is based on libarchive, doesn't\n> support symlinks. It creates a file with the link target path as its only\n> content for such entries.\n\nOk, that makes sense. A quick search finds\n, which if\nI understand correctly was fixed in libarchive 3.0.2. NetBSD 6 uses a\npatched 2.8.4.\n\n[...]\n> For the test script there is no difference: If we don't have a tool to\n> verify symlinks in archives, we better skip that part.\n\nYeah, I just wanted to see if there were other parts of the world that\nneeded fixing while at it. Thanks for explaining.\n\nCiao,\nJonathan\n"}]}