git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Taylor Blau <me@ttaylorr.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	Thomas Guyot-Sionnest <tguyot@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/4] diff --no-index: support reading from named pipes
Date: Wed,  5 Jul 2023 20:49:26 +0100	[thread overview]
Message-ID: <cover.1688586536.git.phillip.wood@dunelm.org.uk> (raw)
In-Reply-To: <cover.1687874975.git.phillip.wood@dunelm.org.uk>

From: Phillip Wood <phillip.wood@dunelm.org.uk>

In some shells, such as bash and zsh, it's possible to use a command
substitution to provide the output of a command as a file argument to
another process, like so:

  diff -u <(printf "a\nb\n") <(printf "a\nc\n")

However, this syntax does not produce useful results with git diff
--no-index. On macOS, the arguments to the command are named pipes
under /dev/fd, and git diff doesn't know how to handle a named
pipe. On Linux, the arguments are symlinks to pipes, so git diff
"helpfully" diffs these symlinks, comparing their targets like
"pipe:[1234]" and "pipe:[5678]".

There have been at least three previous attempts [1-3] to address this
issue. They all seem to have received broad support for the aim of
supporting process substitutions but have foundered on details of the
implementation. In an effort to avoid the same fate this series is
narrowly focussed on making command substitutions work with "diff
--no-index" and does not attempt to add a general facility for
de-referencing symbolic links or reading from pipes to the diff
machinery.

The only functional change is that if a path given on the commandline
is a named pipe or a symbolic link that resolves to a named pipe then
we read the data to diff from that pipe. The first two patches improve
the error handling when reading from stdin and add a test. The third
patch implements support for reading from pipes.

This cover letter and the commit message for the third patch are
largely copied from brian’s patch[2] - do we have a standard commit
trailer for "I stole the commit message from someone else's patch"?

I've cc’d the participants of the discussion of the last attempt[1] to
fix this.

Thanks to Junio for his comments, the changes since v1 are:

* Patch 1: - This is new and changes the code to error out if the user
	     tries to compare stdin to a directory.

* Patch 4: - Changed the implementation of get_mode() to treat stdin and
	     named pipes as special. (suggested by Junio)
	   - Added separate functions to populate a diff_filespec from
	     a stdin or a named pipe. (suggested by Junio)
	   - It now dies if the user tries to compare a named pipe to a
	     directory and added a test for this. (suggested by Junio)
	   - The test for comparing named pipes now cleans up the
	     background processes if it fails.

[1] https://lore.kernel.org/git/20200918113256.8699-3-tguyot@gmail.com/
[2] https://lore.kernel.org/git/20181220002610.43832-1-sandals@crustytoothpaste.net/
[3] https://public-inbox.org/git/20170113102021.6054-1-dennis@kaarsemaker.net/


base-commit: 94486b6763c29144c60932829a65fec0597e17b3
Published-As: https://github.com/phillipwood/git/releases/tag/diff-no-index-pipes%2Fv2
View-Changes-At: https://github.com/phillipwood/git/compare/94486b676...4e05a0be5
Fetch-It-Via: git fetch https://github.com/phillipwood/git diff-no-index-pipes/v2

Phillip Wood (4):
  diff --no-index: refuse to compare stdin to a directory
  diff --no-index: die on error reading stdin
  t4054: test diff --no-index with stdin
  diff --no-index: support reading from named pipes

 diff-no-index.c          | 126 ++++++++++++++++++++++++++++-----------
 t/t4053-diff-no-index.sh |  64 ++++++++++++++++++++
 2 files changed, 156 insertions(+), 34 deletions(-)

Range-diff against v1:
-:  ---------- > 1:  5e65a15223 diff --no-index: refuse to compare stdin to a directory
1:  5dad728f3b = 2:  be1d666769 diff --no-index: die on error reading stdin
2:  b94d59034f ! 3:  1c7db4dbe2 t4054: test diff --no-index with stdin
    @@ t/t4053-diff-no-index.sh: test_expect_success POSIXPERM,SYMLINKS 'diff --no-inde
     +	test_write_lines 1 | git diff --no-index -- a/1 - >actual &&
     +	test_must_be_empty actual
     +'
    - test_done
    ++
    + test_expect_success 'diff --no-index refuses to diff stdin and a directory' '
    + 	test_must_fail git diff --no-index -- - a </dev/null 2>err &&
    + 	grep "fatal: cannot compare stdin to a directory" err
3:  990e71882b < -:  ---------- diff --no-index: support reading from named pipes
-:  ---------- > 4:  4e05a0be54 diff --no-index: support reading from named pipes

base-commit: 94486b6763c29144c60932829a65fec0597e17b3
-- 
2.40.1.852.g0a1e0755a6


  parent reply	other threads:[~2023-07-05 19:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 14:10 [PATCH 0/3] diff --no-index: support reading from named pipes Phillip Wood
2023-06-27 14:10 ` [PATCH 1/3] diff --no-index: die on error reading stdin Phillip Wood
2023-06-27 14:10 ` [PATCH 2/3] t4054: test diff --no-index with stdin Phillip Wood
2023-06-27 14:10 ` [PATCH 3/3] diff --no-index: support reading from named pipes Phillip Wood
2023-06-27 19:44   ` Junio C Hamano
2023-06-28 10:05     ` Phillip Wood
2023-07-05 19:49 ` Phillip Wood [this message]
2023-07-05 19:49   ` [PATCH v2 1/4] diff --no-index: refuse to compare stdin to a directory Phillip Wood
2023-07-05 21:17     ` Junio C Hamano
2023-07-05 19:49   ` [PATCH v2 2/4] diff --no-index: die on error reading stdin Phillip Wood
2023-07-05 21:18     ` Junio C Hamano
2023-07-05 19:49   ` [PATCH v2 3/4] t4054: test diff --no-index with stdin Phillip Wood
2023-07-05 21:22     ` Junio C Hamano
2023-07-05 19:49   ` [PATCH v2 4/4] diff --no-index: support reading from named pipes Phillip Wood
2023-07-05 22:19     ` Junio C Hamano
2023-08-09 17:17     ` Jeff King
2023-08-10 12:56       ` Phillip Wood

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=cover.1688586536.git.phillip.wood@dunelm.org.uk \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=sandals@crustytoothpaste.net \
    --cc=tguyot@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).