Re: [PATCH v2 2/3] transport.c: introduce core.alternateRefsCommand

From: Jeff King <peff@peff.net>
Date: Sep 26, 2018, 03:33 UTC
Message-ID: <20180926033337.GC32064@sigill.intra.peff.net>
In-Reply-To: <20180926010935.GK4364@syl>

On Tue, Sep 25, 2018 at 06:09:35PM -0700, Taylor Blau wrote:

Show 8 quoted lines

> > So I think this is fine (modulo that the grep and sed can be combined).
> > Yet another option would be to simply strip away everything except the
> > object id (which is all we care about), like:
> >
> >   depacketize | perl -lne '/^(\S+) \.have/ and print $1'
> 
> Thanks for this. This is the suggestion I ended up taking (modulo taking
> '-' as the first argument to 'depacketize').

I don't think depacketize takes any arguments. It always reads from stdin directly, doesn't it? Your "-" is not hurting anything, but it is totally ignored.

A perl tangent if you're interested:

  Normally for shell functions like this that are just wrappers around
  perl snippets, I would suggest to pass "$@" from the function's
  arguments to perl. So for example if we had:

    haves_from_packets () {
	perl -lne '/^(\S+) \.have/ and print $1' "$@"
    }

  then you could call it with a filename:

    haves_from_packets packets

  or input on stdin:

    haves_from_packets <packets

  and either works (this is magic from perl's "-p" loop, but you get the
  same if you write "while (<>)" explicitly in your program).

  But because depacketize() has to use byte-wise read() calls, it
  doesn't get that magic for free. And it did not seem worth the effort
  to implement, when shell redirections are so easy. ;)

  Just skimming through test-lib-functions.sh, though, it does seem that
  we often deviate from that pattern (e.g., all of the q_to_nul family).
  And has seemed to mind.

> The 'print $1' part of this makes things a lot nicer, actually, having
> removed the " .have" suffix. We can get rid of the expect_haves()
> function above, and instead call 'git rev-parse' inline and get the
> right results.

Yes. You can even do it all in a single rev-parse call.

-Peff