Re: Bug: problem with file named with dash character

From: Jeff King <peff@peff.net>
Date: Jun 27, 2012, 19:52 UTC
Message-ID: <20120627195205.GA11700@sigill.intra.peff.net>
In-Reply-To: <7vk3ys8v96.fsf@alter.siamese.dyndns.org>

On Wed, Jun 27, 2012 at 11:28:21AM -0700, Junio C Hamano wrote:

> Thanks for a report.  I think the "diff" callchain should be
> refactored so that the caller can mark the special "stdin" token in
> a saner way, but until it happens, the following one-liner should
> do.

Yeah, I assume this is there at all to support "diff --no-index -", so the special marking should happen at that layer.

Show 16 quoted lines

> diff --git a/diff.c b/diff.c
> index 1a594df..caa2309 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2589,6 +2589,14 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
>  	if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))
>  		return 0;
>  
> +	/*
> +	 * And asking to read "-" from the working tree triggers stdin
> +	 * input (which needs to be fixed separately by refactoring the
> +	 * callchain), forbid "reuse" for now.
> +	 */
> +	if (!strcmp(name, "-"))
> +		return 0;
> +

Unfortunately this is not enough. The problematic code path is the call to populate_from_stdin in diff_populate_filespec. And we follow that conditional if reuse_worktree_file is true, _or_ if the sha1_valid flag on the filespec is not set. We hit the latter due to the --no-index case, but we can also hit it if we are comparing a working tree file in the repo that is stat-dirty.

So without your patch, this reads from stdin:

  git init repo &&
  cd repo &&
  echo foo >- &&
  git add - &&
  git commit -m foo

when the commit command tries to generate the diff summary. Your patch fixes it, but remains broken if you then do:

  echo changes >>- &&
  git diff

I think you'd want to do just do something like:

diff --git a/diff.c b/diff.c
index 1a594df..aac72b7 100644
--- a/diff.c
+++ b/diff.c
@@ -2684,9 +2684,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 		struct stat st;
 		int fd;
 
-		if (!strcmp(s->path, "-"))
-			return populate_from_stdin(s);
-
 		if (lstat(s->path, &st) < 0) {
 			if (errno == ENOENT) {
 			err_empty:

to temporarily fix it. That breaks

  echo content | git diff --no-index - some-file

but that code path should be fixed properly (with a use_stdin flag in
the filespec).

-Peff