Re: [PATCH 2/2] Allow passing pipes for input pipes to diff --no-index

From: Jeff King <peff@peff.net>
Date: Sep 18, 2020, 17:19 UTC
Message-ID: <20200918171950.GA183026@coredump.intra.peff.net>
In-Reply-To: <CALqVohfFjsh-2jZLNNwON_V95Dfh-aEh1aMb53t4NQrM0qz1tQ@mail.gmail.com>

On Fri, Sep 18, 2020 at 12:34:48PM -0400, Thomas Guyot-Sionnest wrote:

Show 18 quoted lines

> Hi Taylor,
> 
> On Fri, 18 Sep 2020 at 10:36, Taylor Blau <me@ttaylorr.com> wrote:
> > On Fri, Sep 18, 2020 at 07:32:56AM -0400, Thomas Guyot-Sionnest wrote:
> > > A very handy way to pass data to applications is to use the <() process
> > > substitution syntax in bash variants. It allow comparing files streamed
> > > from a remote server or doing on-the-fly stream processing to alter the
> > > diff. These are usually implemented as a symlink that points to a bogus
> > > name (ex "pipe:[209326419]") but opens as a pipe.
> >
> > This is true in bash, but sh does not support process substitution with
> > <().
> 
> Bash, ksh, zsh and likely any more moden shell. Other programming
> languages also setup such pipes. It's much cleaner than creating temp
> files and cleaning them up and in some cases faster too (I've ran
> diff's like this over GB's of test data, it's very handy to remove
> known patterns that would cause needless diffs).

Yeah, it's definitely a reasonable thing to want (see below). And from a portability perspective, it is outside of Git's scope; users with those shells can use the feature, and people on other shells don't have to care.

But we do have to account for this in the test suite, which must be able to run under a vanilla POSIX shell. So you'd probably want to set up a prerequisite that lets us skip these tests on other shells, like:

  test_lazy_prereq PROCESS_SUBSTITUTION '
	echo foo >expect &&
	cat >actual <(echo foo) &&
	test_cmp expect actual
  '

  test_expect_success PROCESS_SUBSTITUTION 'some test...' '
	# safe because we skip this test on shells that do not support it
	git diff --no-index <(cat whatever)
  '

Though it is a little sad that people running the suite with a vanilla /bin/sh like dash wouldn't ever run the tests. I wonder if there's a more portable way to formulate it.

Getting back to the overall feature, this is definitely something that has come up before. The last I know of is:

  https://lore.kernel.org/git/20181220002610.43832-1-sandals@crustytoothpaste.net/

which everybody seemed to like the direction of; I suspect the original author (cc'd) just never got around to it again. Compared to this approach, it uses a command-line option to avoid dereferencing symlinks. That puts an extra burden on the caller to pass the option, but it's way less magical; you could drop all of the "does this look like a symlink to a pipe" heuristics. It would also be much easier to test. ;)

-Peff