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?
I haven't looked at the context closely --- another option could be a
note in the commit message about how that '! cmp' line was not testing
anything useful in the first place.
[...]
> @@ -209,11 +217,8 @@ test_expect_success \
> git ls-files --stage >14.out &&
> test_must_fail git diff -U0 --no-index M.out 14.out >14diff.out &&
> compare_change 14diff.out expected &&
> - sum bozbar frotz >actual14.sum &&
> - grep -v nitfol M.sum > expected14.sum &&
> - cmp expected14.sum actual14.sum &&
> - sum bozbar frotz nitfol >actual14a.sum &&
> - if cmp M.sum actual14a.sum; then false; else :; fi &&
> + test_cmp bozbar.M bozbar &&
> + test_cmp frotz.M frotz &&
Same question here: the preimage seems to be a stricter test than the
postimage.
[...]
> @@ -231,11 +236,8 @@ test_expect_success \
> test_must_fail git diff -U0 --no-index M.out 15.out >15diff.out &&
> compare_change 15diff.out expected &&
> check_cache_at nitfol dirty &&
> - sum bozbar frotz >actual15.sum &&
> - grep -v nitfol M.sum > expected15.sum &&
> - cmp expected15.sum actual15.sum &&
> - sum bozbar frotz nitfol >actual15a.sum &&
> - if cmp M.sum actual15a.sum; then false; else :; fi &&
> + test_cmp bozbar.M bozbar &&
> + test_cmp frotz.M frotz &&
Likewise.
[...]
> @@ -281,11 +285,8 @@ test_expect_success \
> git ls-files --stage >19.out &&
> test_cmp M.out 19.out &&
> check_cache_at bozbar dirty &&
> - sum frotz nitfol >actual19.sum &&
> - grep -v bozbar M.sum > expected19.sum &&
> - cmp expected19.sum actual19.sum &&
> - sum bozbar frotz nitfol >actual19a.sum &&
> - if cmp M.sum actual19a.sum; then false; else :; fi &&
> + test_cmp frotz.M frotz &&
> + test_cmp nitfol.M nitfol &&
Likewise.
The rest looks good.
Thanks,
Jonathan
