# [PATCH 4/8] diff: support reading a file from stdin via "-" 6 messages from 2007-02-25 to 2007-02-26. Participants: Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/6960 ## Johannes Schindelin, 2007-02-25 22:36 Subject: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0702252335410.22628%40wbgn013.biozentrum.uni-wuerzburg.de ``` This allows you to say echo Hello World | git diff x - to compare the contents of file "x" with the line "Hello World". This automatically switches to --no-index mode. Signed-off-by: Johannes Schindelin --- Since the revision machinery checks for the presence of files, diff_populate_filespec() will only change behaviour when there is a file "-"... I have yet to think of an elegant fix for that. diff-lib.c | 16 +++++++++++----- diff.c | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 6678e22..089c94c 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -37,14 +37,20 @@ static int queue_diff(struct diff_options *o, int mode1 = 0, mode2 = 0; if (name1) { - if (stat(name1, &st)) + if (!strcmp(name1, "-")) + mode1 = 0644; + else if (stat(name1, &st)) return error("Could not access '%s'", name1); - mode1 = st.st_mode; + else + mode1 = st.st_mode; } if (name2) { - if (stat(name2, &st)) + if (!strcmp(name2, "-")) + mode2 = 0644; + else if (stat(name2, &st)) return error("Could not access '%s'", name2); - mode2 = st.st_mode; + else + mode2 = st.st_mode; } if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) @@ -224,7 +230,7 @@ int setup_diff_no_index(struct rev_info *revs, { int i; for (i = 1; i < argc; i++) - if (argv[i][0] != '-') + if (argv[i][0] != '-' || argv[i][1] == '\0') break; else if (!strcmp(argv[i], "--")) { i++; diff --git a/diff.c b/diff.c index 5651152..31129d1 100644 --- a/diff.c +++ b/diff.c @@ -1389,6 +1389,22 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only) char *buf; unsigned long size; + if (!strcmp(s->path, "-")) { +#define INCREMENT 1024 + int i = INCREMENT; + size = 0; + buf = NULL; + while (i == INCREMENT) { + buf = xrealloc(buf, size + INCREMENT); + i = xread(0, buf + size, INCREMENT); + size += i; + } + s->should_munmap = 0; + s->data = buf; + s->size = size; + s->should_free = 1; + return 0; + } if (lstat(s->path, &st) < 0) { if (errno == ENOENT) { err_empty: @@ -1688,6 +1704,10 @@ static void diff_fill_sha1_info(struct diff_filespec *one) if (DIFF_FILE_VALID(one)) { if (!one->sha1_valid) { struct stat st; + if (!strcmp(one->path, "-")) { + hashcpy(one->sha1, null_sha1); + return; + } if (lstat(one->path, &st) < 0) die("stat %s", one->path); if (index_path(one->sha1, one->path, &st, 0)) -- 1.5.0.1.788.g8ca52 ``` ## Junio C Hamano, 2007-02-25 22:57 Subject: Re: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: <7vvehprgl7.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vvehprgl7.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > This allows you to say > > echo Hello World | git diff x - > > to compare the contents of file "x" with the line "Hello World". > This automatically switches to --no-index mode. > > Signed-off-by: Johannes Schindelin > --- > > Since the revision machinery checks for the presence of files, > diff_populate_filespec() will only change behaviour when there > is a file "-"... I have yet to think of an elegant fix for that. Another thing is that at some point diff_populate_filespec() needs to have a way to discard what was cached if memory pressure gets tight, and we would want to keep this data read from the standard input. One solution would be to add a "const char *stdin_data" to diffopts and read the data from stdin when you parse the options, and have populate_filespec point at that with s->data (setting should_free and should_munmap both to 0). ``` ## Johannes Schindelin, 2007-02-25 23:11 Subject: Re: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0702260009530.22628%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vvehprgl7.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Sun, 25 Feb 2007, Junio C Hamano wrote: > Johannes Schindelin writes: > > > This allows you to say > > > > echo Hello World | git diff x - > > > > to compare the contents of file "x" with the line "Hello World". > > This automatically switches to --no-index mode. > > > > Signed-off-by: Johannes Schindelin > > --- > > > > Since the revision machinery checks for the presence of files, > > diff_populate_filespec() will only change behaviour when there > > is a file "-"... I have yet to think of an elegant fix for that. > > Another thing is that at some point diff_populate_filespec() > needs to have a way to discard what was cached if memory > pressure gets tight, and we would want to keep this data read > from the standard input. I see this purely for purposes of --no-index diff. And in that case, we only compare one file pair. Either one of them is from stdin, or it is not. Therefore, if memory gets tight, we cannot compare that file pair anyway, and have to error out. Maybe I missed something obvious? Ciao, Dscho ``` ## Junio C Hamano, 2007-02-25 23:41 Subject: Re: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: <7virdprejh.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7virdprejh.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > I see this purely for purposes of --no-index diff. And in that case, we > only compare one file pair. Either one of them is from stdin, or it is > not. Therefore, if memory gets tight, we cannot compare that file pair > anyway, and have to error out. > > Maybe I missed something obvious? I think the responsibility to mark if a path whose name is "-" is a filesystem object or stdin lies within the option parser. If the user says "diff - a" or diff a -" then the user clearly means the stdin, while if the user says "diff ./- a" it is a path (and the later stages like populate_filespec() would see canonicalized "-" as the filename). The point is that the disambiguation must be done by somebody, and I think the option parser, who calls get_pathspec(), should be the one that does it. So we at least need a bit in diff_options that lets the option parser to tell the later stages "if you see - in pathspec, the user means stdin". And my suggestion was that if you are going to do that in the option parser anyway, you could read stdin in the option parser and hang that data in diff_options, and the presence of that stdin data (pointer being non NULL) can be used as that signal. I think the callers of populate_filespec() may need to pass around diff_options as a parameter for the above to work, but hopefully that should not be a rocket surgery. ``` ## Junio C Hamano, 2007-02-26 00:03 Subject: Re: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: <7vejodrdi4.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vejodrdi4.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7virdprejh.fsf@assigned-by-dhcp.cox.net> ``` Another hack just occurred to me... get_pathspec() could translate '-' to /dev/stdin while leaving './-' to '-'. It could be rather too ugly to the taste of some people. Myself, I dunno... ``` ## Johannes Schindelin, 2007-02-26 00:44 Subject: Re: [PATCH 4/8] diff: support reading a file from stdin via "-" Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0702260139290.22628%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7virdprejh.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Sun, 25 Feb 2007, Junio C Hamano wrote: > I think the callers of populate_filespec() may need to pass around > diff_options as a parameter for the above to work, but hopefully that > should not be a rocket surgery. That's a good idea! Actually, that pointer can be the pointer in the diff_filespec itself! This is my current version of [4/8] (Gosh, I really _love_ rebase -i ;-): -- snipsnap -- [PATCH] diff: support reading a file from stdin via "-" This allows you to say echo Hello World | git diff x - to compare the contents of file "x" with the line "Hello World". This automatically switches to --no-index mode. Signed-off-by: Johannes Schindelin --- This gives revs->max_count yet another meaning, oh well... The diff is a bit borked, because get_mode() and the first part of the old queue_diff() are apparently a little similar... diff-lib.c | 92 ++++++++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 65 insertions(+), 27 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 6678e22..9302d2b 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -30,22 +30,61 @@ static int read_directory(const char *path, struct path_list *list) return 0; } -static int queue_diff(struct diff_options *o, - const char *name1, const char *name2) +static int get_mode(const char *path, int *mode, int seen_dashdash) { struct stat st; - int mode1 = 0, mode2 = 0; - if (name1) { - if (stat(name1, &st)) - return error("Could not access '%s'", name1); - mode1 = st.st_mode; + if (!path) + *mode = 0; + else if (!seen_dashdash && !strcmp(path, "-")) + *mode = 020600; + else if (stat(path, &st)) + return error("Could not access '%s'", path); + else + *mode = st.st_mode; + return 0; +} + +static void read_stdin(void **buffer, unsigned long *size) +{ + char *buf = NULL; +#define INCREMENT 1024 + int i = INCREMENT; + + *size = 0; + buf = NULL; + while (i == INCREMENT) { + buf = xrealloc(buf, *size + INCREMENT); + i = xread(0, buf + *size, INCREMENT); + *size += i; } - if (name2) { - if (stat(name2, &st)) - return error("Could not access '%s'", name2); - mode2 = st.st_mode; + *buffer = buf; +} + +static struct diff_filespec *make_diff_filespec(const char *path, int mode) +{ + struct diff_filespec *result; + if (!path) + path = "/dev/null"; + result = alloc_filespec(path); + fill_filespec(result, null_sha1, mode); + if (mode == 020600) { + read_stdin(&result->data, &result->size); + result->mode = 0100644; + result->should_munmap = 0; + result->should_free = 1; } + return result; +} + +static int queue_diff(struct diff_options *o, + const char *name1, const char *name2, int seen_dashdash) +{ + int mode1 = 0, mode2 = 0; + + if (get_mode(name1, &mode1, seen_dashdash) || + get_mode(name2, &mode2, seen_dashdash)) + return -1; if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) return error("file/directory conflict: %s, %s", name1, name2); @@ -106,7 +145,7 @@ static int queue_diff(struct diff_options *o, PATH_MAX - len2); } - ret = queue_diff(o, n1, n2); + ret = queue_diff(o, n1, n2, 1); } path_list_clear(&p1, 0); path_list_clear(&p2, 0); @@ -122,14 +161,8 @@ static int queue_diff(struct diff_options *o, tmp_c = name1; name1 = name2; name2 = tmp_c; } - if (!name1) - name1 = "/dev/null"; - if (!name2) - name2 = "/dev/null"; - d1 = alloc_filespec(name1); - d2 = alloc_filespec(name2); - fill_filespec(d1, null_sha1, mode1); - fill_filespec(d2, null_sha1, mode2); + d1 = make_diff_filespec(name1, mode1); + d2 = make_diff_filespec(name2, mode2); diff_queue(&diff_queued_diff, d1, d2); return 0; @@ -222,11 +255,12 @@ static int is_outside_repo(const char *path, int nongit, const char *prefix) int setup_diff_no_index(struct rev_info *revs, int argc, const char ** argv, int nongit, const char *prefix) { - int i; + int i, seen_dashdash = 0; for (i = 1; i < argc; i++) - if (argv[i][0] != '-') + if (argv[i][0] != '-' || argv[i][1] == '\0') break; else if (!strcmp(argv[i], "--")) { + seen_dashdash = 1; i++; break; } else if (i < argc - 3 && !strcmp(argv[i], "--no-index")) { @@ -238,7 +272,7 @@ int setup_diff_no_index(struct rev_info *revs, return -1; diff_setup(&revs->diffopt); - for (i = 1; i < argc - 2; ) + for (i = 1; i < argc - 2 - seen_dashdash; ) if (!strcmp(argv[i], "--no-index")) i++; else { @@ -250,22 +284,26 @@ int setup_diff_no_index(struct rev_info *revs, } revs->diffopt.paths = argv + argc - 2; revs->diffopt.nr_paths = 2; - revs->max_count = -2; + revs->max_count = -2 - seen_dashdash; return 0; } int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv) { - int silent_on_removed; + int silent_on_removed, seen_dashdash = 0; - if (handle_diff_files_args(revs, argc, argv, &silent_on_removed)) + if (revs->max_count == -3) { + seen_dashdash = 1; + revs->max_count = -2; + } else if (handle_diff_files_args(revs, argc, argv, + &silent_on_removed)) return -1; if (revs->max_count == -2) { if (revs->diffopt.nr_paths != 2) return error("need two files/directories with --no-index"); if (queue_diff(&revs->diffopt, revs->diffopt.paths[0], - revs->diffopt.paths[1])) + revs->diffopt.paths[1], seen_dashdash)) return -1; diffcore_std(&revs->diffopt); diff_flush(&revs->diffopt); -- 1.5.0.1.784.g105d9-dirty ```