{"thread":{"id":"33760","subject":"[PATCH 1/2] t5004: ignore pax global header file","startedAt":"2013-05-09T13:10:48Z","lastAt":"2013-05-11T06:03:32Z","messageCount":6,"participants":["René Scharfe","Eric Sunshine","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"216764","messageId":"518BA058.6050300@lsrfire.ath.cx","threadId":"33760","inReplyTo":null,"subject":"[PATCH 1/2] t5004: ignore pax global header file","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-09T13:10:48Z","receivedAt":"2013-05-09T13:10:48Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Versions of tar that don't know pax headers -- like the ones in NetBSD 6\nand OpenBSD 5.2 -- extract them as regular files. Explicitly ignore the\nfile created for our global header when checking the list of extracted\nfiles, as this is normal and harmless fall-back behaviour. This fixes\ntest 3 of t5004 on these platforms.\n\nSigned-off-by: Rene Scharfe \n---\nFixes a failure of a test added after v1.8.2.\n\n t/t5004-archive-corner-cases.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh\nindex bfdb56a..ddf6e35 100755\n--- a/t/t5004-archive-corner-cases.sh\n+++ b/t/t5004-archive-corner-cases.sh\n@@ -23,7 +23,7 @@ check_dir() {\n \t\t\techo \"$dir/$i\"\n \t\tdone\n \t} | sort >expect &&\n-\tfind \"$dir\" -print | sort >actual &&\n+\tfind \"$dir\" ! -name pax_global_header -print | sort >actual &&\n \ttest_cmp expect actual\n }\n \n-- \n1.8.2.1\n"},{"id":"216765","messageId":"518BA10B.2080003@lsrfire.ath.cx","threadId":"33760","inReplyTo":"518BA058.6050300@lsrfire.ath.cx","subject":"[PATCH 2/2] t5004: avoid using tar for checking emptiness of archive","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-09T13:13:47Z","receivedAt":"2013-05-09T13:13:47Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Test 2 of t5004 checks if a supposedly empty tar archive really\ncontains no files. 24676f02 (t5004: fix issue with empty archive test\nand bsdtar) removed our commit hash to make it work with bsdtar, but\nthe test still fails on NetBSD and OpenBSD, which use their own tar\nthat considers a tar file containing only NULs as broken.\n\nHere's what the different archivers do when asked to create a tar\nfile without entries:\n\n\t$ uname -v\n\tNetBSD 6.0.1 (GENERIC)\n\t$ gtar --version | head -1\n\ttar (GNU tar) 1.26\n\t$ bsdtar --version\n\tbsdtar 2.8.4 - libarchive 2.8.4\n\n\t$ : >zero.tar\n\t$ perl -e 'print \"\\0\" x 10240' >tenk.tar\n\t$ sha1 zero.tar tenk.tar\n\tSHA1 (zero.tar) = da39a3ee5e6b4b0d3255bfef95601890afd80709\n\tSHA1 (tenk.tar) = 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n\n\t$ : | tar cf - -T - | sha1\n\tda39a3ee5e6b4b0d3255bfef95601890afd80709\n\t$ : | gtar cf - -T - | sha1\n\t34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n\t$ : | bsdtar cf - -T - | sha1\n\t34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n\nSo NetBSD's native tar creates an empty file, while GNU tar and bsdtar\nboth give us 10KB of NULs -- just like git archive with an empty tree.\nNow let's see how the archivers handle these two kinds of empty tar\nfiles:\n\t\n\t$ tar tf zero.tar; echo $?\n\ttar: Unexpected EOF on archive file\n\t1\n\t$ gtar tf zero.tar; echo $?\n\tgtar: This does not look like a tar archive\n\tgtar: Exiting with failure status due to previous errors\n\t2\n\t$ bsdtar tf zero.tar; echo $?\n\t0\n\n\t$ tar tf tenk.tar; echo $?\n\ttar: Cannot identify format. Searching...\n\ttar: End of archive volume 1 reached\n\ttar: Sorry, unable to determine archive format.\n\t$ gtar tf tenk.tar; echo $?\n\t0\n\t$ bsdtar tf tenk.tar; echo $?\n\t0\n\nNetBSD's tar complains about both, bsdtar happily accepts any of them\nand GNU tar doesn't like zero-length archive files. So the safest\ncourse of action is to stay with our block-of-NULs format which is\ncompatible with GNU tar and bsdtar, as we can't make NetBSD's native\ntar happy anyway.\n\nWe can simplify our test, however, by taking tar out of the picture.\nInstead of extracting the archive and checking for the non-presence of\nfiles, check if the file has a size of 10KB and contains only NULs.\nThis makes t5004 pass on NetBSD and OpenBSD.\n\nSigned-off-by: Rene Scharfe \n---\nFixes a failure of a test added after v1.8.2.\n\n t/t5004-archive-corner-cases.sh | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh\nindex ddf6e35..8d1bbd3 100755\n--- a/t/t5004-archive-corner-cases.sh\n+++ b/t/t5004-archive-corner-cases.sh\n@@ -29,9 +29,8 @@ check_dir() {\n \n test_expect_success 'tar archive of empty tree is empty' '\n \tgit archive --format=tar HEAD: >empty.tar &&\n-\tmake_dir extract &&\n-\t\"$TAR\" xf empty.tar -C extract &&\n-\tcheck_dir extract\n+\tperl -e \"print \\\"\\\\0\\\" x 10240\" >10knuls.tar &&\n+\ttest_cmp 10knuls.tar empty.tar\n '\n \n test_expect_success 'tar archive of empty tree with prefix' '\n-- \n1.8.2.1\n"},{"id":"216767","messageId":"518BA64A.5020302@lsrfire.ath.cx","threadId":"33760","inReplyTo":"518BA10B.2080003@lsrfire.ath.cx","subject":"[PATCH 3/2] t5004: resurrect original empty tar archive test","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-09T13:36:10Z","receivedAt":"2013-05-09T13:36:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add a test to verify the emptiness of an archive by extracting its\ncontents. Don't run this test if the version of tar doesn't support\narchives containing only a comment header, though.\n\nThe existing check 'tar archive of empty tree is empty' used to work\nlike that (minus the tar capability check) but was changed to depend\non the exact representation of empty tar files created by git archive\ninstead of on the behaviour of tar in order to avoid issues with\ndifferent tar versions.\n\nThe different approaches test different things: The existing one is\nfor empty trees, for which we know the exact expected output and thus\nwe can simply check it without extracting; the new one is for commits\nwith empty trees, whose archives include stamps and so the more\n\"natural\" check by extraction is a better fit because it focuses on\nthe interesting aspect, namely the absence of any archive entries.\n\nSigned-off-by: Rene Scharfe \n---\nNot urgent. By the way: A fix for the bsdtar issue worked around by\nour 24676f02 (t5004: fix issue with empty archive test and bsdtar) has\nbeen committed to the libarchive repo, but it's not in any released\nversion, yet.\n\n t/t5004-archive-corner-cases.sh | 14 ++++++++++++++\n t/t5004/empty-with-pax-header.tar | Bin 0 -> 10240 bytes\n 2 files changed, 14 insertions(+)\n create mode 100644 t/t5004/empty-with-pax-header.tar\n\ndiff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh\nindex 8d1bbd3..f25f06b 100755\n--- a/t/t5004-archive-corner-cases.sh\n+++ b/t/t5004-archive-corner-cases.sh\n@@ -27,6 +27,20 @@ check_dir() {\n \ttest_cmp expect actual\n }\n \n+# bsdtar/libarchive versions before 3.1.3 consider a tar file with a\n+# global pax header that is not followed by a file record as corrupt.\n+if \"$TAR\" tf \"$TEST_DIRECTORY\"/t5004/empty-with-pax-header.tar >/dev/null 2>&1\n+then\n+\ttest_set_prereq HEADER_ONLY_TAR_OK\n+fi\n+\n+test_expect_success HEADER_ONLY_TAR_OK 'tar archive of commit with empty tree' '\n+\tgit archive --format=tar HEAD >empty-with-pax-header.tar &&\n+\tmake_dir extract &&\n+\t\"$TAR\" xf empty-with-pax-header.tar -C extract &&\n+\tcheck_dir extract\n+'\n+\n test_expect_success 'tar archive of empty tree is empty' '\n \tgit archive --format=tar HEAD: >empty.tar &&\n \tperl -e \"print \\\"\\\\0\\\" x 10240\" >10knuls.tar &&\ndiff --git a/t/t5004/empty-with-pax-header.tar b/t/t5004/empty-with-pax-header.tar\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..da9e39e6cf49841254a2d75aabb9ef575f9fd805\nGIT binary patch\nliteral 10240\nzcmeIuF%H5Y6vlC8PvH>&fx_TfnwHW!v|?&aJib+jCU$V?VB-HRkMITZ-tSV~%dXFL\nz)t9GKHE9&vmz>KvC!T$-&pwAnD6NckQtDT(j47<>wjX8v wrote:\n> Test 2 of t5004 checks if a supposedly empty tar archive really\n> contains no files. 24676f02 (t5004: fix issue with empty archive test\n> and bsdtar) removed our commit hash to make it work with bsdtar, but\n> the test still fails on NetBSD and OpenBSD, which use their own tar\n> that considers a tar file containing only NULs as broken.\n>\n> Here's what the different archivers do when asked to create a tar\n> file without entries:\n>\n> $ uname -v\n> NetBSD 6.0.1 (GENERIC)\n> $ gtar --version | head -1\n> tar (GNU tar) 1.26\n> $ bsdtar --version\n> bsdtar 2.8.4 - libarchive 2.8.4\n>\n> $ : >zero.tar\n> $ perl -e 'print \"\\0\" x 10240' >tenk.tar\n> $ sha1 zero.tar tenk.tar\n> SHA1 (zero.tar) = da39a3ee5e6b4b0d3255bfef95601890afd80709\n> SHA1 (tenk.tar) = 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n>\n> $ : | tar cf - -T - | sha1\n> da39a3ee5e6b4b0d3255bfef95601890afd80709\n> $ : | gtar cf - -T - | sha1\n> 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n> $ : | bsdtar cf - -T - | sha1\n> 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c\n>\n> So NetBSD's native tar creates an empty file, while GNU tar and bsdtar\n> both give us 10KB of NULs -- just like git archive with an empty tree.\n> Now let's see how the archivers handle these two kinds of empty tar\n> files:\n>\n> $ tar tf zero.tar; echo $?\n> tar: Unexpected EOF on archive file\n> 1\n> $ gtar tf zero.tar; echo $?\n> gtar: This does not look like a tar archive\n> gtar: Exiting with failure status due to previous errors\n> 2\n> $ bsdtar tf zero.tar; echo $?\n> 0\n>\n> $ tar tf tenk.tar; echo $?\n> tar: Cannot identify format. Searching...\n> tar: End of archive volume 1 reached\n> tar: Sorry, unable to determine archive format.\n\nMissing \"echo $?\" output.\n\n> $ gtar tf tenk.tar; echo $?\n> 0\n> $ bsdtar tf tenk.tar; echo $?\n> 0\n>\n> NetBSD's tar complains about both, bsdtar happily accepts any of them\n> and GNU tar doesn't like zero-length archive files. So the safest\n> course of action is to stay with our block-of-NULs format which is\n> compatible with GNU tar and bsdtar, as we can't make NetBSD's native\n> tar happy anyway.\n>\n> We can simplify our test, however, by taking tar out of the picture.\n> Instead of extracting the archive and checking for the non-presence of\n> files, check if the file has a size of 10KB and contains only NULs.\n> This makes t5004 pass on NetBSD and OpenBSD.\n>\n> Signed-off-by: Rene Scharfe \n"},{"id":"216806","messageId":"518BF527.3070609@lsrfire.ath.cx","threadId":"33760","inReplyTo":"CAPig+cTidjz4D1vZ7Aci4YZK8ph9HJ1vMqZ1-n2hVPbLuf4AKw@mail.gmail.com","subject":"Re: [PATCH 2/2] t5004: avoid using tar for checking emptiness of archive","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2013-05-09T19:12:39Z","receivedAt":"2013-05-09T19:12:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 09.05.2013 20:21, schrieb Eric Sunshine:\n> On Thu, May 9, 2013 at 9:13 AM, René Scharfe\n> wrote:\n>> $ tar tf tenk.tar; echo $?\n>> tar: Cannot identify format. Searching...\n>> tar: End of archive volume 1 reached\n>> tar: Sorry, unable to determine archive format.\n>\n> Missing \"echo $?\" output.\n\nHuh, where did that go? It's 1.\n\nThanks for catching this omission.\n\nRené\n"},{"id":"216962","messageId":"20130511060332.GA3394@elie","threadId":"33760","inReplyTo":"518BA64A.5020302@lsrfire.ath.cx","subject":"Re: [PATCH 3/2] t5004: resurrect original empty tar archive test","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-11T06:03:32Z","receivedAt":"2013-05-11T06:03:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRené Scharfe wrote:\n\n> [Subject: t5004: resurrect original empty tar archive test]\n[...]\n> The different approaches test different things: The existing one is\n> for empty trees, for which we know the exact expected output and thus\n> we can simply check it without extracting; the new one is for commits\n> with empty trees, whose archives include stamps and so the more\n> \"natural\" check by extraction is a better fit because it focuses on\n> the interesting aspect, namely the absence of any archive entries.\n>\n> Signed-off-by: Rene Scharfe \n\nWhen first reading I was a little confused: does this patch resurrect\nthe original, existing test for empty tree handling in the form it had\nbefore patch 2/3, or is it adding a new, distinct test that\ncomplements the existing one that patch 2/3 modified?\n\nA quick glance back at v1.8.2.2~7^2 (t5004: fix issue with empty\narchive test and bsdtar, 2013-04-10) cleared matters up. The original\ntest that is being resurrected is the one from before that commit.\n\nMaybe a reminder in the commit message would help. E.g.,\n\n\tThe earlier version of the same check (before 24676f02, \"t5004: fix\n\tissue with empty archive test and bsdtar\") revived by this patch tests\n\ta different thing: The modified check is for empty trees, for which we\n\tknow the exact expected output and thus we can simply check it without\n\textracting; the original one is for commits with empty trees, whose\n\tarchives include stamps and so the more \"natural\" check by extraction\n\tis a better fit because it focuses on the interesting aspect, namely\n\tthe absence of any archive entries.\n\nWith or without such a change,\nReviewed-by: Jonathan Nieder \n\nWould it make sense to define HEADER_ONLY_TAR_OK as a lazy prereq in\nthe same file (even though it is only used once), so the code that\nchecks \"tar\" is not run if this test is being skipped (e.g.,\nusing GIT_TEST_SKIP) for some other reason?\n\nThanks,\nJonathan\n"}]}