Subject: Re: mac test failure -- test 3301

From: Jeff King <[email-masked]>
Date: Mon, 10 Nov 2014 21:41:29 -0500

On Tue, Nov 11, 2014 at 02:40:19AM +0100, Johan Herland wrote:

> > This and all other failures are due to the output of 'wc -l', which on
> > Mac is "{whitespace}1" rather than just "1" as it is on other
> > platforms. fbe4f748 added quotes around the $(... | wc -l) invocation
> > which caused the whitespace to be retained. A minimum fix would be to
> > drop the quotes surrounding $().
> 
> Ah, thanks!
> 
> I thought that quoting command output was a good idea in general. Am I
> wrong, or is this just one exception to an otherwise good guideline?

It usually is a good idea to prevent whitespace splitting by the shell
(and especially with things that might unexpectedly be empty, in which
case they end up as "no argument" and not an empty one). So yeah, this
is an exception.

> Anyway, here is the minimal diff to remove quoting from the $(... | wc
> -l) commands (probably whitespace damaged by GMail - sorry). Junio:
> feel free to squash this in, or ask for a re-roll:

I think we have test_line_count exactly because of this unportability
with wc output.

You'd want:

  git ls-tree refs/notes/commits >actual &&
  test_line_count = 1 actual

or similar.

By the way, the point of that "ls-tree" appears to be to check the
number of total notes stored. Since notes are stored in a hashed
structure, should it be "ls-tree -r"? Otherwise, we see only the leading
directories; two notes whose sha1s start with the same two characters
would be considered a single entry.

-Peff