Re: mac test failure -- test 3301

[Johan Herland <johan@herland.net>]

On Tue, Nov 11, 2014 at 3:41 AM, Jeff King <peff@peff.net> wrote:
> 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.

Thanks. Will use this in the re-roll.

> 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.

Yes and no... The notes' nested fanout structure is developed
dynamically as the number of notes increase. As long as the number of
notes stay low, the notes tree remain at fanout level 0 (i.e. no
nesting), so ls-tree happens to work here. Still ls-tree -r is the
correct for any size notes tree. Will fix to use ls-tree -r.


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net