Re: Re* [PATCH] tests: replace mingw_test_cmp with a helper in C
- From
René Scharfe <l.s.r@web.de>
- Date
- Sep 7, 2022, 22:39 UTC
- Message-ID
- <22402f5d-c463-acee-f57d-e507f23822ca@web.de>
- In-Reply-To
- <xmqqlequsvt4.fsf_-_@gitster.g>
Am 07.09.2022 um 23:45 schrieb Junio C Hamano:
Show 92 quoted lines
> 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?
Show 12 quoted lines
> + > 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) &&