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

2013-05-11 Thread Jonathan Nieder
Hi,

René Scharfe wrote:

 [Subject: t5004: resurrect original empty tar archive test]
[...]
 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.scha...@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 jrnie...@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
--
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 3/2] t5004: resurrect original empty tar archive test

2013-05-09 Thread René Scharfe
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.scha...@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

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 21
+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 
..da9e39e6cf49841254a2d75aabb9ef575f9fd805
GIT binary patch
literal 10240
zcmeIuF%H5Y6vlC8PvHfx_TfnwHW!v|?aJib+jCU$V?VB-HRkMITZ-tSV~%dXFL
z)t9GKHE9vmzKvC!T$-pwAnD6NckQtDT(j47wjX8vLx?C2=%s^R!Nvn{WAh
zw`IBIxiii-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?d1

-- 
1.8.2.1

--
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