git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Aug 15, 2017, 00:46 UTC
Message-ID
<20170815004546.GA78174@aiede.mtv.corp.google.com>
In-Reply-To
<9f6e13d3-07ff-1eaa-9453-05ca26a3c1ff@web.de>
Hi,
René Scharfe wrote:
Show 32 quoted lines
> 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
[...]
Show 8 quoted lines
> @@ -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.

[...]
Show 11 quoted lines
> @@ -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.

[...]
Show 11 quoted lines
> @@ -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.
[...]
Show 11 quoted lines
> @@ -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

Previous: Junio C HamanoNext: Johannes Sixt
Message 3 of 6 in “t1002: stop using sum(1)”
  1. t1002: stop using sum(1)René Scharfe, Aug 14, 2017
  2. Junio C HamanoAug 14, 2017
  3. Jonathan NiederAug 15, 2017
  4. Johannes SixtAug 15, 2017
  5. René ScharfeAug 15, 2017
  6. René ScharfeAug 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.