From: "René Scharfe" <l.s.r@web.de>
To: German Lashevich <german.lashevich@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 2/2] diff: fix --exit-code with external diff
Date: Sun, 5 May 2024 12:20:03 +0200 [thread overview]
Message-ID: <e2e4a4e9-55db-403c-902d-fd8af3aea05c@web.de> (raw)
In-Reply-To: <82561c70-ec33-41bf-b036-52310ffc1926@web.de>
You can ask the diff machinery to let the exit code indicate whether
there are changes, e.g. with --exit-code. It as two ways to calculate
that bit: The quick one assumes blobs with different hashes have
different content, and the more elaborate way actually compares the
contents, possibly applying transformations like ignoring whitespace.
Always use the slower path by setting the flag diff_from_contents,
because any of the files could have an external diff driver set via an
attribute, which might consider binary differences irrelevant, like e.g.
diff -b.
And don't stop walking that path in diff_flush() just because
output_format is unset. Instead run diff_flush_patch() with output
redirected to /dev/null to get the exit status, like we already do for
DIFF_FORMAT_NO_OUTPUT. That's consistent with the comments in diff.h:
/* Same as output_format = 0 but we know that -s flag was given
* and we should not give default value to output_format.
*/
#define DIFF_FORMAT_NO_OUTPUT 0x0800
An unset output_format is given e.g. by repo_index_has_changes(). We
could set it right there, but there may be other places, so simply let
diff_flush() handle it for all of them consistently.
Finally, capture the output of the external diff and only register a
change by setting found_changes if the program wrote anything.
Reported-by: German Lashevich <german.lashevich@gmail.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
diff.c | 33 ++++++++++++++++++++++++++++++---
t/t4020-diff-external.sh | 8 ++++++++
2 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/diff.c b/diff.c
index ded9ac70df..cd3c8a3978 100644
--- a/diff.c
+++ b/diff.c
@@ -40,6 +40,7 @@
#include "setup.h"
#include "strmap.h"
#include "ws.h"
+#include "write-or-die.h"
#ifdef NO_FAST_WORKING_DIRECTORY
#define FAST_WORKING_DIRECTORY 0
@@ -4404,8 +4405,33 @@ static void run_external_diff(const char *pgm,
diff_free_filespec_data(one);
diff_free_filespec_data(two);
cmd.use_shell = 1;
- if (run_command(&cmd))
- die(_("external diff died, stopping at %s"), name);
+ if (o->flags.diff_from_contents) {
+ int got_output = 0;
+ cmd.out = -1;
+ if (start_command(&cmd))
+ die(_("external diff died, stopping at %s"), name);
+ for (;;) {
+ char buffer[8192];
+ ssize_t len = xread(cmd.out, buffer, sizeof(buffer));
+ if (!len)
+ break;
+ if (len < 0)
+ die(_("unable to read from external diff,"
+ " stopping at %s"), name);
+ got_output = 1;
+ if (write_in_full(1, buffer, len) < 0)
+ die(_("unable to write output of external diff,"
+ " stopping at %s"), name);
+ }
+ close(cmd.out);
+ if (finish_command(&cmd))
+ die(_("external diff died, stopping at %s"), name);
+ if (got_output)
+ o->found_changes = 1;
+ } else {
+ if (run_command(&cmd))
+ die(_("external diff died, stopping at %s"), name);
+ }
remove_tempfile();
}
@@ -4852,6 +4878,7 @@ void diff_setup_done(struct diff_options *options)
*/
if ((options->xdl_opts & XDF_WHITESPACE_FLAGS) ||
+ options->flags.exit_with_status ||
options->ignore_regex_nr)
options->flags.diff_from_contents = 1;
else
@@ -6742,7 +6769,7 @@ void diff_flush(struct diff_options *options)
if (output_format & DIFF_FORMAT_CALLBACK)
options->format_callback(q, options, options->format_callback_data);
- if (output_format & DIFF_FORMAT_NO_OUTPUT &&
+ if ((!output_format || output_format & DIFF_FORMAT_NO_OUTPUT) &&
options->flags.exit_with_status &&
options->flags.diff_from_contents) {
/*
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index fdd865f7c3..26430ca66b 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -172,6 +172,14 @@ test_expect_success 'no diff with -diff' '
grep Binary out
'
+test_expect_success 'diff.external and --exit-code with output' '
+ test_expect_code 1 git -c diff.external=echo diff --exit-code
+'
+
+test_expect_success 'diff.external and --exit-code without output' '
+ git -c diff.external=true diff --exit-code
+'
+
echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file
test_expect_success 'force diff with "diff"' '
--
2.45.0
next prev parent reply other threads:[~2024-05-05 10:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 1:13 Possible git-diff bug when using exit-code with diff filters German Lashevich
2024-04-21 10:42 ` René Scharfe
2024-04-21 18:17 ` Junio C Hamano
2024-04-21 18:32 ` rsbecker
2024-04-21 19:09 ` Junio C Hamano
2024-04-21 20:18 ` rsbecker
2024-05-05 10:19 ` René Scharfe
2024-05-06 17:22 ` Junio C Hamano
2024-05-05 10:19 ` [PATCH 1/2] diff: report unmerged paths as changes in run_diff_cmd() René Scharfe
2024-05-05 10:20 ` René Scharfe [this message]
2024-05-05 15:25 ` [PATCH 2/2] diff: fix --exit-code with external diff Phillip Wood
2024-05-06 17:31 ` Junio C Hamano
2024-05-06 18:23 ` René Scharfe
2024-05-08 15:25 ` phillip.wood123
2024-05-11 20:32 ` René Scharfe
2024-05-12 9:38 ` René Scharfe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e2e4a4e9-55db-403c-902d-fd8af3aea05c@web.de \
--to=l.s.r@web.de \
--cc=german.lashevich@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.