From patchwork Sun Jun 9 07:38:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13691170 Received: from mout.web.de (mout.web.de [212.227.15.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 363C318C3D for ; Sun, 9 Jun 2024 07:38:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.15.4 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918722; cv=none; b=lmOK7EZSoWdwnwTim/G5A5dFETWC8/dfz9TeJVSOtFT2lb5ptpYmsxK4EZTSfmzKy1/mY4HWz/TsdM2Ofd2UC7JdqRvTcgg5jBnm4tOuJF4Zv3vxgc19xwAzEheSiL5EN+jmO/mpgJEkhN/MEc0xxG6N+mEz924FX2SV2aJRmxQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918722; c=relaxed/simple; bh=0STcVmAPNNsn85GMwJVIiF216J3lKtbRFlSMLJ2cNu8=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=i7OTQ/A+wZI5sl19I8x7dbgRTzSn2Eizgusm+n9i4JBEujJHBLUT6LNpu14iXtByLQHsL3rCrzwiuOdLmOrPd3p8fM0zmDJfI5NtI0zYa6XKvLEebNZpgyFyfaWSlxjE7Udsn88Rz45bKHe1QVl/S7f0OXf8vKP4JhwBFvPsZOs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de; spf=pass smtp.mailfrom=web.de; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b=m+rQYuZV; arc=none smtp.client-ip=212.227.15.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=web.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b="m+rQYuZV" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717918705; x=1718523505; i=l.s.r@web.de; bh=Bqt9Rg7hPRbBAzr4LAsNgx/ralLwMDpofq4Rnpicyy4=; h=X-UI-Sender-Class:Message-ID:Date:MIME-Version:Subject:From:To: Cc:References:In-Reply-To:Content-Type:Content-Transfer-Encoding: cc:content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=m+rQYuZVOg09aVjRtC5/t/XNvl7a/Xik/IC0tXNDC4w7obArxzGD5ePNi4ZEKy55 maN4Hwp5XEAihY0Lw99K/y+JbmbjYUu+w4/h1AxlAMrLIaVlKoIZ/1wFC8EX52NVO RwwSmYgXR6gliX0pLiIbIJcVmWzy7YhKsu48kaVBuxnFoVh5hx9Gou2UyTYJVR6iq GtXHe3VsJt2MzARy9TGzM1lQ9meoT+GbVTSXyXugfB5hzHa6WmcRr/fbxDLzC7bUJ x1zZdQFK8598ATxLS8fOw9Ije899ejWBy0MMY6pCIOCOVLbkqmoYCkdnM+l/jTNDT QBUVXCHCa++a4T7OPg== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb006 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MJnrh-1s0ST21oQz-00Qy1k; Sun, 09 Jun 2024 09:38:25 +0200 Message-ID: <79a951f8-cf35-4159-a90e-f95d69773413@web.de> Date: Sun, 9 Jun 2024 09:38:24 +0200 Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PATCH v3 1/3] t4020: test exit code with external diffs From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: git@vger.kernel.org Cc: Junio C Hamano , German Lashevich , Phillip Wood , Johannes Sixt References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> Content-Language: en-US In-Reply-To: <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> X-Provags-ID: V03:K1:j7JNx8eqHtolTuYE5hlPELhEc75L8vhlJyMmKuFwU/YcR4RBt35 MF7TLiIkiUgduncNIjYLPTf7/PCvYVkgKoN92Hj4fxQbjoc3u9IqNZVG4U9AzOtj/GdVBSY iZoct3uto95nld9qDskDXNnEXla2R6NjTcbCEB6J/TNNFlCzS9p/YY41VSTuL8GgVwomZaZ 3cWyFUe0bB2Pw2F0ba4pw== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:1Mmr9sZm+58=;HVgamukdEZXd4Sr1YRULz4vTYW/ vxOetxIshsEMJnwfBca7ViiyU4BTZNf4Yo1PZFO+hafLC565GOkvkwyQ7egmIjrnqaDjhrnLZ Vc3ARbknYLg+ZI0qgxzuW6znGUIAyDNNhcmW8hieqjt80/dMhLgKuI5zfwVQSeFNFmX45Vu4q 3QLTDo8vUZ5JHZdgiBdW6Mss8aUv9oQS6PEYradn1jRmZob7/0Kyz13VKoVSaORthZUTmsMBJ wdAIVpetdXCRbZcjJioarSaMPXkH8r1DXohVetou2iotptqYGnt1QImEpvM4RlTqruD3KslC1 2kaebyPRog9CD57+mLoXvLQVTg0p2R6PdV5qYEKrLJ/i0rjdP/vfpGPL4iTd85NQLhqtNUo9r j5+M38grku+IIRtn+2XMH2x3iY2mycDm5WwNgyLM76j0kO3yduzL0BfpdM0YwzNK5ZD1VjqXe mCiTXBc5QtV7/l/lO8szoxa7GrQ4+QYV772pBJ3eHULRVx+wSGEm1W4xLdqpvqH5ZqlyBAQRH ou0iMBq0+gXZOMKBLbOLvcz2dOzgvEuWpG81WH29N8jIx9WKlRa+wjmWausaOyyxWpqlEl4sL +T41ps26JSFKycthiiVnPeL0tRkuFqCKaUK8HzN8g3hEG6BVdOFlMmYBzSIk56Cbe5/b0QkVi Q4/DM3eJL/38I+CEtXmetiZl3H2jDVHXQY6v1+HqIGlG1MbajcYNeJ/CoB5q+67YODnaqHmBv cNiFLTwTaxlSIHo8VU/op+gjZtJ+QQGLusivmrchFTvNrSA82QKuvX0cz7CWbCka4WX1aZ1qz g6T08OWW6BYradj31rpyWpbaPW/TJ5if56e0rjEMpxu4g= Add tests to check the exit code of git diff with its options --quiet and --exit-code when using an external diff program. Currently we cannot tell whether it found significant changes or not. While at it, document briefly that --quiet turns off execution of external diff programs because that behavior surprised me for a moment while writing the tests. Signed-off-by: René Scharfe --- Documentation/diff-options.txt | 1 + t/t4020-diff-external.sh | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) -- 2.45.2 diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index c7df20e571..6b73daf540 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -820,6 +820,7 @@ ifndef::git-log[] --quiet:: Disable all output of the program. Implies `--exit-code`. + Disables execution of external diff helpers. endif::git-log[] endif::git-format-patch[] diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index fdd865f7c3..4070523cdb 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -172,6 +172,59 @@ test_expect_success 'no diff with -diff' ' grep Binary out ' +check_external_diff () { + expect_code=$1 + expect_out=$2 + expect_err=$3 + command_code=$4 + shift 4 + options="$@" + + command="echo output; exit $command_code;" + desc="external diff '$command'" + with_options="${options:+ with }$options" + + test_expect_success "$desc via attribute$with_options" " + test_config diff.foo.command \"$command\" && + echo \"file diff=foo\" >.gitattributes && + test_expect_code $expect_code git diff $options >out 2>err && + test_cmp $expect_out out && + test_cmp $expect_err err + " + + test_expect_success "$desc via diff.external$with_options" " + test_config diff.external \"$command\" && + >.gitattributes && + test_expect_code $expect_code git diff $options >out 2>err && + test_cmp $expect_out out && + test_cmp $expect_err err + " + + test_expect_success "$desc via GIT_EXTERNAL_DIFF$with_options" " + >.gitattributes && + test_expect_code $expect_code env \ + GIT_EXTERNAL_DIFF=\"$command\" \ + git diff $options >out 2>err && + test_cmp $expect_out out && + test_cmp $expect_err err + " +} + +test_expect_success 'setup output files' ' + : >empty && + echo output >output && + echo "fatal: external diff died, stopping at file" >error +' + +check_external_diff 0 output empty 0 +check_external_diff 128 output error 1 + +check_external_diff 1 output empty 0 --exit-code +check_external_diff 128 output error 1 --exit-code + +check_external_diff 1 empty empty 0 --quiet +check_external_diff 1 empty empty 1 --quiet # we don't even call the program + echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file test_expect_success 'force diff with "diff"' ' From patchwork Sun Jun 9 07:39:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13691171 Received: from mout.web.de (mout.web.de [212.227.15.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 38D9D1BF40 for ; Sun, 9 Jun 2024 07:39:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.15.4 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918757; cv=none; b=pFLw8TJ6eb2iAm+fdMsGESS7pvVCJfpKgOcrRhEcz348WtPrqXbESrKwl0A7vhLkSPTzLNCHT7m28HxergSy9IF1MUzJuyfadp97p9ocsNsFkgmbknulNlngdlvYNHWMCdRTYU+If7+DH3IdQDwJsWMoAzESvtCF8oyCKfT2UNs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918757; c=relaxed/simple; bh=Ff4SKO1h8WU9QiV/k7A7pSBYC3F5y78Rm1F+KokwtOQ=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=IZEvP5M5J5+F3h39v4VGP4tcO2VHLcuKHpJDUtfuBsx/Yhe/qvTgZ1o9+0gVYb20PwQ4LnHppa09cvxx44oMcJzNDqxMygMW7MXfo4NxbsdiHBJN7FGN5vk9QUb0cNeHczDtGfxkm2ZmO6L+Ww/294AWQIEiiAlY9am2djlwXBg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de; spf=pass smtp.mailfrom=web.de; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b=ho1Zm7kH; arc=none smtp.client-ip=212.227.15.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=web.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b="ho1Zm7kH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717918748; x=1718523548; i=l.s.r@web.de; bh=oCLkyEMNqoqKFAfp5x4mE0m6AwFFc2iCve8pW9tfhOI=; h=X-UI-Sender-Class:Message-ID:Date:MIME-Version:Subject:From:To: Cc:References:In-Reply-To:Content-Type:Content-Transfer-Encoding: cc:content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=ho1Zm7kH/bbWX7zjyln4CYxMTR8F1179Qtu898SJduEe15v16GOiovkcO+W7zNoM a+4tDATyTDjanCpQt8NGJmwXF/I5AZVwwRZ/jQET8xS2UdOM9402Uk1RABaZb3eFP 6wvCvzMZE0gDDG/sCwVTf6UbXhQC9R6RcbVMp/o9c/tlzeQk9jYh6mdqelT2SKBXJ noNWluOQdisLrjw8CJTZ7wtZM+I85CQ7oLjLPGoiolisZBD6gps0OAXH4If6HGAo2 PWDtbo8Ar0ZPCPAMFBsy46XkoM4RTujxDRNslzmC4naABqiaai0e6C9ozrcGhKAte zKrAcytCxdiH3sTtgQ== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb006 [213.165.67.108]) with ESMTPSA (Nemesis) id 1MK574-1s0BOM2USU-00RnBv; Sun, 09 Jun 2024 09:39:08 +0200 Message-ID: <0dd72a49-1c6d-4cd9-9257-d942443e1bc3@web.de> Date: Sun, 9 Jun 2024 09:39:08 +0200 Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PATCH v3 2/3] userdiff: add and use struct external_diff From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: git@vger.kernel.org Cc: Junio C Hamano , German Lashevich , Phillip Wood , Johannes Sixt References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> Content-Language: en-US In-Reply-To: <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> X-Provags-ID: V03:K1:jjqr12oY32eEBXrB5GfjYghKMaAUcKK2f0zce7J0XZCHtq9YUzY JVUO+tLcHd1K8QUWYU9OvjyAfksDTN5fWIKfxY3483wQbwYmWX+dj7rENNMQX8HZupiSTO6 AVnNPLZlQYbWwQv4VsBmmI8PCRDfKPWgZxU0Cq8FCgeDdFNAG+n8SZ9JZwiomCjjQCl5YW5 csH+xzEznzvzl2v3m6PAQ== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:uqhf/LgFh6o=;VCMhs2U//7kgEH+PknBQL9yqj/O UnNyr6QVOmDYEwX8mga0FJlAubfE6Yd5pGXO/10fWdnlisypbWMfUN8TSt+BQQyVmUIG8m5G6 sGnhhrcW8IEA8jOAdi6X/7QUe1qQ7B6YXS4xbO8WrcD3LKWrRm1l3UZRHaHHo/IaRS/o7V4EH LyvVN2va1YK5z8WFiu0+uhsisdf2MrNJ+PtyvGwqpsZrzxFFzZicz/qUyDfDNV3T42jnDusKB jRzi+WG1NB5WP6wZLXATrLhbc0T4Q7gVmwtT3/V9JPkWbnvBfs7gEjv4Ub0m5ofwuyG4+JCA0 30sjcmiqN6hkPg/4FZGrNGyMJqWJZYgt1UqsgAnRQupcoyxlCNrFoXTvxEOvY/ySh8W/KfKwq sz1Efp5stvTsgyir4EZKtrPuh21iupffHWZF4cPW6ysqJdl++LbdSIfBsogyWB3hmkuWqmbDX Uwhew4ZX/tiqfqGvRgQLEAT2Joi8x9IADISBPHDIVQfGkh0YAq0Msxhl1HIM8w7zIc+2qFWVa wCUlww9cpEntWaimwMWEi7Y7f1TZK+ivICm/jvgflGSFMfeEg0FTz4WpYNtAhqJbGDstgq3Wk C2p5DBxzYgGHoPmDkV2p1qX+T9VAb4uXBmvi8VU5KKgviP187BC1AmP2I9kk2Gf/1e5jvxN80 skZ+NOR3a+Th8k0ESJ7cbkMSm0PtAfAGLWIdz44paeLH6jNbM3VISkPtWZSurCFk7zAK3Fp4j A5eYBQ6LzfDpuqkc2Qft9Va/0XIuCC+gB4ybpcRZ1A9U7fTElhCHUSQHA9Imt5GjBCedfWyA4 +B3HPMFU0JQDQyxjovApYDEvLpQ0fOSE1tsIKZReyakms= Wrap the string specifying the external diff command in a new struct to simplify adding attributes, which the next patch will do. Make sure external_diff() still returns NULL if neither the environment variable GIT_EXTERNAL_DIFF nor the configuration option diff.external is set, to continue allowing its use in a boolean context. Use a designated initializer for the default builtin userdiff driver to adjust to the type change of the second struct member. Spelling out only the non-zero members improves readability as a nice side-effect. No functional change intended. Signed-off-by: René Scharfe --- diff.c | 32 +++++++++++++++++--------------- userdiff.c | 4 ++-- userdiff.h | 6 +++++- 3 files changed, 24 insertions(+), 18 deletions(-) -- 2.45.2 diff --git a/diff.c b/diff.c index e70301df76..4b86c61631 100644 --- a/diff.c +++ b/diff.c @@ -57,7 +57,7 @@ static int diff_color_moved_ws_default; static int diff_context_default = 3; static int diff_interhunk_context_default; static char *diff_word_regex_cfg; -static char *external_diff_cmd_cfg; +static struct external_diff external_diff_cfg; static char *diff_order_file_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; @@ -431,7 +431,7 @@ int git_diff_ui_config(const char *var, const char *value, return 0; } if (!strcmp(var, "diff.external")) - return git_config_string(&external_diff_cmd_cfg, var, value); + return git_config_string(&external_diff_cfg.cmd, var, value); if (!strcmp(var, "diff.wordregex")) return git_config_string(&diff_word_regex_cfg, var, value); if (!strcmp(var, "diff.orderfile")) @@ -548,18 +548,20 @@ static char *quote_two(const char *one, const char *two) return strbuf_detach(&res, NULL); } -static const char *external_diff(void) +static const struct external_diff *external_diff(void) { - static const char *external_diff_cmd = NULL; + static struct external_diff external_diff_env, *external_diff_ptr; static int done_preparing = 0; if (done_preparing) - return external_diff_cmd; - external_diff_cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF")); - if (!external_diff_cmd) - external_diff_cmd = external_diff_cmd_cfg; + return external_diff_ptr; + external_diff_env.cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF")); + if (external_diff_env.cmd) + external_diff_ptr = &external_diff_env; + else if (external_diff_cfg.cmd) + external_diff_ptr = &external_diff_cfg; done_preparing = 1; - return external_diff_cmd; + return external_diff_ptr; } /* @@ -4375,7 +4377,7 @@ static void add_external_diff_name(struct repository *r, * infile2 infile2-sha1 infile2-mode [ rename-to ] * */ -static void run_external_diff(const char *pgm, +static void run_external_diff(const struct external_diff *pgm, const char *name, const char *other, struct diff_filespec *one, @@ -4386,7 +4388,7 @@ static void run_external_diff(const char *pgm, struct child_process cmd = CHILD_PROCESS_INIT; struct diff_queue_struct *q = &diff_queued_diff; - strvec_push(&cmd.args, pgm); + strvec_push(&cmd.args, pgm->cmd); strvec_push(&cmd.args, name); if (one && two) { @@ -4512,7 +4514,7 @@ static void fill_metainfo(struct strbuf *msg, } } -static void run_diff_cmd(const char *pgm, +static void run_diff_cmd(const struct external_diff *pgm, const char *name, const char *other, const char *attr_path, @@ -4530,8 +4532,8 @@ static void run_diff_cmd(const char *pgm, if (o->flags.allow_external || !o->ignore_driver_algorithm) drv = userdiff_find_by_path(o->repo->index, attr_path); - if (o->flags.allow_external && drv && drv->external) - pgm = drv->external; + if (o->flags.allow_external && drv && drv->external.cmd) + pgm = &drv->external; if (msg) { /* @@ -4597,7 +4599,7 @@ static void strip_prefix(int prefix_length, const char **namep, const char **oth static void run_diff(struct diff_filepair *p, struct diff_options *o) { - const char *pgm = external_diff(); + const struct external_diff *pgm = external_diff(); struct strbuf msg; struct diff_filespec *one = p->one; struct diff_filespec *two = p->two; diff --git a/userdiff.c b/userdiff.c index 82bc76b910..f47e2d9f36 100644 --- a/userdiff.c +++ b/userdiff.c @@ -333,7 +333,7 @@ PATTERNS("scheme", "|([^][)(}{[ \t])+"), PATTERNS("tex", "^(\\\\((sub)*section|chapter|part)\\*{0,1}\\{.*)$", "\\\\[a-zA-Z@]+|\\\\.|([a-zA-Z0-9]|[^\x01-\x7f])+"), -{ "default", NULL, NULL, -1, { NULL, 0 } }, +{ .name = "default", .binary = -1 }, }; #undef PATTERNS #undef IPATTERN @@ -445,7 +445,7 @@ int userdiff_config(const char *k, const char *v) if (!strcmp(type, "binary")) return parse_tristate(&drv->binary, k, v); if (!strcmp(type, "command")) - return git_config_string(&drv->external, k, v); + return git_config_string(&drv->external.cmd, k, v); if (!strcmp(type, "textconv")) return git_config_string(&drv->textconv, k, v); if (!strcmp(type, "cachetextconv")) diff --git a/userdiff.h b/userdiff.h index cc8e5abfef..2d59a8fc56 100644 --- a/userdiff.h +++ b/userdiff.h @@ -11,9 +11,13 @@ struct userdiff_funcname { int cflags; }; +struct external_diff { + char *cmd; +}; + struct userdiff_driver { const char *name; - char *external; + struct external_diff external; char *algorithm; int binary; struct userdiff_funcname funcname; From patchwork Sun Jun 9 07:41:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ren=C3=A9_Scharfe?= X-Patchwork-Id: 13691172 Received: from mout.web.de (mout.web.de [212.227.15.14]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BD2681C2A3 for ; Sun, 9 Jun 2024 07:41:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.15.14 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918921; cv=none; b=V0oWZ8r1O8R3zTLJFoQgbr7GGdsypv9eh8XUreV2C18OFNHm0f3h2kbMfRheFYrt6CGlYjrVBLhL7GJKLLUy+7DN2QpJMp49eh02teAoxlMvuaU83q6RSdXxJo+3jT1vxEDWKatleugb2jjXJr7mNli00Yky40HlGQlV1caNmhk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717918921; c=relaxed/simple; bh=cHOLHbj3WikykLBzJlI84zhX0kpIi8lTVuNo20juVls=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=HnfHDXu1eRFtP/8r/ml9GMCFOdUJNWfKEP7FM5TjsjQPBTyAH6TfeY1FtNPk50vtUxNC1L8/rOSXcB/5fRKSNFao8n0vheM0n2Sgn8j/4wG8sx4Baf21gKTJ4+OP7Db5QSLd4PxB2O1YP/ooAqix08++DKoYqSoXEfLQ0XhDIfE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de; spf=pass smtp.mailfrom=web.de; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b=fGo85c+5; arc=none smtp.client-ip=212.227.15.14 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=web.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=web.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=web.de header.i=l.s.r@web.de header.b="fGo85c+5" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717918905; x=1718523705; i=l.s.r@web.de; bh=aaWZQPiq5fUEiV6WDmWGIXu8RZbcFTirUZIhtKDOsJQ=; h=X-UI-Sender-Class:Message-ID:Date:MIME-Version:Subject:From:To: Cc:References:In-Reply-To:Content-Type:Content-Transfer-Encoding: cc:content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=fGo85c+5QZt7NHnKi8Juk4XZ+gs5JHy+2EiZ0swYRZuwa3PBvL6bKb7RQAppddKB Eo9Wd2+kvqE3SQXajZ+EiHvLwVs4NHUC5YbfVR3eEAqmgQ0zjK/OMUsENaLAc/sib cACRXg6SgWZEBH41ZzmjIMH85kA86XiOcIBYrFhP3ysgwBGivKBfWTrAE24IaKiyE 5z11mOI+HtLqpau4cm4KZ/d0Lp6/k4VS4BFb7AIuN0ujPQHTIM+yBCb3dbB34/EGW 8PxRa23ninIOLCQUfAf3ept8EMudH8o8ZZZta/57seJbWa1PuAnWinVmOgAa1gQDf TeNOWJJd2PLf6AWpnw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb005 [213.165.67.108]) with ESMTPSA (Nemesis) id 1N9479-1sSfcM1x8Z-00yhKb; Sun, 09 Jun 2024 09:41:45 +0200 Message-ID: <6ed75dd5-f4e7-4dac-a8e8-a0c44376e5af@web.de> Date: Sun, 9 Jun 2024 09:41:44 +0200 Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [PATCH v3 3/3] diff: let external diffs report that changes are uninteresting From: =?utf-8?q?Ren=C3=A9_Scharfe?= To: git@vger.kernel.org Cc: Junio C Hamano , German Lashevich , Phillip Wood , Johannes Sixt References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> Content-Language: en-US In-Reply-To: <168fecaa-2ebd-4897-b0ba-3bd2a37c01e7@web.de> X-Provags-ID: V03:K1:vNNL/KQCHOTJPZa4ucozqPL/1BoQfa5NCAe97lLYb5jF68N5sMv levtjNq/UHso4d/VpoD0azGblPPztDiD3WjyfH1Qbf/w+//MgVAq5qs2+NlfnPWy62bpHjQ eRisX77dDFz6X92MgpevxqzFbZsi6gDCFBsK856fnpZCCt0UThP5iu0MWTl1IBDNYl5SjGk grPbRYZBtSGBl+32UFmXg== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:yQWebd9bHnI=;urgWiqZrqXCrLdK4hTC8lYxWX+J BydyblqIMWiSKkc+MeoHoS3xeHwPANm0Hc+ryRXQLcmgE9zR8tVx71+Py4RPBb3a+J++tjEb2 fABC9lE4f0xYRvGvyHDGayUZ0QY5C1LfAEc2TljRSPKLR16ROsNa26J6Jo5FDSuT7y09xjIZN vLiK/B//fEpQ9BAzOzk4Grh7fYxAjhO4Uhpc2AiQNNyXOBChNN6A/dVBJcCRSBeRhMP0DfVn/ 2PuX+/sgPEDON1WKcCAKm512ZTaWBTHnp1pNTCBSaj/1DqTsPLTXEF3AbZJ83TK1PeUXM56+4 xIBbOTayNubslX17vuuFu+UVEQSBRicXixGsvcRdYc71+vXd+BxTswM4wuIhIwLh+9Mf4EJrL vcGzX7Ta0zS374Olv3Uw9qzQQwhErwinWlKfSTKeT6gLSkdz2w5wyfyUtjeCYZ1R7Gk92YQto UPd+fjxqpfdXoz36qyMTGiIsEVwfp/K0yPk6YL2p6LTAs6pRZPNHaBAbNQUkdRMzjqjJ/S3az 8WiQRpeO02bXXlUwLcL0TgcPK/2U6zVXKxGIF8IBVU/ZEOLyydoioP0PzCCiMkMcn9R52b7Va cS2pCYZT2G7zJdZgadz7OgMRZwM638KiL8Qn+vMcV1dYRfDfUMrsMN9Z0jImgqhDcxStUHWNG 20MldlCwKRMZpf4qgq5st1+gwKmMJRWtnY6oZxpuViKS7x5v8wmx1M54g3CJx25wEznHX/rGJ vzPnY8IdKlY4WzFYDw+b7o+0HH9ED5u9dQTPbflgIlVS5YldCc/AtNf30x7koo6igLgrOsNvH vj23QpeXxBUVVwfnXpFm4hxkd6yl2fpIhW/JqFIapfxSA= The options --exit-code and --quiet instruct git diff to indicate whether it found any significant changes by exiting with code 1 if it did and 0 if there were none. Currently this doesn't work if external --- 2.45.2 diff programs are involved, as we have no way to learn what they found. Add that ability in the form of the new configuration options diff.trustExitCode and diff..trustExitCode and the environment variable GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE. They pair with the config options diff.external and diff..command and the environment variable GIT_EXTERNAL_DIFF, respectively. The new options are off by default, keeping the old behavior. Enabling them indicates that the external diff returns exit code 1 if it finds significant changes and 0 if it doesn't, like diff(1). The name of the new options is taken from the git difftool and mergetool options of similar purpose. (There they enable passing on the exit code of a diff tool and to infer whether a merge done by a merge tool is successful.) The new feature sets the diff flag diff_from_contents in diff_setup_done() if we need the exit code and are allowed to call external diffs. This disables the optimization that avoids calling the program with --quiet. Add it back by skipping the call if the external diff is not able to report empty diffs. We can only do that check after evaluating the file-specific attributes in run_external_diff(). If we do run the external diff with --quiet, send its output to /dev/null. I considered checking the output of the external diff to check whether its empty. It was added as 11be65cfa4 (diff: fix --exit-code with external diff, 2024-05-05) and quickly reverted, as it does not work with external diffs that do not write to stdout. There's no reason why a graphical diff tool would even need to write anything there at all. I also considered using a non-zero exit code for empty diffs, which could be done without adding new configuration options. We'd need to disable the optimization that allows git diff --quiet to skip calling external diffs, though -- that might be quite surprising if graphical diff programs are involved. And assigning the opposite meaning of the exit codes compared to diff(1) and git diff --exit-code to the external diff can cause unnecessary confusion. Suggested-by: Phillip Wood Signed-off-by: René Scharfe --- Documentation/config/diff.txt | 18 +++++++++++++++++ Documentation/diff-options.txt | 6 +++++- Documentation/git.txt | 10 +++++++++ Documentation/gitattributes.txt | 5 +++++ diff.c | 36 ++++++++++++++++++++++++++++++++- t/t4020-diff-external.sh | 33 +++++++++++++++++++++--------- userdiff.c | 4 ++++ userdiff.h | 1 + 8 files changed, 101 insertions(+), 12 deletions(-) diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt index 5ce7b91f1d..190bda17e5 100644 --- a/Documentation/config/diff.txt +++ b/Documentation/config/diff.txt @@ -79,6 +79,15 @@ diff.external:: you want to use an external diff program only on a subset of your files, you might want to use linkgit:gitattributes[5] instead. +diff.trustExitCode:: + If this boolean value is set to true then the + `diff.external` command is expected to return exit code + 0 if it considers the input files to be equal or 1 if it + considers them to be different, like `diff(1)`. + If it is set to false, which is the default, then the command + is expected to return exit code 0 regardless of equality. + Any other exit code causes Git to report a fatal error. + diff.ignoreSubmodules:: Sets the default value of --ignore-submodules. Note that this affects only 'git diff' Porcelain, and not lower level 'diff' @@ -164,6 +173,15 @@ diff..command:: The custom diff driver command. See linkgit:gitattributes[5] for details. +diff..trustExitCode:: + If this boolean value is set to true then the + `diff..command` command is expected to return exit code + 0 if it considers the input files to be equal or 1 if it + considers them to be different, like `diff(1)`. + If it is set to false, which is the default, then the command + is expected to return exit code 0 regardless of equality. + Any other exit code causes Git to report a fatal error. + diff..xfuncname:: The regular expression that the diff driver should use to recognize the hunk header. A built-in pattern may also be used. diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 6b73daf540..cd0b81adbb 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -820,7 +820,11 @@ ifndef::git-log[] --quiet:: Disable all output of the program. Implies `--exit-code`. - Disables execution of external diff helpers. + Disables execution of external diff helpers whose exit code + is not trusted, i.e. their respective configuration option + `diff.trustExitCode` or `diff..trustExitCode` or + environment variable `GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE` is + false. endif::git-log[] endif::git-format-patch[] diff --git a/Documentation/git.txt b/Documentation/git.txt index a31a70acca..4489e2297a 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -644,6 +644,16 @@ parameter, . For each path `GIT_EXTERNAL_DIFF` is called, two environment variables, `GIT_DIFF_PATH_COUNTER` and `GIT_DIFF_PATH_TOTAL` are set. +`GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE`:: + If this Boolean environment variable is set to true then the + `GIT_EXTERNAL_DIFF` command is expected to return exit code + 0 if it considers the input files to be equal or 1 if it + considers them to be different, like `diff(1)`. + If it is set to false, which is the default, then the command + is expected to return exit code 0 regardless of equality. + Any other exit code causes Git to report a fatal error. + + `GIT_DIFF_PATH_COUNTER`:: A 1-based counter incremented by one for every path. diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 4338d023d9..80cae17f37 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -776,6 +776,11 @@ with the above configuration, i.e. `j-c-diff`, with 7 parameters, just like `GIT_EXTERNAL_DIFF` program is called. See linkgit:git[1] for details. +If the program is able to ignore certain changes (similar to +`git diff --ignore-space-change`), then also set the option +`trustExitCode` to true. It is then expected to return exit code 1 if +it finds significant changes and 0 if it doesn't. + Setting the internal diff algorithm ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/diff.c b/diff.c index 4b86c61631..39ba842049 100644 --- a/diff.c +++ b/diff.c @@ -432,6 +432,10 @@ int git_diff_ui_config(const char *var, const char *value, } if (!strcmp(var, "diff.external")) return git_config_string(&external_diff_cfg.cmd, var, value); + if (!strcmp(var, "diff.trustexitcode")) { + external_diff_cfg.trust_exit_code = git_config_bool(var, value); + return 0; + } if (!strcmp(var, "diff.wordregex")) return git_config_string(&diff_word_regex_cfg, var, value); if (!strcmp(var, "diff.orderfile")) @@ -556,6 +560,8 @@ static const struct external_diff *external_diff(void) if (done_preparing) return external_diff_ptr; external_diff_env.cmd = xstrdup_or_null(getenv("GIT_EXTERNAL_DIFF")); + if (git_env_bool("GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE", 0)) + external_diff_env.trust_exit_code = 1; if (external_diff_env.cmd) external_diff_ptr = &external_diff_env; else if (external_diff_cfg.cmd) @@ -4387,6 +4393,19 @@ static void run_external_diff(const struct external_diff *pgm, { struct child_process cmd = CHILD_PROCESS_INIT; struct diff_queue_struct *q = &diff_queued_diff; + int quiet = !(o->output_format & DIFF_FORMAT_PATCH); + int rc; + + /* + * Trivial equality is handled by diff_unmodified_pair() before + * we get here. If we don't need to show the diff and the + * external diff program lacks the ability to tell us whether + * it's empty then we consider it non-empty without even asking. + */ + if (!pgm->trust_exit_code && quiet) { + o->found_changes = 1; + return; + } strvec_push(&cmd.args, pgm->cmd); strvec_push(&cmd.args, name); @@ -4408,7 +4427,15 @@ static void run_external_diff(const struct external_diff *pgm, diff_free_filespec_data(one); diff_free_filespec_data(two); cmd.use_shell = 1; - if (run_command(&cmd)) + cmd.no_stdout = quiet; + rc = run_command(&cmd); + if (!pgm->trust_exit_code && rc == 0) + o->found_changes = 1; + else if (pgm->trust_exit_code && rc == 0) + ; /* nothing */ + else if (pgm->trust_exit_code && rc == 1) + o->found_changes = 1; + else die(_("external diff died, stopping at %s"), name); remove_tempfile(); @@ -4926,6 +4953,13 @@ void diff_setup_done(struct diff_options *options) options->flags.exit_with_status = 1; } + /* + * External diffs could declare non-identical contents equal + * (think diff --ignore-space-change). + */ + if (options->flags.allow_external && options->flags.exit_with_status) + options->flags.diff_from_contents = 1; + options->diff_path_counter = 0; if (options->flags.follow_renames) diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index 4070523cdb..3baa52a9bf 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -177,15 +177,17 @@ check_external_diff () { expect_out=$2 expect_err=$3 command_code=$4 - shift 4 + trust_exit_code=$5 + shift 5 options="$@" command="echo output; exit $command_code;" - desc="external diff '$command'" + desc="external diff '$command' with trustExitCode=$trust_exit_code" with_options="${options:+ with }$options" test_expect_success "$desc via attribute$with_options" " test_config diff.foo.command \"$command\" && + test_config diff.foo.trustExitCode $trust_exit_code && echo \"file diff=foo\" >.gitattributes && test_expect_code $expect_code git diff $options >out 2>err && test_cmp $expect_out out && @@ -194,6 +196,7 @@ check_external_diff () { test_expect_success "$desc via diff.external$with_options" " test_config diff.external \"$command\" && + test_config diff.trustExitCode $trust_exit_code && >.gitattributes && test_expect_code $expect_code git diff $options >out 2>err && test_cmp $expect_out out && @@ -204,6 +207,7 @@ check_external_diff () { >.gitattributes && test_expect_code $expect_code env \ GIT_EXTERNAL_DIFF=\"$command\" \ + GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE=$trust_exit_code \ git diff $options >out 2>err && test_cmp $expect_out out && test_cmp $expect_err err @@ -216,14 +220,23 @@ test_expect_success 'setup output files' ' echo "fatal: external diff died, stopping at file" >error ' -check_external_diff 0 output empty 0 -check_external_diff 128 output error 1 - -check_external_diff 1 output empty 0 --exit-code -check_external_diff 128 output error 1 --exit-code - -check_external_diff 1 empty empty 0 --quiet -check_external_diff 1 empty empty 1 --quiet # we don't even call the program +check_external_diff 0 output empty 0 off +check_external_diff 128 output error 1 off +check_external_diff 0 output empty 0 on +check_external_diff 0 output empty 1 on +check_external_diff 128 output error 2 on + +check_external_diff 1 output empty 0 off --exit-code +check_external_diff 128 output error 1 off --exit-code +check_external_diff 0 output empty 0 on --exit-code +check_external_diff 1 output empty 1 on --exit-code +check_external_diff 128 output error 2 on --exit-code + +check_external_diff 1 empty empty 0 off --quiet +check_external_diff 1 empty empty 1 off --quiet # we don't even call the program +check_external_diff 0 empty empty 0 on --quiet +check_external_diff 1 empty empty 1 on --quiet +check_external_diff 128 empty error 2 on --quiet echo NULZbetweenZwords | perl -pe 'y/Z/\000/' > file diff --git a/userdiff.c b/userdiff.c index f47e2d9f36..6cdfb147c3 100644 --- a/userdiff.c +++ b/userdiff.c @@ -446,6 +446,10 @@ int userdiff_config(const char *k, const char *v) return parse_tristate(&drv->binary, k, v); if (!strcmp(type, "command")) return git_config_string(&drv->external.cmd, k, v); + if (!strcmp(type, "trustexitcode")) { + drv->external.trust_exit_code = git_config_bool(k, v); + return 0; + } if (!strcmp(type, "textconv")) return git_config_string(&drv->textconv, k, v); if (!strcmp(type, "cachetextconv")) diff --git a/userdiff.h b/userdiff.h index 2d59a8fc56..f5cb53d0d4 100644 --- a/userdiff.h +++ b/userdiff.h @@ -13,6 +13,7 @@ struct userdiff_funcname { struct external_diff { char *cmd; + unsigned trust_exit_code:1; }; struct userdiff_driver {