{"thread":{"id":"30913","subject":"Bug: problem with file named with dash character","startedAt":"2012-06-27T07:32:21Z","lastAt":"2012-06-27T22:41:29Z","messageCount":13,"participants":["Daniel Lyubomirov -|- Digitalus Bulgaria","faux@goeswhere.com","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"194334","messageId":"80a7ce17-3ee7-4f09-b984-b6685e163b87@office.digitalus.nl","threadId":"30913","inReplyTo":"52ae7682-3e9a-4b52-bec1-08ba3aadffc0@office.digitalus.nl","subject":"Bug: problem with file named with dash character","fromName":"Daniel Lyubomirov -|- Digitalus Bulgaria","fromEmail":"daniel@digitalus.bg","sentAt":"2012-06-27T07:32:21Z","receivedAt":"2012-06-27T07:32:21Z","isPatch":false,"sender":{"key":"daniel@digitalus.bg","avatar":null},"body":"Hi,\n\nАccidentally my colleague created a file in the root dir of the git repo called - (just dash).\nAs result for every commit having this file, diff , merge, cherry-pick maybe others just hang.\nI tried to make rebase interactive to edit the commit and remove the file but that fails too.\n\nsystem: Fedora Linux 3.4.2-1.fc16.x86_64\ngit version: 1.7.7.6\n\nHow to reproduce the bug:\n1. create a file named -\n2. add the file in the index and commit\n3. try diff with another commit, branch\n\nSo far i found following workarounds:\n1. You can delete the file. Then the problem stays in the history only for the commits that have the file.\n2. Use git filter-branch and deleted the file from the history. \n\n   git filter-branch --index-filter \"git rm -rf --cached --ignore-unmatch -- -\" <problem commit>..HEAD\n\n   But this changes the history.\n\n\nThanks for your work.\n\nRegards,\nDaniel Lyubomirov\n"},{"id":"194342","messageId":"20120627095735.GA19118@goeswhere.com","threadId":"30913","inReplyTo":"80a7ce17-3ee7-4f09-b984-b6685e163b87@office.digitalus.nl","subject":"Re: Bug: problem with file named with dash character","fromName":"","fromEmail":"faux@goeswhere.com","sentAt":"2012-06-27T09:57:35Z","receivedAt":"2012-06-27T09:57:35Z","isPatch":false,"sender":{"key":"faux@goeswhere.com","avatar":null},"body":"On Wed, Jun 27, 2012 at 09:32:21AM +0200, Daniel Lyubomirov -|- Digitalus Bulgaria wrote:\n> Hi,\n> \n> Аccidentally my colleague created a file in the root dir of the git repo called - (just dash).\n> As result for every commit having this file, diff , merge, cherry-pick maybe others just hang.\n> I tried to make rebase interactive to edit the commit and remove the file but that fails too.\n\nHaha, it tries to read from standard in:\n\n$ git init && printf '' >- && git add -- - && git commit -m \"lol\"\n\nPressing ^D allows it to continue.\n"},{"id":"194363","messageId":"7vk3ys8v96.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"80a7ce17-3ee7-4f09-b984-b6685e163b87@office.digitalus.nl","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T18:28:21Z","receivedAt":"2012-06-27T18:28:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Lyubomirov -|- Digitalus Bulgaria <daniel@digitalus.bg>\nwrites:\n\n> Аccidentally my colleague created a file in the root dir of the git repo called - (just dash).\n> As result for every commit having this file, diff , merge, cherry-pick maybe others just hang.\n\nThanks for a report.  I think the \"diff\" callchain should be\nrefactored so that the caller can mark the special \"stdin\" token in\na saner way, but until it happens, the following one-liner should\ndo.\n\n\n diff.c | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 1a594df..caa2309 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2589,6 +2589,14 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int\n \tif (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))\n \t\treturn 0;\n \n+\t/*\n+\t * And asking to read \"-\" from the working tree triggers stdin\n+\t * input (which needs to be fixed separately by refactoring the\n+\t * callchain), forbid \"reuse\" for now.\n+\t */\n+\tif (!strcmp(name, \"-\"))\n+\t\treturn 0;\n+\n \tlen = strlen(name);\n \tpos = cache_name_pos(name, len);\n \tif (pos < 0)\n"},{"id":"194365","messageId":"20120627195205.GA11700@sigill.intra.peff.net","threadId":"30913","inReplyTo":"7vk3ys8v96.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-27T19:52:05Z","receivedAt":"2012-06-27T19:52:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 27, 2012 at 11:28:21AM -0700, Junio C Hamano wrote:\n\n> Thanks for a report.  I think the \"diff\" callchain should be\n> refactored so that the caller can mark the special \"stdin\" token in\n> a saner way, but until it happens, the following one-liner should\n> do.\n\nYeah, I assume this is there at all to support \"diff --no-index -\", so the\nspecial marking should happen at that layer.\n\n> diff --git a/diff.c b/diff.c\n> index 1a594df..caa2309 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2589,6 +2589,14 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int\n>  \tif (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))\n>  \t\treturn 0;\n>  \n> +\t/*\n> +\t * And asking to read \"-\" from the working tree triggers stdin\n> +\t * input (which needs to be fixed separately by refactoring the\n> +\t * callchain), forbid \"reuse\" for now.\n> +\t */\n> +\tif (!strcmp(name, \"-\"))\n> +\t\treturn 0;\n> +\n\nUnfortunately this is not enough. The problematic code path is the call\nto populate_from_stdin in diff_populate_filespec. And we follow that\nconditional if reuse_worktree_file is true, _or_ if the sha1_valid flag\non the filespec is not set. We hit the latter due to the --no-index\ncase, but we can also hit it if we are comparing a working tree file in\nthe repo that is stat-dirty.\n\nSo without your patch, this reads from stdin:\n\n  git init repo &&\n  cd repo &&\n  echo foo >- &&\n  git add - &&\n  git commit -m foo\n\nwhen the commit command tries to generate the diff summary. Your patch\nfixes it, but remains broken if you then do:\n\n  echo changes >>- &&\n  git diff\n\nI think you'd want to do just do something like:\n\ndiff --git a/diff.c b/diff.c\nindex 1a594df..aac72b7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2684,9 +2684,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\tstruct stat st;\n \t\tint fd;\n \n-\t\tif (!strcmp(s->path, \"-\"))\n-\t\t\treturn populate_from_stdin(s);\n-\n \t\tif (lstat(s->path, &st) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\terr_empty:\n\nto temporarily fix it. That breaks\n\n  echo content | git diff --no-index - some-file\n\nbut that code path should be fixed properly (with a use_stdin flag in\nthe filespec).\n\n-Peff\n"},{"id":"194366","messageId":"20120627202500.GB11700@sigill.intra.peff.net","threadId":"30913","inReplyTo":"20120627195205.GA11700@sigill.intra.peff.net","subject":"Re: Bug: problem with file named with dash character","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-27T20:25:00Z","receivedAt":"2012-06-27T20:25:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 27, 2012 at 03:52:05PM -0400, Jeff King wrote:\n\n> I think you'd want to do just do something like:\n> \n> diff --git a/diff.c b/diff.c\n> index 1a594df..aac72b7 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2684,9 +2684,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n>  \t\tstruct stat st;\n>  \t\tint fd;\n>  \n> -\t\tif (!strcmp(s->path, \"-\"))\n> -\t\t\treturn populate_from_stdin(s);\n> -\n>  \t\tif (lstat(s->path, &st) < 0) {\n>  \t\t\tif (errno == ENOENT) {\n>  \t\t\terr_empty:\n> \n> to temporarily fix it. That breaks\n> \n>   echo content | git diff --no-index - some-file\n> \n> but that code path should be fixed properly (with a use_stdin flag in\n> the filespec).\n\nSomething like the patch below, which keeps the stdin test in t4002\nworking for me.\n\nI suspect there are other problems lurking with the stdin case. For\nexample, we try to drop filespec contents whenever we can to reduce\nmemory pressure, under the assumption that we can always re-read the\nblob later. But with stdin, we would need to be careful to mark the\ncontents as precious somehow.\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f0b0010..c64bd5c 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -51,6 +51,21 @@ static int get_mode(const char *path, int *mode)\n \treturn 0;\n }\n \n+static struct diff_filespec *noindex_filespec(const char *name, int mode)\n+{\n+\tstruct diff_filespec *r;\n+\n+\tif (!name)\n+\t\tname = \"/dev/null\";\n+\tr = alloc_filespec(name);\n+\n+\tfill_filespec(r, null_sha1, mode);\n+\tif (!strcmp(name, \"-\"))\n+\t\tr->is_stdin = 1;\n+\n+\treturn r;\n+}\n+\n static int queue_diff(struct diff_options *o,\n \t\t      const char *name1, const char *name2)\n {\n@@ -137,15 +152,8 @@ static int queue_diff(struct diff_options *o,\n \t\t\ttmp_c = name1; name1 = name2; name2 = tmp_c;\n \t\t}\n \n-\t\tif (!name1)\n-\t\t\tname1 = \"/dev/null\";\n-\t\tif (!name2)\n-\t\t\tname2 = \"/dev/null\";\n-\t\td1 = alloc_filespec(name1);\n-\t\td2 = alloc_filespec(name2);\n-\t\tfill_filespec(d1, null_sha1, mode1);\n-\t\tfill_filespec(d2, null_sha1, mode2);\n-\n+\t\td1 = noindex_filespec(name1, mode1);\n+\t\td2 = noindex_filespec(name2, mode2);\n \t\tdiff_queue(&diff_queued_diff, d1, d2);\n \t\treturn 0;\n \t}\ndiff --git a/diff.c b/diff.c\nindex 1a594df..449bfba 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2675,6 +2675,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \tif (size_only && 0 < s->size)\n \t\treturn 0;\n \n+\tif (s->is_stdin)\n+\t\treturn populate_from_stdin(s);\n+\n \tif (S_ISGITLINK(s->mode))\n \t\treturn diff_populate_gitlink(s, size_only);\n \n@@ -2684,9 +2687,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\tstruct stat st;\n \t\tint fd;\n \n-\t\tif (!strcmp(s->path, \"-\"))\n-\t\t\treturn populate_from_stdin(s);\n-\n \t\tif (lstat(s->path, &st) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\terr_empty:\ndiff --git a/diffcore.h b/diffcore.h\nindex 8f32b82..be0739c 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -43,6 +43,7 @@ struct diff_filespec {\n \tunsigned should_free : 1; /* data should be free()'ed */\n \tunsigned should_munmap : 1; /* data should be munmap()'ed */\n \tunsigned dirty_submodule : 2;  /* For submodules: its work tree is dirty */\n+\tunsigned is_stdin : 1;\n #define DIRTY_SUBMODULE_UNTRACKED 1\n #define DIRTY_SUBMODULE_MODIFIED  2\n \tunsigned has_more_entries : 1; /* only appear in combined diff */\n"},{"id":"194367","messageId":"7vfw9g8pqn.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"20120627195205.GA11700@sigill.intra.peff.net","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T20:27:28Z","receivedAt":"2012-06-27T20:27:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> but that code path should be fixed properly (with a use_stdin flag in\n> the filespec).\n\nYes, just as I said; I am finding more and more issues with the\nno-index hack that I have been fixing a bit by bit since I send the\nmessage you responded to.\n"},{"id":"194368","messageId":"7vbok48pfu.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"7vfw9g8pqn.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T20:33:57Z","receivedAt":"2012-06-27T20:33:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> but that code path should be fixed properly (with a use_stdin flag in\n>> the filespec).\n>\n> Yes, just as I said; I am finding more and more issues with the\n> no-index hack that I have been fixing a bit by bit since I send the\n> message you responded to.\n\nIt is not ready yet, but here are a few patches WIP.\n\n-- >8 --\nFrom: Junio C Hamano <gitster@pobox.com>\nDate: Wed, 27 Jun 2012 11:51:15 -0700\nSubject: [PATCH 1/?] diff-index.c: do not pretend paths are pathspecs\n\n\"git diff --no-index\" takes exactly two paths, not pathspecs, and\nhas its own way queue_diff() to populate the diff_queue.  Do not\ncall diff_tree_setup_paths(), pretending as it takes pathspecs.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff-no-index.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f0b0010..ca875da 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -175,6 +175,7 @@ void diff_no_index(struct rev_info *revs,\n \tint i;\n \tint no_index = 0;\n \tunsigned options = 0;\n+\tconst char *paths[2];\n \n \t/* Were we asked to do --no-index explicitly? */\n \tfor (i = 1; i < argc; i++) {\n@@ -233,8 +234,6 @@ void diff_no_index(struct rev_info *revs,\n \n \tif (prefix) {\n \t\tint len = strlen(prefix);\n-\t\tconst char *paths[3];\n-\t\tmemset(paths, 0, sizeof(paths));\n \n \t\tfor (i = 0; i < 2; i++) {\n \t\t\tconst char *p = argv[argc - 2 + i];\n@@ -247,10 +246,10 @@ void diff_no_index(struct rev_info *revs,\n \t\t\t     : p);\n \t\t\tpaths[i] = p;\n \t\t}\n-\t\tdiff_tree_setup_paths(paths, &revs->diffopt);\n+\t} else {\n+\t\tfor (i = 0; i < 2; i++)\n+\t\t\tpaths[i] = argv[argc - 2 + i];\n \t}\n-\telse\n-\t\tdiff_tree_setup_paths(argv + argc - 2, &revs->diffopt);\n \trevs->diffopt.skip_stat_unmatch = 1;\n \tif (!revs->diffopt.output_format)\n \t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n@@ -262,8 +261,7 @@ void diff_no_index(struct rev_info *revs,\n \tif (diff_setup_done(&revs->diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\n \n-\tif (queue_diff(&revs->diffopt, revs->diffopt.pathspec.raw[0],\n-\t\t       revs->diffopt.pathspec.raw[1]))\n+\tif (queue_diff(&revs->diffopt, paths[0], paths[1]))\n \t\texit(1);\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\n-- \n1.7.11.1.184.g3ee8f69\n"},{"id":"194369","messageId":"7v4npw8pdo.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"7vbok48pfu.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T20:35:15Z","receivedAt":"2012-06-27T20:35:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Jeff King <peff@peff.net> writes:\n>>\n>>> but that code path should be fixed properly (with a use_stdin flag in\n>>> the filespec).\n>>\n>> Yes, just as I said; I am finding more and more issues with the\n>> no-index hack that I have been fixing a bit by bit since I send the\n>> message you responded to.\n>\n> It is not ready yet, but here are a few patches WIP.\n\nAnd this is the second clean-up\n\n-- >8 --\nSubject: [PATCH 2/?] diff-index.c: unify handling of command line paths\n\nRegardless of where in the directory hierarchy you are, \"-\" on the\ncommand line means the standard input.  The old code knew too much\nabout how the low level machinery uses paths to read from the\nworking tree and did not bother to have the same check for \"-\" when\nthe command is run from the top-level.\n\nUnify the codepaths for subdirectory case and toplevel case into one\nand make it clearer.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff-no-index.c | 27 +++++++++++----------------\n 1 file changed, 11 insertions(+), 16 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex ca875da..a5c1e3e 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -172,7 +172,7 @@ void diff_no_index(struct rev_info *revs,\n \t\t   int argc, const char **argv,\n \t\t   int nongit, const char *prefix)\n {\n-\tint i;\n+\tint i, prefixlen;\n \tint no_index = 0;\n \tunsigned options = 0;\n \tconst char *paths[2];\n@@ -232,23 +232,18 @@ void diff_no_index(struct rev_info *revs,\n \tif (!DIFF_OPT_TST(&revs->diffopt, EXIT_WITH_STATUS))\n \t\tsetup_pager();\n \n-\tif (prefix) {\n-\t\tint len = strlen(prefix);\n-\n-\t\tfor (i = 0; i < 2; i++) {\n-\t\t\tconst char *p = argv[argc - 2 + i];\n+\tprefixlen = prefix ? strlen(prefix) : 0;\n+\tfor (i = 0; i < 2; i++) {\n+\t\tconst char *p = argv[argc - 2 + i];\n+\t\tif (!strcmp(p, \"-\"))\n \t\t\t/*\n-\t\t\t * stdin should be spelled as '-'; if you have\n-\t\t\t * path that is '-', spell it as ./-.\n+\t\t\t * stdin should be spelled as \"-\"; if you have\n+\t\t\t * path that is \"-\", spell it as \"./-\".\n \t\t\t */\n-\t\t\tp = (strcmp(p, \"-\")\n-\t\t\t     ? xstrdup(prefix_filename(prefix, len, p))\n-\t\t\t     : p);\n-\t\t\tpaths[i] = p;\n-\t\t}\n-\t} else {\n-\t\tfor (i = 0; i < 2; i++)\n-\t\t\tpaths[i] = argv[argc - 2 + i];\n+\t\t\tp = p;\n+\t\telse if (prefixlen)\n+\t\t\tp = xstrdup(prefix_filename(prefix, prefixlen, p));\n+\t\tpaths[i] = p;\n \t}\n \trevs->diffopt.skip_stat_unmatch = 1;\n \tif (!revs->diffopt.output_format)\n-- \n1.7.11.1.184.g3ee8f69\n"},{"id":"194370","messageId":"7vzk7o7am5.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"7v4npw8pdo.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T20:39:30Z","receivedAt":"2012-06-27T20:39:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Jeff King <peff@peff.net> writes:\n>>>\n>>>> but that code path should be fixed properly (with a use_stdin flag in\n>>>> the filespec).\n>>>\n>>> Yes, just as I said; I am finding more and more issues with the\n>>> no-index hack that I have been fixing a bit by bit since I send the\n>>> message you responded to.\n>>\n>> It is not ready yet, but here are a few patches WIP.\n>\n> And this is the second clean-up\n\nAnd this is the third one.\n\n-- >8 --\nSubject: [PATCH 3/?] diff-index.c: \"git diff\" has no need to read blob from the standard input\n\nOnly \"diff --no-index -\" does.  Bolting the logic into the low-level\nfunction diff_populate_filespec() was a layering violation from day\none.  Move populate_from_stdin() function out of the generic diff.c\nto its only user, diff-index.c.\n\nAlso make sure \"-\" from the command line stays a special token \"read\nfrom the standard input\", even if we later decide to sanitize the\nresult from prefix_filename() function in a few obvious ways,\ne.g. removing unnecessary \"./\" prefix, duplicated slashes \"//\" in\nthe middle, etc.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff-no-index.c | 33 +++++++++++++++++++++++++++++++--\n diff.c          | 21 +--------------------\n diffcore.h      |  1 +\n 3 files changed, 33 insertions(+), 22 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex a5c1e3e..9385d53 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -32,6 +32,12 @@ static int read_directory(const char *path, struct string_list *list)\n \treturn 0;\n }\n \n+/*\n+ * This should be \"(standard input)\", but it will probably expose many\n+ * more breakages.\n+ */\n+static const char file_from_standard_input[] = \"-\";\n+\n static int get_mode(const char *path, int *mode)\n {\n \tstruct stat st;\n@@ -42,7 +48,7 @@ static int get_mode(const char *path, int *mode)\n \telse if (!strcasecmp(path, \"nul\"))\n \t\t*mode = 0;\n #endif\n-\telse if (!strcmp(path, \"-\"))\n+\telse if (path == file_from_standard_input)\n \t\t*mode = create_ce_mode(0666);\n \telse if (lstat(path, &st))\n \t\treturn error(\"Could not access '%s'\", path);\n@@ -51,6 +57,23 @@ static int get_mode(const char *path, int *mode)\n \treturn 0;\n }\n \n+static int populate_from_stdin(struct diff_filespec *s)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tsize_t size = 0;\n+\n+\tif (strbuf_read(&buf, 0, 0) < 0)\n+\t\treturn error(\"error while reading from stdin %s\",\n+\t\t\t\t     strerror(errno));\n+\n+\ts->should_munmap = 0;\n+\ts->data = strbuf_detach(&buf, &size);\n+\ts->size = size;\n+\ts->should_free = 1;\n+\ts->nongit_stdin = 1;\n+\treturn 0;\n+}\n+\n static int queue_diff(struct diff_options *o,\n \t\t      const char *name1, const char *name2)\n {\n@@ -143,8 +166,14 @@ static int queue_diff(struct diff_options *o,\n \t\t\tname2 = \"/dev/null\";\n \t\td1 = alloc_filespec(name1);\n \t\td2 = alloc_filespec(name2);\n+\n \t\tfill_filespec(d1, null_sha1, mode1);\n+\t\tif (name1 == file_from_standard_input)\n+\t\t\tpopulate_from_stdin(d1);\n+\n \t\tfill_filespec(d2, null_sha1, mode2);\n+\t\tif (name2 == file_from_standard_input)\n+\t\t\tpopulate_from_stdin(d2);\n \n \t\tdiff_queue(&diff_queued_diff, d1, d2);\n \t\treturn 0;\n@@ -240,7 +269,7 @@ void diff_no_index(struct rev_info *revs,\n \t\t\t * stdin should be spelled as \"-\"; if you have\n \t\t\t * path that is \"-\", spell it as \"./-\".\n \t\t\t */\n-\t\t\tp = p;\n+\t\t\tp = file_from_standard_input;\n \t\telse if (prefixlen)\n \t\t\tp = xstrdup(prefix_filename(prefix, prefixlen, p));\n \t\tpaths[i] = p;\ndiff --git a/diff.c b/diff.c\nindex 1a594df..0c73c84 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2619,22 +2619,6 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int\n \treturn 0;\n }\n \n-static int populate_from_stdin(struct diff_filespec *s)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tsize_t size = 0;\n-\n-\tif (strbuf_read(&buf, 0, 0) < 0)\n-\t\treturn error(\"error while reading from stdin %s\",\n-\t\t\t\t     strerror(errno));\n-\n-\ts->should_munmap = 0;\n-\ts->data = strbuf_detach(&buf, &size);\n-\ts->size = size;\n-\ts->should_free = 1;\n-\treturn 0;\n-}\n-\n static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n {\n \tint len;\n@@ -2684,9 +2668,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\tstruct stat st;\n \t\tint fd;\n \n-\t\tif (!strcmp(s->path, \"-\"))\n-\t\t\treturn populate_from_stdin(s);\n-\n \t\tif (lstat(s->path, &st) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\terr_empty:\n@@ -3048,7 +3029,7 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n \tif (DIFF_FILE_VALID(one)) {\n \t\tif (!one->sha1_valid) {\n \t\t\tstruct stat st;\n-\t\t\tif (!strcmp(one->path, \"-\")) {\n+\t\t\tif (one->nongit_stdin) {\n \t\t\t\thashcpy(one->sha1, null_sha1);\n \t\t\t\treturn;\n \t\t\t}\ndiff --git a/diffcore.h b/diffcore.h\nindex 8f32b82..ef4fe17 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -42,6 +42,7 @@ struct diff_filespec {\n #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)\n \tunsigned should_free : 1; /* data should be free()'ed */\n \tunsigned should_munmap : 1; /* data should be munmap()'ed */\n+\tunsigned nongit_stdin : 1; /* data is from a nongit source */\n \tunsigned dirty_submodule : 2;  /* For submodules: its work tree is dirty */\n #define DIRTY_SUBMODULE_UNTRACKED 1\n #define DIRTY_SUBMODULE_MODIFIED  2\n-- \n1.7.11.1.184.g3ee8f69\n"},{"id":"194371","messageId":"7vvcic7a6t.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"7vzk7o7am5.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T20:48:42Z","receivedAt":"2012-06-27T20:48:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> Junio C Hamano <gitster@pobox.com> writes:\n>>>\n>>>> Jeff King <peff@peff.net> writes:\n>>>>\n>>>>> but that code path should be fixed properly (with a use_stdin flag in\n>>>>> the filespec).\n>>>>\n>>>> Yes, just as I said; I am finding more and more issues with the\n>>>> no-index hack that I have been fixing a bit by bit since I send the\n>>>> message you responded to.\n>>>\n>>> It is not ready yet, but here are a few patches WIP.\n>>\n>> And this is the second clean-up\n>\n> And this is the third one.\n\nSome of the other breakages that comes from the \"no-index\" codepath\nthat we may want to consider addressing I have found so far are:\n\n - We say on the \"diff --git\" header uglyness like \"a/-\", \"b/-\";\n   likewise in the metainfo;\n\n - We show on the \"index\" header \"0*\" value for these entries, even\n   though we should be able to compute it (after all we do so for\n   files on disk in a non-git directory);\n\n - We still apply attributes and textconv as if we are dealing with\n   a regular file \"-\" at the root level.\n"},{"id":"194376","messageId":"20120627210039.GA2292@sigill.intra.peff.net","threadId":"30913","inReplyTo":"7vvcic7a6t.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-27T21:00:39Z","receivedAt":"2012-06-27T21:00:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 27, 2012 at 01:48:42PM -0700, Junio C Hamano wrote:\n\n> >>> It is not ready yet, but here are a few patches WIP.\n> >>\n> >> And this is the second clean-up\n> >\n> > And this is the third one.\n> \n> Some of the other breakages that comes from the \"no-index\" codepath\n> that we may want to consider addressing I have found so far are:\n\n>From a cursory look, these definitely go in the right direction. I like\nhow the third one was able to rip populate_from_stdin entirely out of\ndiff.c.\n\nI suspect we could get by without even the nongit_stdin flag you added;\nthe only place which uses it is diff_fill_sha1_info, but theoretically\nwe don't even need it there; we could just index_mem the file contents\nwe get via diff_populate_filespec, and the stdin contents will\nalready be there.\n\nRight now we call index_path for worktree files, but I don't really see\nmuch point; we have to read the whole data either way, and\npopulate_filespec should be mmap-ing them for us.\n\n>  - We say on the \"diff --git\" header uglyness like \"a/-\", \"b/-\";\n>    likewise in the metainfo;\n\nI'd consider changing the path to \"/dev/stdin\" for this case. It doesn't\nexist on some platforms, of course, but neither does /dev/null, which we\nuse similarly.\n\n>  - We show on the \"index\" header \"0*\" value for these entries, even\n>    though we should be able to compute it (after all we do so for\n>    files on disk in a non-git directory);\n\nThe index_mem I mentioned above would fix that.\n\n>  - We still apply attributes and textconv as if we are dealing with\n>    a regular file \"-\" at the root level.\n\nI think that's bad. I wonder if it should have \"*\" attributes applied to\nit or not. While I can see it being convenient in some cases, I think it\nmakes the rules confusingly complex.\n\n-Peff\n"},{"id":"194391","messageId":"7v7gus7625.fsf@alter.siamese.dyndns.org","threadId":"30913","inReplyTo":"20120627210039.GA2292@sigill.intra.peff.net","subject":"Re: Bug: problem with file named with dash character","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-27T22:17:54Z","receivedAt":"2012-06-27T22:17:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> From a cursory look, these definitely go in the right direction. I like\n> how the third one was able to rip populate_from_stdin entirely out of\n> diff.c.\n>\n> I suspect we could get by without even the nongit_stdin flag you added;\n> the only place which uses it is diff_fill_sha1_info, but theoretically\n> we don't even need it there; we could just index_mem the file contents\n> we get via diff_populate_filespec, and the stdin contents will\n> already be there.\n\nProbably that is a sane thing to do.\n\nAnd as you hinted, we definitely need to give the \"the copy in the\nfilespec->data is the only one; do not discard until we are really\ndone with it\" semantics to the bit, or introduce a separate bit that\nis not necessarily related to the \"this came from the standard input\"\nbit.  I offhand do not know what data source we would later add\nthat needs such a treatment.  \"git diff http://example.com/{foo,bar}\",\nperhaps?  The \"do not discard\" bit at that point may need to learn\nto swap it out to a temporary file or something when it happens.\n\n> Right now we call index_path for worktree files, but I don't really see\n> much point; we have to read the whole data either way, and\n> populate_filespec should be mmap-ing them for us.\n>\n>>  - We say on the \"diff --git\" header uglyness like \"a/-\", \"b/-\";\n>>    likewise in the metainfo;\n>\n> I'd consider changing the path to \"/dev/stdin\" for this case. It doesn't\n> exist on some platforms, of course, but neither does /dev/null, which we\n> use similarly.\n\nSensible.\n\n>>  - We show on the \"index\" header \"0*\" value for these entries, even\n>>    though we should be able to compute it (after all we do so for\n>>    files on disk in a non-git directory);\n>\n> The index_mem I mentioned above would fix that.\n\nYes.\n\n>>  - We still apply attributes and textconv as if we are dealing with\n>>    a regular file \"-\" at the root level.\n>\n> I think that's bad. I wonder if it should have \"*\" attributes applied to\n> it or not. While I can see it being convenient in some cases, I think it\n> makes the rules confusingly complex.\n\nIt is likely that the crlf conversion on Dos systems wants to be\napplied regardless.\n\nThis is unrelated to the \"standard input confusion\" issue, but there\nare two more things coming either from the way the no-index code is\ndone or from the way it is bolted onto git.\n\nWith the current code, this:\n\n\tmkdir foo bar\n        echo hello >foo/1\n        echo bye >bar/2\n        git diff --no-index foo bar\n\nshows differences between a/foo/1 and b/bar/1, as the no-index code\nrecords foo/1 and bar/1 as the paths in the filespec for them.\n\nBut if you are comparing two directory hierarchies, it is a lot more\nlikely that you would want to see corresponding files in these two\ndirectories.  In other words, the patch is better shown as\ndifferences between a/1 and b/1 (the code makes the core compare\nfoo/1 and bar/2 after all).  This of course requires no-index to\ndifferentiate the logical pathname (i.e. \"this is the path inside\ncollection a/ (or b/)\") and the physical location from which the\ncontents are read.  Such a differentiation would allow us to also do\nrenames and rename classifications much more sanely.  We had to add\nDIFF_PAIR_RENAME() and filepair->renamed_pair only to support this\ncodepath because of this misdesign.  We could have just run strcmp()\nbetween the logical pathname of one/two members of the filepair to\nsee if the pair was renamed if it was done that way.\n\nAnd the way diff-no-index.c::queue_diff() walks two directories to\ngrab paths from them in parallel and incrementally means that the\nfilesystem walking code cannot be reused for something like this:\n\n\tgit diff master:Documentation /var/tmp/docs\n\nto compare a hierarchy represented with a tree object with another\nhierarchy stored in the filesystem outside git's control.  A natural\nway to do so would be to grab a set of paths from /var/tmp/docs and\nhave that set compared against the other set that comes from the tree,\nand the \"grab a set of paths from /var/tmp/docs\" machinery can be\nused twice to implement the current\n\n\tgit diff --no-index /var/tmp/foo /var/tmp/bar\n\nwhich would have been a lot cleaner implementation.\n\nOh well.\n"},{"id":"194394","messageId":"20120627224129.GA27566@sigill.intra.peff.net","threadId":"30913","inReplyTo":"7v7gus7625.fsf@alter.siamese.dyndns.org","subject":"Re: Bug: problem with file named with dash character","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-06-27T22:41:29Z","receivedAt":"2012-06-27T22:41:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 27, 2012 at 03:17:54PM -0700, Junio C Hamano wrote:\n\n> > I think that's bad. I wonder if it should have \"*\" attributes applied to\n> > it or not. While I can see it being convenient in some cases, I think it\n> > makes the rules confusingly complex.\n> \n> It is likely that the crlf conversion on Dos systems wants to be\n> applied regardless.\n\nYeah, that's specifically the case I was thinking of. I would say \"well,\nwe don't need to care about path at all, they can just use\ncore.autocrlf\", but I think autocrlf is discouraged these days in favor\nof using attributes.\n\n> This is unrelated to the \"standard input confusion\" issue, but there\n> are two more things coming either from the way the no-index code is\n> done or from the way it is bolted onto git.\n> \n> With the current code, this:\n> \n> \tmkdir foo bar\n>         echo hello >foo/1\n>         echo bye >bar/2\n>         git diff --no-index foo bar\n> \n> shows differences between a/foo/1 and b/bar/1, as the no-index code\n> records foo/1 and bar/1 as the paths in the filespec for them.\n> \n> But if you are comparing two directory hierarchies, it is a lot more\n> likely that you would want to see corresponding files in these two\n> directories.  In other words, the patch is better shown as\n> differences between a/1 and b/1 (the code makes the core compare\n> foo/1 and bar/2 after all).  This of course requires no-index to\n> differentiate the logical pathname (i.e. \"this is the path inside\n> collection a/ (or b/)\") and the physical location from which the\n> contents are read.  Such a differentiation would allow us to also do\n> renames and rename classifications much more sanely.  We had to add\n> DIFF_PAIR_RENAME() and filepair->renamed_pair only to support this\n> codepath because of this misdesign.  We could have just run strcmp()\n> between the logical pathname of one/two members of the filepair to\n> see if the pair was renamed if it was done that way.\n\nYeah, that makes sense. Really you want to split the idea of\ndiff_filespec into a logical unit of \"the thing I am diffing\" and a\nsource struct of \"here is where I get the data from\". And the latter\ncould be a union of blob information, filesystem path, and stdin flag,\nall contained inside the filespec.\n\n> And the way diff-no-index.c::queue_diff() walks two directories to\n> grab paths from them in parallel and incrementally means that the\n> filesystem walking code cannot be reused for something like this:\n> \n> \tgit diff master:Documentation /var/tmp/docs\n> \n> to compare a hierarchy represented with a tree object with another\n> hierarchy stored in the filesystem outside git's control.  A natural\n> way to do so would be to grab a set of paths from /var/tmp/docs and\n> have that set compared against the other set that comes from the tree,\n> and the \"grab a set of paths from /var/tmp/docs\" machinery can be\n> used twice to implement the current\n> \n> \tgit diff --no-index /var/tmp/foo /var/tmp/bar\n> \n> which would have been a lot cleaner implementation.\n\nAgreed. I have occasionally wanted to do something like the tree\ncomparison you mentioned above, and I think I resorted to actually\nmaking a git tree out of it.\n\nAll of that is nice, and if you feel like working on it, great. But I\nadmit I don't care too much about the --no-index code path. The key\nthing to me is fixing the \"-\" path in the regular code path without\nregressing the no-index stdin code-path too badly. And I think your\npatches already do that, so it might be a good stopping point.\n\n-Peff\n"}]}