| Message ID | xmqqlequsvt4.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Re* [PATCH] tests: replace mingw_test_cmp with a helper in C | expand |
Am 07.09.2022 um 23:45 schrieb Junio C Hamano: > Junio C Hamano <gitster@pobox.com> writes: > >> René Scharfe <l.s.r@web.de> writes: >> >>> "git diff --no-index - -" also doesn't complain, by the way. >> >> True, but in this case hopefully it is worth to call it out, as both >> this code that uses "diff --no-index" and "diff --no-index" itself >> came from the same author ;-) >> >> I think "git diff --no-index - -" should just exit 0 after slurping >> all its input (i.e. allow it to be placed downstream of a pipe >> without blocking the upstream), but it is also fine to exit with 0 >> without reading a single byte from the standard input. Of course >> the latter is easier to implement ;-) > > > ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] diff: "--no-index - -" compares the same standard input > > Conceptually, "git diff --no-index - A" slurps the standard input to > the EOF and compares it with the contents of A, so it is a natural > extension for "git diff --no-index - -" to slurp the standard input > to the end, and compares it with itself, which should always yield > "true". > > There can be two plausible implementations. We could read and > discard the input to the end and exit 0. It would allow us to avoid > sigpipe on the upstream command > > $ dd if=/dev/zero bs=1m count=1 | git diff --no-index - - > > As we can tell the outcome without even consuming a single byte from > the standard input, we can just notice the two input files specified > are "-" and immediately exit. It would allow us to give a correct > answer to > > $ git diff --no-index - - </dev/full > > but can kill the command on the upstream side of a pipe that feeds > us. > > We pick the latter (i.e. not touching the input at all), simply > because it is more efficient. Data producer that can be placed on > the upstream side of a pipe should always be prepared for a consumer > that does not consume all its output (e.g. "head" and pager) anyway. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > diff-no-index.c | 11 +++++++++++ > t/t4053-diff-no-index.sh | 11 +++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 18edbdf4b5..940b66b4c8 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -278,6 +278,17 @@ int diff_no_index(struct rev_info *revs, > p = to_free[i] = prefix_filename(prefix, p); > paths[i] = p; > } > + if (paths[0] == file_from_standard_input && > + paths[1] == file_from_standard_input) { > + /* > + * "git diff --no-index - -". We are asked to compare > + * what came from the standard input with itself; we > + * know they are the same without even reading a > + * single byte. > + */ > + ret = 0; > + goto out; > + } > > fixup_paths(paths, &replacement); > > diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh > index 3feadf0e35..d089d9c2b1 100755 > --- a/t/t4053-diff-no-index.sh > +++ b/t/t4053-diff-no-index.sh > @@ -156,6 +156,16 @@ test_expect_success 'diff --no-index normalizes mode: no changes' ' > test_must_be_empty out > ' > > +test_expect_success 'diff --no-index with standard input' ' > + echo foo >x && > + echo bar >z && > + git diff --no-index - x <x && > + git diff --no-index x - <x && > + test_must_fail git diff --no-index - x <z && > + test_must_fail git diff --no-index x - <z && > + git diff --no-index - - <x > +' This test passes even without the changes to diff-no-index.c above. Because diff.c::diff_fill_oid_info() sets an all-zero object ID for stdin on both sides, I guess, so the comparison is skipped even though the data is read already for the first dash and the data size of the second one is thus zero? > + > test_expect_success POSIXPERM 'diff --no-index normalizes mode: chmod +x' ' > chmod +x y && > cat >expected <<-\EOF && > @@ -180,6 +190,7 @@ test_expect_success POSIXPERM 'diff --no-index normalizes: mode not like git mod > ' > > test_expect_success POSIXPERM,SYMLINKS 'diff --no-index normalizes: mode not like git mode (symlink)' ' > + rm -f z && > ln -s y z && > X_OID=$(git hash-object --stdin <x) && > Z_OID=$(printf y | git hash-object --stdin) &&
René Scharfe <l.s.r@web.de> writes: > This test passes even without the changes to diff-no-index.c above. > Because diff.c::diff_fill_oid_info() sets an all-zero object ID for > stdin on both sides, I guess, so the comparison is skipped even though > the data is read already for the first dash and the data size of the > second one is thus zero? OK, that sounds like a happy accident ;-)
diff --git a/diff-no-index.c b/diff-no-index.c index 18edbdf4b5..940b66b4c8 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -278,6 +278,17 @@ int diff_no_index(struct rev_info *revs, p = to_free[i] = prefix_filename(prefix, p); paths[i] = p; } + if (paths[0] == file_from_standard_input && + paths[1] == file_from_standard_input) { + /* + * "git diff --no-index - -". We are asked to compare + * what came from the standard input with itself; we + * know they are the same without even reading a + * single byte. + */ + ret = 0; + goto out; + } fixup_paths(paths, &replacement); diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh index 3feadf0e35..d089d9c2b1 100755 --- a/t/t4053-diff-no-index.sh +++ b/t/t4053-diff-no-index.sh @@ -156,6 +156,16 @@ test_expect_success 'diff --no-index normalizes mode: no changes' ' test_must_be_empty out ' +test_expect_success 'diff --no-index with standard input' ' + echo foo >x && + echo bar >z && + git diff --no-index - x <x && + git diff --no-index x - <x && + test_must_fail git diff --no-index - x <z && + test_must_fail git diff --no-index x - <z && + git diff --no-index - - <x +' + test_expect_success POSIXPERM 'diff --no-index normalizes mode: chmod +x' ' chmod +x y && cat >expected <<-\EOF && @@ -180,6 +190,7 @@ test_expect_success POSIXPERM 'diff --no-index normalizes: mode not like git mod ' test_expect_success POSIXPERM,SYMLINKS 'diff --no-index normalizes: mode not like git mode (symlink)' ' + rm -f z && ln -s y z && X_OID=$(git hash-object --stdin <x) && Z_OID=$(printf y | git hash-object --stdin) &&