[PATCH] t5000, t5003: do not use test_cmp to compare binary files

2014-06-04 Thread Stepan Kasal 
test_cmp() is primarily meant to compare text files (and display the
difference for debug purposes).

Raw cmp is better suited to compare binary files (tar, zip, etc.).

On MinGW, test_cmp is a shell function mingw_test_cmp that tries to
read both files into environment, stripping CR characters (introduced
in commit 4d715ac0).

This function usually speeds things up, as fork is extremly slow on
Windows.  But no wonder that this function is extremely slow and
sometimes even crashes when comparing large tar or zip files.

Signed-off-by: Stepan Kasal ka...@ucw.cz
---
 t/t5000-tar-tree.sh | 34 +-
 t/t5001-archive-attr.sh |  2 +-
 t/t5003-archive-zip.sh  |  6 +++---
 t/t5004-archive-corner-cases.sh |  2 +-
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 1cf0a4e..31b1fd1 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -164,7 +164,7 @@ check_tar with_olde-prefix olde-
 test_expect_success 'git archive on large files' '
 test_config core.bigfilethreshold 1 
 git archive HEAD b3.tar 
-test_cmp b.tar b3.tar
+cmp b.tar b3.tar
 '
 
 test_expect_success \
@@ -173,15 +173,15 @@ test_expect_success \
 
 test_expect_success \
 'git archive vs. the same in a bare repo' \
-'test_cmp b.tar b3.tar'
+'cmp b.tar b3.tar'
 
 test_expect_success 'git archive with --output' \
 'git archive --output=b4.tar HEAD 
-test_cmp b.tar b4.tar'
+cmp b.tar b4.tar'
 
 test_expect_success 'git archive --remote' \
 'git archive --remote=. HEAD b5.tar 
-test_cmp b.tar b5.tar'
+cmp b.tar b5.tar'
 
 test_expect_success \
 'validate file modification time' \
@@ -198,7 +198,7 @@ test_expect_success \
 
 test_expect_success 'git archive with --output, override inferred format' '
git archive --format=tar --output=d4.zip HEAD 
-   test_cmp b.tar d4.zip
+   cmp b.tar d4.zip
 '
 
 test_expect_success \
@@ -244,34 +244,34 @@ test_expect_success 'archive --list shows only enabled 
remote filters' '
 test_expect_success 'invoke tar filter by format' '
git archive --format=tar.foo HEAD config.tar.foo 
tr ab ba config.tar.foo config.tar 
-   test_cmp b.tar config.tar 
+   cmp b.tar config.tar 
git archive --format=bar HEAD config.bar 
tr ab ba config.bar config.tar 
-   test_cmp b.tar config.tar
+   cmp b.tar config.tar
 '
 
 test_expect_success 'invoke tar filter by extension' '
git archive -o config-implicit.tar.foo HEAD 
-   test_cmp config.tar.foo config-implicit.tar.foo 
+   cmp config.tar.foo config-implicit.tar.foo 
git archive -o config-implicit.bar HEAD 
-   test_cmp config.tar.foo config-implicit.bar
+   cmp config.tar.foo config-implicit.bar
 '
 
 test_expect_success 'default output format remains tar' '
git archive -o config-implicit.baz HEAD 
-   test_cmp b.tar config-implicit.baz
+   cmp b.tar config-implicit.baz
 '
 
 test_expect_success 'extension matching requires dot' '
git archive -o config-implicittar.foo HEAD 
-   test_cmp b.tar config-implicittar.foo
+   cmp b.tar config-implicittar.foo
 '
 
 test_expect_success 'only enabled filters are available remotely' '
test_must_fail git archive --remote=. --format=tar.foo HEAD \
remote.tar.foo 
git archive --remote=. --format=bar remote.bar HEAD 
-   test_cmp remote.bar config.bar
+   cmp remote.bar config.bar
 '
 
 test_expect_success GZIP 'git archive --format=tgz' '
@@ -280,27 +280,27 @@ test_expect_success GZIP 'git archive --format=tgz' '
 
 test_expect_success GZIP 'git archive --format=tar.gz' '
git archive --format=tar.gz HEAD j1.tar.gz 
-   test_cmp j.tgz j1.tar.gz
+   cmp j.tgz j1.tar.gz
 '
 
 test_expect_success GZIP 'infer tgz from .tgz filename' '
git archive --output=j2.tgz HEAD 
-   test_cmp j.tgz j2.tgz
+   cmp j.tgz j2.tgz
 '
 
 test_expect_success GZIP 'infer tgz from .tar.gz filename' '
git archive --output=j3.tar.gz HEAD 
-   test_cmp j.tgz j3.tar.gz
+   cmp j.tgz j3.tar.gz
 '
 
 test_expect_success GZIP 'extract tgz file' '
gzip -d -c j.tgz j.tar 
-   test_cmp b.tar j.tar
+   cmp b.tar j.tar
 '
 
 test_expect_success GZIP 'remote tar.gz is allowed by default' '
git archive --remote=. --format=tar.gz HEAD remote.tar.gz 
-   test_cmp j.tgz remote.tar.gz
+   cmp j.tgz remote.tar.gz
 '
 
 test_expect_success GZIP 'remote tar.gz can be disabled' '
diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh
index 51dedab..dfc35b3 100755
--- a/t/t5001-archive-attr.sh
+++ b/t/t5001-archive-attr.sh
@@ -68,7 +68,7 @@ test_expect_missing   worktree2/ignored-by-worktree
 
 test_expect_success 'git archive vs. bare' '
(cd bare  git archive HEAD) bare-archive.tar 
-   test_cmp archive.tar bare-archive.tar
+   cmp archive.tar

Re: [PATCH] t5000, t5003: do not use test_cmp to compare binary files

2014-06-04 Thread Thomas Braun 
Am 04.06.2014 13:42, schrieb Stepan Kasal:
 test_cmp() is primarily meant to compare text files (and display the
 difference for debug purposes).
 
 Raw cmp is better suited to compare binary files (tar, zip, etc.).
 
 On MinGW, test_cmp is a shell function mingw_test_cmp that tries to
 read both files into environment, stripping CR characters (introduced
 in commit 4d715ac0).
 
 This function usually speeds things up, as fork is extremly slow on
 Windows.  But no wonder that this function is extremely slow and
 sometimes even crashes when comparing large tar or zip files.
 
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 ---
  t/t5000-tar-tree.sh | 34 +-
  t/t5001-archive-attr.sh |  2 +-
  t/t5003-archive-zip.sh  |  6 +++---
  t/t5004-archive-corner-cases.sh |  2 +-
  4 files changed, 22 insertions(+), 22 deletions(-)
 
 diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
 index 1cf0a4e..31b1fd1 100755
 --- a/t/t5000-tar-tree.sh
 +++ b/t/t5000-tar-tree.sh
 @@ -164,7 +164,7 @@ check_tar with_olde-prefix olde-
  test_expect_success 'git archive on large files' '
  test_config core.bigfilethreshold 1 
  git archive HEAD b3.tar 
 -test_cmp b.tar b3.tar
 +cmp b.tar b3.tar
  '

Wouldn't a function like test_cmp_bin() be better suited for all?
The windows folks can then use cmp inside test_cmp_bin() and all others
just use test_cmp.

--
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] t5000, t5003: do not use test_cmp to compare binary files

2014-06-04 Thread Stepan Kasal 
Hello Thomas,

On Wed, Jun 04, 2014 at 02:13:44PM +0200, Thomas Braun wrote:
 Wouldn't a function like test_cmp_bin() be better suited for all?

I also considered it.  The advantage is that is shows that
this intentionally differs from test_cmp.

 The windows folks can then use cmp inside test_cmp_bin() and all others

... would use cmp as well because it is better suited for the task
than diff -u.  So test_cmp_bin would be just an alias for cmp, on all
platforms.  Doesn't that sound weird?

Stepan
--
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: [msysGit] Re: [PATCH] t5000, t5003: do not use test_cmp to compare binary files

2014-06-04 Thread Thomas Braun 
Hi Stephan,

Am 04.06.2014 14:42, schrieb Stepan Kasal:
 On Wed, Jun 04, 2014 at 02:13:44PM +0200, Thomas Braun wrote:
 Wouldn't a function like test_cmp_bin() be better suited for all?

 I also considered it.  The advantage is that is shows that
 this intentionally differs from test_cmp.

 The windows folks can then use cmp inside test_cmp_bin() and all others

 ... would use cmp as well because it is better suited for the task
 than diff -u.  So test_cmp_bin would be just an alias for cmp, on all
 platforms.  Doesn't that sound weird?

I actually like the idea that the test assertions follow a common naming
scheme and can easily be overriden by $arbitrary-crazy-platform.

Using test_cmp_bin instead of cmp would result in then four assertions
for comparing arbitrary data
test_cmp
test_18ncmp
test_cmp_text
test_cmp_bin
where I think the purpose of each function is clear from its name.
--
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