threads / patch / 33760

patch, 2 partst5004: ignore pax global header file

Subject: [PATCH 1/2] t5004: ignore pax global header file

## tl;dr

6 messages between May 9, 2013 and May 11, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

René Scharfe· May 9, 2013, 13:10 UTC · lore

Versions of tar that don't know pax headers -- like the ones in NetBSD 6 and OpenBSD 5.2 -- extract them as regular files. Explicitly ignore the file created for our global header when checking the list of extracted files, as this is normal and harmless fall-back behaviour. This fixes test 3 of t5004 on these platforms.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Fixes a failure of a test added after v1.8.2.
 t/t5004-archive-corner-cases.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t5004-archive-corner-cases.sh +1 −1
diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index bfdb56a..ddf6e35 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -23,7 +23,7 @@ check_dir() {
 			echo "$dir/$i"
 		done
 	} | sort >expect &&
-	find "$dir" -print | sort >actual &&
+	find "$dir" ! -name pax_global_header -print | sort >actual &&
 	test_cmp expect actual
 }
 
-- 
1.8.2.1
René Scharfe· May 9, 2013, 13:13 UTC · re: René Scharfe · lore

[PATCH 2/2] t5004: avoid using tar for checking emptiness of archive

Test 2 of t5004 checks if a supposedly empty tar archive really contains no files. 24676f02 (t5004: fix issue with empty archive test and bsdtar) removed our commit hash to make it work with bsdtar, but the test still fails on NetBSD and OpenBSD, which use their own tar that considers a tar file containing only NULs as broken.

Here's what the different archivers do when asked to create a tar file without entries:

	$ uname -v
	NetBSD 6.0.1 (GENERIC)
	$ gtar --version | head -1
	tar (GNU tar) 1.26
	$ bsdtar --version
	bsdtar 2.8.4 - libarchive 2.8.4
	$ : >zero.tar
	$ perl -e 'print "\0" x 10240' >tenk.tar
	$ sha1 zero.tar tenk.tar
	SHA1 (zero.tar) = da39a3ee5e6b4b0d3255bfef95601890afd80709
	SHA1 (tenk.tar) = 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
	$ : | tar cf - -T - | sha1
	da39a3ee5e6b4b0d3255bfef95601890afd80709
	$ : | gtar cf - -T - | sha1
	34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
	$ : | bsdtar cf - -T - | sha1
	34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
So NetBSD's native tar creates an empty file, while GNU tar and bsdtar
both give us 10KB of NULs -- just like git archive with an empty tree.
Now let's see how the archivers handle these two kinds of empty tar
files:
	
	$ tar tf zero.tar; echo $?
	tar: Unexpected EOF on archive file
	1
	$ gtar tf zero.tar; echo $?
	gtar: This does not look like a tar archive
	gtar: Exiting with failure status due to previous errors
	2
	$ bsdtar tf zero.tar; echo $?
	0
	$ tar tf tenk.tar; echo $?
	tar: Cannot identify format. Searching...
	tar: End of archive volume 1 reached
	tar: Sorry, unable to determine archive format.
	$ gtar tf tenk.tar; echo $?
	0
	$ bsdtar tf tenk.tar; echo $?
	0

NetBSD's tar complains about both, bsdtar happily accepts any of them and GNU tar doesn't like zero-length archive files. So the safest course of action is to stay with our block-of-NULs format which is compatible with GNU tar and bsdtar, as we can't make NetBSD's native tar happy anyway.

We can simplify our test, however, by taking tar out of the picture. Instead of extracting the archive and checking for the non-presence of files, check if the file has a size of 10KB and contains only NULs. This makes t5004 pass on NetBSD and OpenBSD.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Fixes a failure of a test added after v1.8.2.
 t/t5004-archive-corner-cases.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Show changes to t/t5004-archive-corner-cases.sh +2 −3
diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index ddf6e35..8d1bbd3 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -29,9 +29,8 @@ check_dir() {
 
 test_expect_success 'tar archive of empty tree is empty' '
 	git archive --format=tar HEAD: >empty.tar &&
-	make_dir extract &&
-	"$TAR" xf empty.tar -C extract &&
-	check_dir extract
+	perl -e "print \"\\0\" x 10240" >10knuls.tar &&
+	test_cmp 10knuls.tar empty.tar
 '
 
 test_expect_success 'tar archive of empty tree with prefix' '
-- 
1.8.2.1
René Scharfe· May 9, 2013, 13:36 UTC · re: René Scharfe · lore

[PATCH 3/2] t5004: resurrect original empty tar archive test

Add a test to verify the emptiness of an archive by extracting its contents. Don't run this test if the version of tar doesn't support archives containing only a comment header, though.

The existing check 'tar archive of empty tree is empty' used to work like that (minus the tar capability check) but was changed to depend on the exact representation of empty tar files created by git archive instead of on the behaviour of tar in order to avoid issues with different tar versions.

The different approaches test different things: The existing one is for empty trees, for which we know the exact expected output and thus we can simply check it without extracting; the new one is for commits with empty trees, whose archives include stamps and so the more "natural" check by extraction is a better fit because it focuses on the interesting aspect, namely the absence of any archive entries.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Not urgent.  By the way: A fix for the bsdtar issue worked around by
our 24676f02 (t5004: fix issue with empty archive test and bsdtar) has
been committed to the libarchive repo, but it's not in any released
version, yet.
 t/t5004-archive-corner-cases.sh   |  14 ++++++++++++++
 t/t5004/empty-with-pax-header.tar | Bin 0 -> 10240 bytes
 2 files changed, 14 insertions(+)
 create mode 100644 t/t5004/empty-with-pax-header.tar
Show changes to 2 files +14 −0

t/t5004-archive-corner-cases.sh, t/t5004/empty-with-pax-header.tar

diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index 8d1bbd3..f25f06b 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -27,6 +27,20 @@ check_dir() {
 	test_cmp expect actual
 }
 
+# bsdtar/libarchive versions before 3.1.3 consider a tar file with a
+# global pax header that is not followed by a file record as corrupt.
+if "$TAR" tf "$TEST_DIRECTORY"/t5004/empty-with-pax-header.tar >/dev/null 2>&1
+then
+	test_set_prereq HEADER_ONLY_TAR_OK
+fi
+
+test_expect_success HEADER_ONLY_TAR_OK 'tar archive of commit with empty tree' '
+	git archive --format=tar HEAD >empty-with-pax-header.tar &&
+	make_dir extract &&
+	"$TAR" xf empty-with-pax-header.tar -C extract &&
+	check_dir extract
+'
+
 test_expect_success 'tar archive of empty tree is empty' '
 	git archive --format=tar HEAD: >empty.tar &&
 	perl -e "print \"\\0\" x 10240" >10knuls.tar &&
diff --git a/t/t5004/empty-with-pax-header.tar b/t/t5004/empty-with-pax-header.tar
new file mode 100644
index 0000000000000000000000000000000000000000..da9e39e6cf49841254a2d75aabb9ef575f9fd805
GIT binary patch
literal 10240
zcmeIuF%H5Y6vlC8PvH>&fx_TfnwHW!v|?&aJib+jCU$V?VB-HRkMITZ-tSV~%dXFL
z)t9GKHE9&vmz>KvC!T$-&pwAnD6NckQtDT(j47<>wjX8v<Lx?C<2=%s^R!Nvn{WAh
zw`IBI<xiii-p4!)={y}{+a@P%2!(3ZA%xgSuTAZ&@lBAmw}p36CcpdXg%}P21Q0*~
t0R#|0009ILKmY**5I_I{1Q0*~0R#|0009ILKmY**5I_I{1Q7TWfd`KEC+GkG

literal 0
HcmV?d00001
-- 
1.8.2.1
Jonathan Nieder· May 11, 2013, 06:03 UTC · re: René Scharfe · lore

Re: [PATCH 3/2] t5004: resurrect original empty tar archive test

Hi,
René Scharfe wrote:
> [Subject: t5004: resurrect original empty tar archive test]
[...]
Show 8 quoted lines
> The different approaches test different things: The existing one is
> for empty trees, for which we know the exact expected output and thus
> we can simply check it without extracting; the new one is for commits
> with empty trees, whose archives include stamps and so the more
> "natural" check by extraction is a better fit because it focuses on
> the interesting aspect, namely the absence of any archive entries.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>

When first reading I was a little confused: does this patch resurrect the original, existing test for empty tree handling in the form it had before patch 2/3, or is it adding a new, distinct test that complements the existing one that patch 2/3 modified?

A quick glance back at v1.8.2.2~7^2 (t5004: fix issue with empty archive test and bsdtar, 2013-04-10) cleared matters up. The original test that is being resurrected is the one from before that commit.

Maybe a reminder in the commit message would help.  E.g.,
	The earlier version of the same check (before 24676f02, "t5004: fix
	issue with empty archive test and bsdtar") revived by this patch tests
	a different thing: The modified check is for empty trees, for which we
	know the exact expected output and thus we can simply check it without
	extracting; the original one is for commits with empty trees, whose
	archives include stamps and so the more "natural" check by extraction
	is a better fit because it focuses on the interesting aspect, namely
	the absence of any archive entries.
With or without such a change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Would it make sense to define HEADER_ONLY_TAR_OK as a lazy prereq in the same file (even though it is only used once), so the code that checks "tar" is not run if this test is being skipped (e.g., using GIT_TEST_SKIP) for some other reason?

Thanks, Jonathan

Eric Sunshine· May 9, 2013, 18:21 UTC · re: René Scharfe · lore

Re: [PATCH 2/2] t5004: avoid using tar for checking emptiness of archive

On Thu, May 9, 2013 at 9:13 AM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

Show 48 quoted lines
> Test 2 of t5004 checks if a supposedly empty tar archive really
> contains no files.  24676f02 (t5004: fix issue with empty archive test
> and bsdtar) removed our commit hash to make it work with bsdtar, but
> the test still fails on NetBSD and OpenBSD, which use their own tar
> that considers a tar file containing only NULs as broken.
>
> Here's what the different archivers do when asked to create a tar
> file without entries:
>
>         $ uname -v
>         NetBSD 6.0.1 (GENERIC)
>         $ gtar --version | head -1
>         tar (GNU tar) 1.26
>         $ bsdtar --version
>         bsdtar 2.8.4 - libarchive 2.8.4
>
>         $ : >zero.tar
>         $ perl -e 'print "\0" x 10240' >tenk.tar
>         $ sha1 zero.tar tenk.tar
>         SHA1 (zero.tar) = da39a3ee5e6b4b0d3255bfef95601890afd80709
>         SHA1 (tenk.tar) = 34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
>
>         $ : | tar cf - -T - | sha1
>         da39a3ee5e6b4b0d3255bfef95601890afd80709
>         $ : | gtar cf - -T - | sha1
>         34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
>         $ : | bsdtar cf - -T - | sha1
>         34e163be8e43c5631d8b92e9c43ab0bf0fa62b9c
>
> So NetBSD's native tar creates an empty file, while GNU tar and bsdtar
> both give us 10KB of NULs -- just like git archive with an empty tree.
> Now let's see how the archivers handle these two kinds of empty tar
> files:
>
>         $ tar tf zero.tar; echo $?
>         tar: Unexpected EOF on archive file
>         1
>         $ gtar tf zero.tar; echo $?
>         gtar: This does not look like a tar archive
>         gtar: Exiting with failure status due to previous errors
>         2
>         $ bsdtar tf zero.tar; echo $?
>         0
>
>         $ tar tf tenk.tar; echo $?
>         tar: Cannot identify format. Searching...
>         tar: End of archive volume 1 reached
>         tar: Sorry, unable to determine archive format.
Missing "echo $?" output.
Show 17 quoted lines
>         $ gtar tf tenk.tar; echo $?
>         0
>         $ bsdtar tf tenk.tar; echo $?
>         0
>
> NetBSD's tar complains about both, bsdtar happily accepts any of them
> and GNU tar doesn't like zero-length archive files.  So the safest
> course of action is to stay with our block-of-NULs format which is
> compatible with GNU tar and bsdtar, as we can't make NetBSD's native
> tar happy anyway.
>
> We can simplify our test, however, by taking tar out of the picture.
> Instead of extracting the archive and checking for the non-presence of
> files, check if the file has a size of 10KB and contains only NULs.
> This makes t5004 pass on NetBSD and OpenBSD.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
René Scharfe· May 9, 2013, 19:12 UTC · re: Eric Sunshine · lore

Re: [PATCH 2/2] t5004: avoid using tar for checking emptiness of archive

Am 09.05.2013 20:21, schrieb Eric Sunshine:
Show 8 quoted lines
> On Thu, May 9, 2013 at 9:13 AM, René Scharfe
> <rene.scharfe@lsrfire.ath.cx> wrote:
>>          $ tar tf tenk.tar; echo $?
>>          tar: Cannot identify format. Searching...
>>          tar: End of archive volume 1 reached
>>          tar: Sorry, unable to determine archive format.
>
> Missing "echo $?" output.
Huh, where did that go?  It's 1.
Thanks for catching this omission.
René

← back to recent threads