{"thread":{"id":"44467","subject":"[RFC/PATCH 0/2] git diff <(command1) <(command2)","startedAt":"2016-11-11T20:27:13Z","lastAt":"2016-11-16T18:37:20Z","messageCount":18,"participants":["Dennis Kaarsemaker","Junio C Hamano","Jacob Keller","Jeff King","Johannes Schindelin","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"305809","messageId":"20161111201958.2175-1-dennis@kaarsemaker.net","threadId":"44467","inReplyTo":null,"subject":"[RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-11-11T20:19:56Z","receivedAt":"2016-11-11T20:27:13Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Today on #git, a user asked why git diff <(command1) <(command2) gave only some\ngibberish about pipes as output. The answer is fairly simple: git diff gets as\narguments /dev/fd/62 and /dev/fd/63, which are symlinks. So git simply\nreadlink()s them and gets pipe:[123456] as destination of that link which it\nthen outputs.\n\nGiven that 'normal' diff provides arguably better output in this case (a diff\nof the output of the two commands), I wanted to look at what it would take for\ngit to handle this. Surprisingly: not much. 1/2 adds support for\n--follow-symlinks to git diff --no-index (and only the --no-index variant) and\n2/2 adds support for reading from pipes.\n\nNo tests or documentation updates yet, and I'm not sure whether\n--follow-symlinks in other modes than --no-index should be supported, ignored\n(as it is now) or cause an error, but I'm leaning towards the third option.\n\nDennis Kaarsemaker (2):\n diff --no-index: add option to follow symlinks\n diff --no-index: support reading from pipes\n\n diff-no-index.c | 15 ++++++++++++---\n diff.c | 23 +++++++++++++++++++----\n diff.h | 2 +-\n 3 files changed, 32 insertions(+), 8 deletions(-)\n\n-- \n2.10.1-449-gab0f84c\n\n"},{"id":"305810","messageId":"20161111201958.2175-3-dennis@kaarsemaker.net","threadId":"44467","inReplyTo":"20161111201958.2175-1-dennis@kaarsemaker.net","subject":"[PATCH 2/2] diff --no-index: support reading from pipes","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-11-11T20:19:58Z","receivedAt":"2016-11-11T20:27:25Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"diff <(command1) <(command2) provides useful output, let's make it\npossible for git to do the same.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n diff-no-index.c | 8 ++++++++\n diff.c | 13 +++++++++++--\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 15811c2..487dbf5 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -83,6 +83,14 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)\n \t\tname = \"/dev/null\";\n \ts = alloc_filespec(name);\n \tfill_filespec(s, null_sha1, 0, mode);\n+\t/*\n+\t * In --no-index mode, we support reading from pipes. canon_mode, called by\n+\t * fill_filespec, gets confused by this and thinks we now have subprojects.\n+\t * Detect this and tell the rest of the diff machinery to treat pipes as\n+\t * normal files.\n+\t */\n+\tif (S_ISGITLINK(s->mode))\n+\t\ts->mode = S_IFREG | ce_permissions(mode);\n \tif (name == file_from_standard_input)\n \t\tpopulate_from_stdin(s);\n \treturn s;\ndiff --git a/diff.c b/diff.c\nindex 1eaf604..c599efb 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2839,9 +2839,18 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n-\t\ts->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tif (!S_ISREG(st.st_mode)) {\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tstrbuf_read(&sb, fd, 0);\n+\t\t\ts->size = sb.len;\n+\t\t\ts->data = strbuf_detach(&sb, NULL);\n+\t\t\ts->should_free = 1;\n+\t\t}\n+\t\telse {\n+\t\t\ts->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\t\ts->should_munmap = 1;\n+\t\t}\n \t\tclose(fd);\n-\t\ts->should_munmap = 1;\n \n \t\t/*\n \t\t * Convert from working tree format to canonical git format\n-- \n2.10.1-449-gab0f84c\n\n"},{"id":"305812","messageId":"20161111201958.2175-2-dennis@kaarsemaker.net","threadId":"44467","inReplyTo":"20161111201958.2175-1-dennis@kaarsemaker.net","subject":"[PATCH 1/2] diff --no-index: add option to follow symlinks","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-11-11T20:19:57Z","receivedAt":"2016-11-11T20:27:29Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Git's diff machinery does not follow symlinks, which makes sense as git\nitself also does not, but stores the symlink destination.\n\nIn --no-index mode however, it is useful for diff to be able to follow\nsymlinks, matching the behaviour of ordinary diff.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n diff-no-index.c | 7 ++++---\n diff.c | 10 ++++++++--\n diff.h | 2 +-\n 3 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f420786..15811c2 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -40,7 +40,7 @@ static int read_directory_contents(const char *path, struct string_list *list)\n */\n static const char file_from_standard_input[] = \"-\";\n \n-static int get_mode(const char *path, int *mode)\n+static int get_mode(const char *path, int *mode, int follow_symlinks)\n {\n \tstruct stat st;\n \n@@ -52,7 +52,7 @@ static int get_mode(const char *path, int *mode)\n #endif\n \telse if (path == file_from_standard_input)\n \t\t*mode = create_ce_mode(0666);\n-\telse if (lstat(path, &st))\n+\telse if (follow_symlinks ? stat(path, &st) : lstat(path, &st))\n \t\treturn error(\"Could not access '%s'\", path);\n \telse\n \t\t*mode = st.st_mode;\n@@ -93,7 +93,8 @@ static int queue_diff(struct diff_options *o,\n {\n \tint mode1 = 0, mode2 = 0;\n \n-\tif (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n+\tif (get_mode(name1, &mode1, DIFF_OPT_TST(o, FOLLOW_SYMLINKS)) ||\n+\t\tget_mode(name2, &mode2, DIFF_OPT_TST(o, FOLLOW_SYMLINKS)))\n \t\treturn -1;\n \n \tif (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\ndiff --git a/diff.c b/diff.c\nindex be11e4e..1eaf604 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\ts->size = xsize_t(st.st_size);\n \t\tif (!s->size)\n \t\t\tgoto empty;\n-\t\tif (S_ISLNK(st.st_mode)) {\n+\t\tif (S_ISLNK(s->mode)) {\n \t\t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\t\tif (strbuf_readlink(&sb, s->path, s->size))\n@@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t\ts->should_free = 1;\n \t\t\treturn 0;\n \t\t}\n+\t\tif (S_ISLNK(st.st_mode)) {\n+\t\t\tstat(s->path, &st);\n+\t\t\ts->size = xsize_t(st.st_size);\n+\t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n \t\tif ((flags & CHECK_BINARY) &&\n@@ -3884,7 +3888,9 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (!strcmp(arg, \"--no-follow\")) {\n \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n \t\tDIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);\n-\t} else if (!strcmp(arg, \"--color\"))\n+\t} else if (!strcmp(arg, \"--follow-symlinks\"))\n+\t\tDIFF_OPT_SET(options, FOLLOW_SYMLINKS);\n+\telse if (!strcmp(arg, \"--color\"))\n \t\toptions->use_color = 1;\n \telse if (skip_prefix(arg, \"--color=\", &arg)) {\n \t\tint value = git_config_colorbool(NULL, arg);\ndiff --git a/diff.h b/diff.h\nindex 25ae60d..22b0c5a 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)\n #define DIFF_OPT_FOLLOW_RENAMES (1 << 7)\n #define DIFF_OPT_RENAME_EMPTY (1 << 8)\n-/* (1 << 9) unused */\n+#define DIFF_OPT_FOLLOW_SYMLINKS (1 << 9)\n #define DIFF_OPT_HAS_CHANGES (1 << 10)\n #define DIFF_OPT_QUICK (1 << 11)\n #define DIFF_OPT_NO_INDEX (1 << 12)\n-- \n2.10.1-449-gab0f84c\n\n"},{"id":"305822","messageId":"xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"20161111201958.2175-1-dennis@kaarsemaker.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-11T21:27:26Z","receivedAt":"2016-11-11T21:27:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n> No tests or documentation updates yet, and I'm not sure whether\n> --follow-symlinks in other modes than --no-index should be supported, ignored\n> (as it is now) or cause an error, but I'm leaning towards the third option.\n\nMy knee-jerk reaction is:\n\n * The --no-index mode should default to your --follow-symlinks\n behaviour, without any option to turn it on or off.\n\n * If normal \"diff\" that follows symlinks by default has an option\n to disable it, then it is OK to also add --no-follow-symlinks\n that is only valid in the --no-index mode, so that we can mimick\n it better (I do not think this is the case, though).\n\n * Other modes should not follow symbolic links ever, no need for a\n new option.\n\nIn any case, I'd advise you not to reroll this too quickly and\nfrequently until the end of this cycle. During a feature freeze, I\nwon't take new topics in 'pu' as that would add more things I need\nto worry about, and if you reroll in too quick succession, it will\nbecome harder to identify the latest set and queue after the\nrelease.\n\nThanks.\n"},{"id":"305823","messageId":"CA+P7+xrFYj7sepdz1xrEEaqmXQWfRkBz3CsWjYK6OZsZRMLbUA@mail.gmail.com","threadId":"44467","inReplyTo":"xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-11-11T23:14:28Z","receivedAt":"2016-11-11T23:14:55Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano wrote:\n> Dennis Kaarsemaker writes:\n>\n>> No tests or documentation updates yet, and I'm not sure whether\n>> --follow-symlinks in other modes than --no-index should be supported, ignored\n>> (as it is now) or cause an error, but I'm leaning towards the third option.\n>\n> My knee-jerk reaction is:\n>\n> * The --no-index mode should default to your --follow-symlinks\n> behaviour, without any option to turn it on or off.\n>\n\nI agree. We shouldn't have to specify this for no-index.\n\nThanks,\nJake\n"},{"id":"305825","messageId":"CA+P7+xr4xOhHJdL=1GbTrRXq_sdco7VYAq+arqw3Kc2gFvbO7w@mail.gmail.com","threadId":"44467","inReplyTo":"20161111201958.2175-1-dennis@kaarsemaker.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-11-11T23:15:47Z","receivedAt":"2016-11-11T23:16:12Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Nov 11, 2016 at 12:19 PM, Dennis Kaarsemaker\n wrote:\n> Today on #git, a user asked why git diff <(command1) <(command2) gave only some\n> gibberish about pipes as output. The answer is fairly simple: git diff gets as\n> arguments /dev/fd/62 and /dev/fd/63, which are symlinks. So git simply\n> readlink()s them and gets pipe:[123456] as destination of that link which it\n> then outputs.\n>\n> Given that 'normal' diff provides arguably better output in this case (a diff\n> of the output of the two commands), I wanted to look at what it would take for\n> git to handle this. Surprisingly: not much. 1/2 adds support for\n> --follow-symlinks to git diff --no-index (and only the --no-index variant) and\n> 2/2 adds support for reading from pipes.\n>\n\nI think this is really useful. I have an alias so that \"diff\" is just\ngit diff --no-index. It's really useful, because I find the output of\ngit-diff to be much better. Not being able to diff from pipes or\nsymlinks is something I've run into before and it's annoying. So I\ncould really use this.\n\nThanks,\nJake\n\n> No tests or documentation updates yet, and I'm not sure whether\n> --follow-symlinks in other modes than --no-index should be supported, ignored\n> (as it is now) or cause an error, but I'm leaning towards the third option.\n>\n> Dennis Kaarsemaker (2):\n> diff --no-index: add option to follow symlinks\n> diff --no-index: support reading from pipes\n>\n> diff-no-index.c | 15 ++++++++++++---\n> diff.c | 23 +++++++++++++++++++----\n> diff.h | 2 +-\n> 3 files changed, 32 insertions(+), 8 deletions(-)\n>\n> --\n> 2.10.1-449-gab0f84c\n>\n"},{"id":"305851","messageId":"1478931094.12153.3.camel@kaarsemaker.net","threadId":"44467","inReplyTo":"xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-11-12T06:11:34Z","receivedAt":"2016-11-12T06:11:44Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Fri, 2016-11-11 at 13:27 -0800, Junio C Hamano wrote:\n> Dennis Kaarsemaker writes:\n> \n> > No tests or documentation updates yet, and I'm not sure whether\n> > --follow-symlinks in other modes than --no-index should be supported, ignored\n> > (as it is now) or cause an error, but I'm leaning towards the third option.\n> \n> \n> My knee-jerk reaction is:\n> \n> * The --no-index mode should default to your --follow-symlinks\n> behaviour, without any option to turn it on or off.\n\nok.\n\n> * If normal \"diff\" that follows symlinks by default has an option\n> to disable it, then it is OK to also add --no-follow-symlinks\n> that is only valid in the --no-index mode, so that we can mimick\n> it better (I do not think this is the case, though).\n\nIt does not, so no new option.\n\n> * Other modes should not follow symbolic links ever, no need for a\n> new option.\n\nMakes sense.\n\n> In any case, I'd advise you not to reroll this too quickly and\n> frequently until the end of this cycle. During a feature freeze, I\n> won't take new topics in 'pu' as that would add more things I need\n> to worry about, and if you reroll in too quick succession, it will\n> become harder to identify the latest set and queue after the\n> release.\n\nI'm in no hurry, so I'll sit on this until v2.11 is done.\n\nD.\n"},{"id":"305852","messageId":"20161112070619.hto3lehbyh6oamql@sigill.intra.peff.net","threadId":"44467","inReplyTo":"1478931094.12153.3.camel@kaarsemaker.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-11-12T07:06:19Z","receivedAt":"2016-11-12T07:06:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 12, 2016 at 07:11:34AM +0100, Dennis Kaarsemaker wrote:\n\n> > * If normal \"diff\" that follows symlinks by default has an option\n> > to disable it, then it is OK to also add --no-follow-symlinks\n> > that is only valid in the --no-index mode, so that we can mimick\n> > it better (I do not think this is the case, though).\n> \n> It does not, so no new option.\n\nThere is \"--no-dereference\", but it is not that helpful:\n\n $ echo one >1\n $ echo two >2\n $ ln -s 1 sl1\n $ ln -s 2 sl2\n $ diff --no-dereference sl1 sl2\n Symbolic links sl1 and sl2 differ\n $ diff --no-dereference sl1 2\n File sl1 is a symbolic link while file 2 is a regular file\n\nSo it does do the thing we are talking about here, but diff's handling\nof non-file content is not nearly as useful as git's.\n\nThat said, I have no problem with proceeding without it and waiting\nuntil somebody actually shows up who really _wants_ --no-dereference. I\nwon't be surprised if that never happens.\n\n-Peff\n"},{"id":"305856","messageId":"alpine.DEB.2.20.1611121106110.3746@virtualbox","threadId":"44467","inReplyTo":"CA+P7+xrFYj7sepdz1xrEEaqmXQWfRkBz3CsWjYK6OZsZRMLbUA@mail.gmail.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-12T10:08:58Z","receivedAt":"2016-11-12T10:09:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 11 Nov 2016, Jacob Keller wrote:\n\n> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano wrote:\n> > Dennis Kaarsemaker writes:\n> >\n> >> No tests or documentation updates yet, and I'm not sure whether\n> >> --follow-symlinks in other modes than --no-index should be supported, ignored\n> >> (as it is now) or cause an error, but I'm leaning towards the third option.\n> >\n> > My knee-jerk reaction is:\n> >\n> > * The --no-index mode should default to your --follow-symlinks\n> > behaviour, without any option to turn it on or off.\n> >\n> \n> I agree. We shouldn't have to specify this for no-index.\n\nUmmm. *My* idea of --no-index was for it to behave as similar to the\n--index version as possible. For example when comparing directories\ncontaining symlinks. You seem intent on breaking this scenario.\n\nCiao,\nJohannes\n"},{"id":"305881","messageId":"xmqqwpg6ycpn.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"alpine.DEB.2.20.1611121106110.3746@virtualbox","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T03:14:44Z","receivedAt":"2016-11-14T03:14:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Fri, 11 Nov 2016, Jacob Keller wrote:\n>\n>> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano wrote:\n>> > Dennis Kaarsemaker writes:\n>> >\n>> >> No tests or documentation updates yet, and I'm not sure whether\n>> >> --follow-symlinks in other modes than --no-index should be supported, ignored\n>> >> (as it is now) or cause an error, but I'm leaning towards the third option.\n>> >\n>> > My knee-jerk reaction is:\n>> >\n>> > * The --no-index mode should default to your --follow-symlinks\n>> > behaviour, without any option to turn it on or off.\n>> >\n>> \n>> I agree. We shouldn't have to specify this for no-index.\n>\n> Ummm. *My* idea of --no-index was for it to behave as similar to the\n> --index version as possible. For example when comparing directories\n> containing symlinks. You seem intent on breaking this scenario.\n\nPerhaps a viable compromise between the two is to only always\ndereference at the top-level (i.e. the trees to be compared) under\n\"--no-index\" mode and not changing anything else?\n\nThe original use case by Dennis is not even about doing a recursive\ntwo-directories-in-a-filesystem comparison and encountering a\nsymbolic link (it was to compare two BLOBs, which happen to be\noutput from two commands).\n"},{"id":"305886","messageId":"0c39be16-76f8-0800-41a2-b7b1dccdd652@drmicha.warpmail.net","threadId":"44467","inReplyTo":"alpine.DEB.2.20.1611121106110.3746@virtualbox","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-11-14T15:31:06Z","receivedAt":"2016-11-14T15:31:16Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Johannes Schindelin venit, vidit, dixit 12.11.2016 11:08:\n> Hi,\n> \n> On Fri, 11 Nov 2016, Jacob Keller wrote:\n> \n>> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano wrote:\n>>> Dennis Kaarsemaker writes:\n>>>\n>>>> No tests or documentation updates yet, and I'm not sure whether\n>>>> --follow-symlinks in other modes than --no-index should be supported, ignored\n>>>> (as it is now) or cause an error, but I'm leaning towards the third option.\n>>>\n>>> My knee-jerk reaction is:\n>>>\n>>> * The --no-index mode should default to your --follow-symlinks\n>>> behaviour, without any option to turn it on or off.\n>>>\n>>\n>> I agree. We shouldn't have to specify this for no-index.\n> \n> Ummm. *My* idea of --no-index was for it to behave as similar to the\n> --index version as possible. For example when comparing directories\n> containing symlinks. You seem intent on breaking this scenario.\n\n*My* idea of --no-index was for it to behave as similar to the\n--index-version as possible, regarding formatting etc., and to be a good\nsubstitute for ordinary diff. The proposed patch achieves exactly that -\nwhy should a *file* argument (which is not a pathspec in --no-index\nmode) not be treated in the same way in which every other command treats\na file argument? The patch un-breaks the most natural expectation.\n\nMichael\n\n"},{"id":"305891","messageId":"alpine.DEB.2.20.1611141739560.3746@virtualbox","threadId":"44467","inReplyTo":"0c39be16-76f8-0800-41a2-b7b1dccdd652@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-14T16:40:46Z","receivedAt":"2016-11-14T16:41:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Michael,\n\nOn Mon, 14 Nov 2016, Michael J Gruber wrote:\n\n> why should a *file* argument (which is not a pathspec in --no-index\n> mode) not be treated in the same way in which every other command treats\n> a file argument?\n\nWe are talking about `<(command)` here, which is most distinctly not a\nfile argument at all.\n\nCiao,\nJohannes\n"},{"id":"305896","messageId":"xmqqoa1ix7nq.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"0c39be16-76f8-0800-41a2-b7b1dccdd652@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T18:01:29Z","receivedAt":"2016-11-14T18:01:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> *My* idea of --no-index was for it to behave as similar to the\n> --index-version as possible, regarding formatting etc., and to be a good\n> substitute for ordinary diff. The proposed patch achieves exactly that -\n\nDoes it? It looks to me that it does a lot more.\n\n> why should a *file* argument (which is not a pathspec in --no-index\n> mode) not be treated in the same way in which every other command treats\n> a file argument? The patch un-breaks the most natural expectation.\n\nI think a filename given as a command line argument, e.g. <(cmd), is\nnow treated more sensibly with [2/2]. Something that is not a\ndirectory to be descended into and is not a regular file needs to be\nmade into a form that we can use as a blob, and reading it into an\nin-core buffer is a workable way to do so. \n\nHowever, when taken together with [1/2], doesn't the proposed patch\n\"achieves\" a lot more than \"exactly that\", namely, by not treating\nsymbolic links discovered during traversals of directories given\nfrom the command line as such and dereferencing?\n"},{"id":"305917","messageId":"a3db4c55-550c-f2e8-83b8-46c2be86f7da@drmicha.warpmail.net","threadId":"44467","inReplyTo":"xmqqoa1ix7nq.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-11-14T20:23:07Z","receivedAt":"2016-11-14T20:23:17Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 14.11.2016 19:01:\n> Michael J Gruber writes:\n> \n>> *My* idea of --no-index was for it to behave as similar to the\n>> --index-version as possible, regarding formatting etc., and to be a good\n>> substitute for ordinary diff. The proposed patch achieves exactly that -\n> \n> Does it? It looks to me that it does a lot more.\n\nYes, I didn't mean to say that it achieves only that - it achieves that\none goal exactly, and more.\n\n>> why should a *file* argument (which is not a pathspec in --no-index\n>> mode) not be treated in the same way in which every other command treats\n>> a file argument? The patch un-breaks the most natural expectation.\n> \n> I think a filename given as a command line argument, e.g. <(cmd), is\n> now treated more sensibly with [2/2]. Something that is not a\n> directory to be descended into and is not a regular file needs to be\n> made into a form that we can use as a blob, and reading it into an\n> in-core buffer is a workable way to do so. \n\nYes.\n\n> However, when taken together with [1/2], doesn't the proposed patch\n> \"achieves\" a lot more than \"exactly that\", namely, by not treating\n> symbolic links discovered during traversals of directories given\n> from the command line as such and dereferencing?\n\nIt's not clear to me what you are saying here - 1/2 makes git diff\nfollow symbolic links, yes, just like ordinary diff. If I 'diff' two\ndirs that contain symbolic links with the same name pointing to\ndifferent files I get a diff between the contents, not between the\nfilenames.\n\nI like the proposed change a lot, maybe that didn't come across clearly.\nI think it makes things more \"predictable\" in the sense that it meets\ntypical expectations.\n\nMichael\n\n"},{"id":"305919","messageId":"xmqqtwb9wywp.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"a3db4c55-550c-f2e8-83b8-46c2be86f7da@drmicha.warpmail.net","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-14T21:10:30Z","receivedAt":"2016-11-14T21:10:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Junio C Hamano venit, vidit, dixit 14.11.2016 19:01:\n>> Michael J Gruber writes:\n>> \n>>> *My* idea of --no-index was for it to behave as similar to the\n>>> --index-version as possible, regarding formatting etc., and to be a good\n>>> substitute for ordinary diff. The proposed patch achieves exactly that -\n>> ...\n> It's not clear to me what you are saying here - 1/2 makes git diff\n> follow symbolic links, yes, just like ordinary diff.\n\nYes, which can be seen as deviating from your earlier \"as similar to\nthe --index version as possible\" goal, which I think was where Dscho's\ncomplaint comes from.\n\nI _think_ the no-index mode was primarily for those who want to use\nour diff as a replacement for GNU and other diffs, and from that\npoint of view, I'd favour not doing the \"comparing symbolic link?\nWe'll show the difference between the link contents, not target\"\nunder no-index mode myself. That is a lot closer to the diff other\npeople implemented, not ours. Hence the knee-jerk reaction I gave\nin\n\nhttp://public-inbox.org/git/xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com\n\n\n"},{"id":"306039","messageId":"alpine.DEB.2.20.1611161039590.3746@virtualbox","threadId":"44467","inReplyTo":"xmqqtwb9wywp.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-11-16T09:50:30Z","receivedAt":"2016-11-16T09:50:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 14 Nov 2016, Junio C Hamano wrote:\n\n> I _think_ the no-index mode was primarily for those who want to use\n> our diff as a replacement for GNU and other diffs, and from that\n> point of view, I'd favour not doing the \"comparing symbolic link?\n> We'll show the difference between the link contents, not target\"\n> under no-index mode myself.\n\nIf I read this correctly, then we are in agreement that the default for\n--no-index should be as it is right now, i.e. comparing symlink targets as\nopposed to --follow-links.\n\n> That is a lot closer to the diff other people implemented, not ours.\n> Hence the knee-jerk reaction I gave in\n> \n> http://public-inbox.org/git/xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com\n\nLet me quote the knee-jerk reaction:\n\n> My knee-jerk reaction is:\n>\n> * The --no-index mode should default to your --follow-symlinks\n> behaviour, without any option to turn it on or off.\n\nBut this is the exact opposite of what I find reasonable.\n\nCiao,\nJohannes\n"},{"id":"306068","messageId":"xmqqh977qnvk.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"alpine.DEB.2.20.1611161039590.3746@virtualbox","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T18:29:51Z","receivedAt":"2016-11-16T18:30:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Mon, 14 Nov 2016, Junio C Hamano wrote:\n>\n>> I _think_ the no-index mode was primarily for those who want to use\n>> our diff as a replacement for GNU and other diffs, and from that\n>> point of view, I'd favour not doing the \"comparing symbolic link?\n>> We'll show the difference between the link contents, not target\"\n>> under no-index mode myself.\n>\n> If I read this correctly,...\n\nNow I re-read it and I can see it can be read either way.\n\nBy \"link contents\" in \"comparing symbolic link? We'll show the\ndifference between the link contents, not target\", I meant the\nresult you get from readlink(2), which will result in\n\n diff --git a/RelNotes b/RelNotes\n index c02235fe8c..b54330f7cd 120000\n --- a/RelNotes\n +++ b/RelNotes\n @@ -1 +1 @@\n -Documentation/RelNotes/2.10.2.txt\n \\ No newline at end of file\n +Documentation/RelNotes/2.11.0.txt\n \\ No newline at end of file\n\nnot the comparison between the files that are link targets,\ni.e. hypothetical\n\n diff --git a/RelNotes b/RelNotes\n index c4d4397023..7a1fce7720 100644\n --- a/Documentation/RelNotes/2.10.2.txt\n +++ b/Documentation/RelNotes/2.11.0.txt\n @@ -1,41 +1,402 @@\n -Git v2.10.2 Release Notes\n -=========================\n +Git 2.11 Release Notes\n ...\n\nAnd I'd favour *NOT* doing that if we are using our diff as a\nreplacement for GNU and other diffs in \"no-index\" mode. Which leads\nto ...\n\n>> That is a lot closer to the diff other people implemented, not ours.\n>> Hence the knee-jerk reaction I gave in\n>> \n>> http://public-inbox.org/git/xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com\n\n... this conclusion, which is consistent with ...\n\n>\n> Let me quote the knee-jerk reaction:\n>\n>> My knee-jerk reaction is:\n>>\n>> * The --no-index mode should default to your --follow-symlinks\n>> behaviour, without any option to turn it on or off.\n\n... this one.\n\nBut notice \"I _think_\" in the first sentence you quoted. That is a\nbasic assumption that leads to the conclusion, and that assumption\nis not a fact. Maybe users do *not* want the \"no-index\" mode as a\nreplacement for GNU and other diffs, in which case comparing the\nresult of readlink(2) even in no-index mode might have merit. I\njust didn't think it was the case.\n\n"},{"id":"306070","messageId":"xmqqd1hvqnjc.fsf@gitster.mtv.corp.google.com","threadId":"44467","inReplyTo":"xmqqh977qnvk.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-16T18:37:11Z","receivedAt":"2016-11-16T18:37:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> On Mon, 14 Nov 2016, Junio C Hamano wrote:\n>>\n>>> I _think_ the no-index mode was primarily for those who want to use\n>>> our diff as a replacement for GNU and other diffs, and from that\n>>> point of view, I'd favour not doing the \"comparing symbolic link?\n>>> We'll show the difference between the link contents, not target\"\n>>> under no-index mode myself.\n>>\n>> If I read this correctly,...\n>\n> Now I re-read it and I can see it can be read either way.\n>\n> By \"link contents\" in \"comparing symbolic link? We'll show the\n> difference between the link contents, not target\", I meant the\n> result you get from readlink(2), which will result in\n>\n> diff --git a/RelNotes b/RelNotes\n> index c02235fe8c..b54330f7cd 120000\n> --- a/RelNotes\n> +++ b/RelNotes\n> @@ -1 +1 @@\n> -Documentation/RelNotes/2.10.2.txt\n> \\ No newline at end of file\n> +Documentation/RelNotes/2.11.0.txt\n> \\ No newline at end of file\n>\n> not the comparison between the files that are link targets,\n> i.e. hypothetical\n>\n> diff --git a/RelNotes b/RelNotes\n> index c4d4397023..7a1fce7720 100644\n> --- a/Documentation/RelNotes/2.10.2.txt\n> +++ b/Documentation/RelNotes/2.11.0.txt\n> @@ -1,41 +1,402 @@\n> -Git v2.10.2 Release Notes\n> -=========================\n> +Git 2.11 Release Notes\n> ...\n>\n> And I'd favour *NOT* doing that if we are using our diff as a\n\nAgain, this can be read both ways. By \"that\" on the above line I\nmeant \"the former\".\n\n> replacement for GNU and other diffs in \"no-index\" mode. Which leads\n> to ...\n>\n>>> That is a lot closer to the diff other people implemented, not ours.\n>>> Hence the knee-jerk reaction I gave in\n>>> \n>>> http://public-inbox.org/git/xmqqinrt1zcx.fsf@gitster.mtv.corp.google.com\n>\n> ... this conclusion, which is consistent with ...\n>\n>>\n>> Let me quote the knee-jerk reaction:\n>>\n>>> My knee-jerk reaction is:\n>>>\n>>> * The --no-index mode should default to your --follow-symlinks\n>>> behaviour, without any option to turn it on or off.\n>\n> ... this one.\n>\n> But notice \"I _think_\" in the first sentence you quoted. That is a\n> basic assumption that leads to the conclusion, and that assumption\n> is not a fact. Maybe users do *not* want the \"no-index\" mode as a\n> replacement for GNU and other diffs, in which case comparing the\n> result of readlink(2) even in no-index mode might have merit. I\n> just didn't think it was the case.\n\nAnd \"I just didn't think it was the case\", when fully spelt out, is\n\"I just didn't think that the assumption was incorrect.\"\n"}]}