Re: [PATCH] t1002: stop using sum(1)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 15.08.2017 um 02:46 schrieb Jonathan Nieder:
Hi,

René Scharfe wrote:

sum(1) is a command for calculating checksums of the contents of files.
It was part of early editions of Unix ("Research Unix", 1972/1973, [1]).
cksum(1) appeared in 4.4BSD (1993) as a replacement [2], and became part
of POSIX.1-2008 [3].  OpenBSD 5.6 (2014) removed sum(1).

We only use sum(1) in t1002 to check for changes in three files.  On
MinGW we use md5sum(1) instead.  We could switch to the standard command
cksum(1) for all platforms; MinGW comes with GNU coreutils now, which
provides sum(1), cksum(1) and md5sum(1).  Use our standard method for
checking for file changes instead: test_cmp.

It's more convenient because it shows differences nicely, it's faster on
MinGW because we have a special implementation there based only on
shell-internal commands, it's simpler as it allows us to avoid stripping
out unnecessary entries from the checksum file using grep(1), and it's
more consistent with the rest of the test suite.

We already compare changed files with their expected new contents using
diff(1), so we don't need to check with "test_must_fail test_cmp" if
they differ from their original state.  A later patch could convert the
direct diff(1) calls to test_cmp as well.

With all sum(1) calls gone, remove the MinGW-specific implementation
from test-lib.sh as well.

[1] http://minnie.tuhs.org/cgi-bin/utree.pl?file=V3/man/man1/sum.1
[2] http://minnie.tuhs.org/cgi-bin/utree.pl?file=4.4BSD/usr/share/man/cat1/cksum.0
[3] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cksum.html
---
  t/t1002-read-tree-m-u-2way.sh | 67 ++++++++++++++++++++++---------------------
  t/test-lib.sh                 |  3 --
  2 files changed, 35 insertions(+), 35 deletions(-)

Nicely analyzed.  May we forge your sign-off?

[...]
--- a/t/t1002-read-tree-m-u-2way.sh
+++ b/t/t1002-read-tree-m-u-2way.sh
[...]
@@ -132,8 +138,8 @@ test_expect_success \
       git ls-files --stage >7.out &&
       test_cmp M.out 7.out &&
       check_cache_at frotz dirty &&
-     sum bozbar frotz nitfol >actual7.sum &&
-     if cmp M.sum actual7.sum; then false; else :; fi &&
+     test_cmp bozbar.M bozbar &&
+     test_cmp nitfol.M nitfol &&

This one is strange.  What is that '! cmp' trying to check for?
Does the replacement capture the same thing?

E.g., does it need a '! test_cmp frotz.M frotz &&' line?

In the context that you stripped a 'diff frotz frotz1' occurs, i.e., a check for the expected content of the file whose content changes. Together with the new test_cmp lines, the new text is a stricter check than we have in the old text.

The patch looks good.

Reviewed-by: Johannes Sixt <j6t@xxxxxxxx>

-- Hannes



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux