git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johan Herland <johan@herland.net>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Michael Blume <blume.mike@gmail.com>,
	Git List <git@vger.kernel.org>
Subject: Re: mac test failure -- test 3301
Date: Mon, 10 Nov 2014 21:41:29 -0500	[thread overview]
Message-ID: <20141111024128.GA21328@peff.net> (raw)
In-Reply-To: <CALKQrgexnrCy-P6yTZB+U0--5peaTys9KqB+KGKqbAuyLA=NqA@mail.gmail.com>

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

  parent reply	other threads:[~2014-11-11  2:41 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-11  0:17 mac test failure -- test 3301 Michael Blume
2014-11-11  0:23 ` Michael Blume
2014-11-11  0:46   ` Michael Blume
2014-11-11  1:19 ` Eric Sunshine
2014-11-11  1:40   ` Johan Herland
2014-11-11  1:46     ` Michael Blume
2014-11-11  2:41     ` Jeff King [this message]
2014-11-11 23:57       ` Johan Herland
2014-11-11 15:58     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141111024128.GA21328@peff.net \
    --to=peff@peff.net \
    --cc=blume.mike@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).