From patchwork Wed Jun 5 08:35:23 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: 13686397 Received: from mout.web.de (mout.web.de [212.227.17.11]) (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 237E91946C7 for ; Wed, 5 Jun 2024 08:35:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.17.11 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576534; cv=none; b=MV5q8s/gTT4kxgYS2p51zgITES0aP2Hfk62OINsdRwL0vv1VBp4lGIdGBqz9DmkOLG7RP0xwuTZNWAuoLTwM9lzo/taYUpnLNUIWmsJMro6sjz/kgjlS2dANUIdDdGM3AvcyTIjQsZ9YGYmKRLyPpGJo563ENKRFYvtuPBCZjBY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576534; c=relaxed/simple; bh=dsyN5fCxqGE9go73srEyefJqwbYPDZLxtundaBJxgLo=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=UqsSQ+aS1flwF901t5DTOSmsGrEDG4w2YfatMYarQO1ipv8MkXZmHOdVQHJKPqJ2/k4AgtRo3Tbhc5F+3esv5im0O9+aaviZmUc5v+00gjj/qc0wLfoLbUle5DCerggC93NelA7f/b85Bz9fkcd/pMqmUnaokhZwyyZJv97ql34= 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=XU7E2HCB; arc=none smtp.client-ip=212.227.17.11 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="XU7E2HCB" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717576524; x=1718181324; i=l.s.r@web.de; bh=Y7gbaiAH7UJlV+PURwqCIRBWzErcVOnUGltLLVfsIGI=; 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=XU7E2HCBZZiRjYpCLl2BrolcCXvgvXv/H82EblEUQ/e6ymLWqTwOlF5cXwyykrIS ZQeymVwZyo6x46JsD3t3HGMoFHRHYavFcTLK1wc71uIjTQq2eS7dkVUfYj8eSAxof OVxo7+R/akmqFNzsANHaBGjFb7icsKZZBPzslEDihyw+5SGF5Y7+/U3tOEf57rQEc RfYG3H9dL48XnYMRWAWcdETd4PT/PldR2ab/3zyP58fE/6rpKD/RamK7YJYdniHLI pB9Ya92vTUt8O05gUDRym0k8ze0NMFdkhSS4CBH6WV5kHW1HBM9paNjrjQSp/Swgg /K8A2IrKQhWMU+9ubw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb106 [213.165.67.124]) with ESMTPSA (Nemesis) id 1MZjET-1ruknr32ZD-00WpPj; Wed, 05 Jun 2024 10:35:24 +0200 Message-ID: <2a5167fc-871b-4968-bacb-eaff4d7a2630@web.de> Date: Wed, 5 Jun 2024 10:35:23 +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 v2 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 References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> Content-Language: en-US In-Reply-To: <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> X-Provags-ID: V03:K1:ZDFvw8cw+zVRKV5Bx2+A8MlR25Y/vEuC3hcPv2dn7CzsCFcjLFy 40QG08wG1KIv8MmQxYSI4KMmw7RrGMiSbFueP9DGl6oKZMXOZyNBgR7m2uWfzrBEMFv54HY wAvsO/D6YaIyzcZU8jBxllA4oZijUOZE2dNCkwc2MBBIJMGPfM23U7n9BRr0Fco+H/pjHyj gC4bRZosQ+ORuHwDr5sVw== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:sUhjQOCR0kY=;CQ7DLwEIQOGdAdBGxfffpcUy+pi F2fiRyzMWLyfz5O311EoQiyKAwlkbPe3/G4FmLf9LsxX6N4oLnh7OZrA1OodJ2BDCiJxyBU2u ALBjRNrda4O2MXmM5tTK0adrC23QCYwH5xmB1E62h+XNMof/GbAkRkHkQOvd8QJYnE/b79i+g er4xxYUQK+5wd3NfgqqrR6OQV0Bpzxv1OFOgBf9TocsQLrfcb4PljOSiTtCml3w2Wljzz3EAr jIMa+FjJfTSy35MAwd7N9Vv/+19UcFvkYNsWuI05JGQVQHuhTFwy7tZ1DtgjF1mwgxobNufdG st+vXDM9oiATu+8IJaSlkdCjqzJkFnlu5+OP3p3V+fIVMVWCy2h9CtbhIvd3/fs6COFIbCHOR MMXzpbxt1goCwvQACUyCXd36lLMgwix77rSkMPvoaaO0oD6QmBGxRUABmmZeY9LNfBNrkWnil rXmoQ9G6MFUymBf1kRyK6DkJS3t7ZTBUzXXizXWnh8h3Oy7AbyEyfFEYcgV0F7xjmUDz2fBOA N3hoOVfg0g1G3bzQdXexC3A86EFkZP+J2e6ne5SlwBgVhKVXPs/zyqr5QDdURtq3BG66azpTd nXY4Mu7mG6Z/lK0Je5X6sAEDize+qs49aMeXG5kl/+/9tHMJpO6WPPcNkwH4tbbld/IC7HsPt FEM+M2VSfQ/BNwT/z+g35DCHOMlV2qzgDoqAIwlZSo7INXpBQk7nvF6Hz1fwu+oLSF9Zcjqtz TiqiN8W1tuD2YNc8hQVkDSNpl9zevmdYwK3LZxMuUQWtzOLctbD4pOAMJyZZnKfUQiKhTr0Jl EGansV4HjP971tPMPoB1Sx5hqb98z7VbvSyvFeh5saUdw= 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. Signed-off-by: René Scharfe --- t/t4020-diff-external.sh | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) -- 2.45.2 diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index fdd865f7c3..bed640b2b1 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -172,6 +172,39 @@ test_expect_success 'no diff with -diff' ' grep Binary out ' +check_external_exit_code () { + expect_code=$1 + command_code=$2 + option=$3 + + command="exit $command_code;" + desc="external diff '$command'" + + test_expect_success "$desc via attribute with $option" " + test_config diff.foo.command \"$command\" && + echo \"file diff=foo\" >.gitattributes && + test_expect_code $expect_code git diff $option + " + + test_expect_success "$desc via diff.external with $option" " + test_config diff.external \"$command\" && + >.gitattributes && + test_expect_code $expect_code git diff $option + " + + test_expect_success "$desc via GIT_EXTERNAL_DIFF with $option" " + >.gitattributes && + test_expect_code $expect_code env \ + GIT_EXTERNAL_DIFF=\"$command\" \ + git diff $option + " +} + +check_external_exit_code 1 0 --exit-code +check_external_exit_code 1 0 --quiet +check_external_exit_code 128 1 --exit-code +check_external_exit_code 1 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 Wed Jun 5 08:36:31 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: 13686420 Received: from mout.web.de (mout.web.de [212.227.17.12]) (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 284D91946AA for ; Wed, 5 Jun 2024 08:36:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.17.12 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576602; cv=none; b=iKUr6brT4mkJGWd1RVHyEM/E8HzFlSCXvpsNDRPKHDLq4rIKMhd/wLBDmCqBA9cneLpeFJxkRzy3ITfuMdpEsG6K6GF6p14G3OTDE7nnxM1Z7FxhiSdmPWA2IO41bnD7BTVksa756X5Bss82WBQeLD6yuUNhHqvMHORxs5+wllk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576602; c=relaxed/simple; bh=PxsekTgPu0VhFwaMlhO4prAv1QDs6stHZoA6CZS2YsQ=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=BjmX4H03LEHE1bnIJ3HH02NXA7csfKtXN1fqgr0jDwU5KkTgZJqQFOEfMHxDsUAN3YJKVr3fUwHdOFjjx25wlr/3ltec5/5QeXasFWG4C2XoHhs+Ay7aKplHbesxicOuhBjK/MURpYxejztvWOxIFg/TaCN19R9xgFyiou6SxyM= 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=fdr0Sgvm; arc=none smtp.client-ip=212.227.17.12 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="fdr0Sgvm" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717576592; x=1718181392; i=l.s.r@web.de; bh=2UK/cxdgbVhhp2nxaXhMjdkBam2K0w/l4wNqlAi6JU4=; 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=fdr0SgvmwSoD5KghL5T6C47l46+g5N4SfaUCAjvjbtjg2xJRpUdbK3V4kqSGTWyA qwx8ixeH8c5KS3HXORlHCz0rEUaJGs7LWTiiANl0KYcoUSF6owQgAhJuH+tfo7RNj u2njzGjKN7MNOwDLrH1LZsiHrh8HMi4Jd4Jxy8oqIhkV+BY9Coy2z2mx9N5G6EXUy ygk8qpQu+ISOX1PE4oIRQtKQJWp2s2nAwfTvwtF+cM+S+EHrqSQsG5/bhTWCJytFo 4vOthpYbcNgEIaX79sE8ji0Mkj35l+yUhu9n7h9n0yzYILWBpppVNH80X6+SPKSdp +oqwVH/3avac8Mmlfw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb105 [213.165.67.124]) with ESMTPSA (Nemesis) id 1N5lnF-1sVC3v1WAN-00vBcB; Wed, 05 Jun 2024 10:36:32 +0200 Message-ID: <87251602-1dc8-4115-ad4a-b9160ec082a9@web.de> Date: Wed, 5 Jun 2024 10:36:31 +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 v2 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 References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> Content-Language: en-US In-Reply-To: <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> X-Provags-ID: V03:K1:DUBjgJlls5lJ8BhhN23+rVvDDRd7Qy/e62N1epUbOKuu9UJpNDS phJMzL25a+h3FBWNCFBjHc1/zQrqtlCi9ful67hO9XKDVRee7poFoCSGHzGh22fNyQ+fyLC UF4GQjuZS+9Bgg2oKgCBB51zqyP8LvzlEmjzm7ajKmKkkJrK55MzuKWyuYhAM7HoBEpDTnd TPsIieWSK/7QVLl6aGTfw== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:jK6WZ3WwJNQ=;fTwo58LezK+6Y54wsSzJEMUS0FJ xo5oWyVq9iuW0KUe6rEw9JWo6wdo2l91vmilNrvsynqUrCk9Nk1+65tHVj6JyNSFlO/uS+Z1a 4vhcfVC1wuKkkLBgyY6CR49MWSB7mk/XLfwkGnIwUyPeiaiTMOZFXirsyVf5TWRjyk6bGXo5G PuCQeE7wCKptcuCRWVSFTOdW4QHIx9g0n7eIW0c/Jt7yAxj7oCcAR4aCpdQNHf9GjxocQzSBv Unyg4MfxZrzeCkOW7Wr8Vhxu6v1AWKjjtpy+rytAs7I4lTVP7kzlNCcy+GSkorXySNzhyx/Vc oaouN+3MiSvpzMqJwkQoHqs8jfKFNIFGc6SEkAsIKgBkuD0X3zfwDyGIr2J8IpBDtMunrmfSJ YXvAmDpHb3JuTyR5PH/53TJow15NnSBbdhIYohf84d+tf/Sle0wdXZEJeE+glQCN5M6nawy0u QsXSdPGJDS5HA4N07dIl3eHHsDbhXMEGTbt3muRnrUMHPthcvDpvWupk6/1PKv+XlVOl/IOj7 1uGvxDshtFbyXdCyY8wyQO0lm1SaHTU/llU1WAIf05zXCQP53OscJ2IM8924Lrr31IDhOytuf RN3p7GQeOgsN7AK7J607xafGrnj22fJU/wBvoDT+uv3lAvNygdhL1RESw0Zrn+2//YUC0Ge4d LcDz9mvRF8+tApxzubzc1OPtR3P2cpA4PSS3IjFwWMY/ETsBsapCi5U7CtkDc5P3HGJ60Dsj+ VcdXutTgNL+0PbbMdOW0CU36PoeXCzyp5Ty1HXGYZeKQ1RfKqktUfoTmsCu+43QdMU6P2+4/7 FfofzMkE/4+3Yy762rPDAvprzCNSW0mmyoPQTqsKh7ask= 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 ded9ac70df..286d093bfa 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 const char *diff_word_regex_cfg; -static const char *external_diff_cmd_cfg; +static struct external_diff external_diff_cfg; static const char *diff_order_file_cfg; int diff_auto_refresh_index = 1; static int diff_mnemonic_prefix; @@ -429,7 +429,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")) @@ -546,18 +546,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; } /* @@ -4373,7 +4375,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, @@ -4384,7 +4386,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) { @@ -4510,7 +4512,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, @@ -4528,8 +4530,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) { /* @@ -4595,7 +4597,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 d726804c3e..7e2b28e88d 100644 --- a/userdiff.h +++ b/userdiff.h @@ -11,9 +11,13 @@ struct userdiff_funcname { int cflags; }; +struct external_diff { + const char *cmd; +}; + struct userdiff_driver { const char *name; - const char *external; + struct external_diff external; const char *algorithm; int binary; struct userdiff_funcname funcname; From patchwork Wed Jun 5 08:38:36 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: 13686421 Received: from mout.web.de (mout.web.de [217.72.192.78]) (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 C329F1946A3 for ; Wed, 5 Jun 2024 08:38:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.72.192.78 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576729; cv=none; b=RKGgMxeRD1QYT3q1OX7nYV9K7ZAIZ56Y/ah/z4ojK6VFDuzBooYJOxTooo2K0nTJHcXC71Ik2yHPmGnw35Os9gmhpXQTRcVehTTdQvRY+wvaRj7RrkEQEyLBpyAlBpxMO2j+Z85c14kjjXTfVfC0ETeqHIRgGhDU7I6bt+W2DfQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717576729; c=relaxed/simple; bh=uA3WMB75sNcei0BfRApI0sz/EgUJzP8Tj+/7hFL1/Wg=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=kyg2uP5itBeDFdkmO4WfULFT9P+PHrpwy5PzNwfGC5V6NjimojDbXi88sjocNWImp9XlsjkEW8W+N9DeOkilhYHaQXfrO/nyjPSRQQkXWd0fOYOI+3Z8qlJsPpgmcQKOVGeAKiO0cO9X9MiXWmWqsVxbBHO7iHS+D9ZgRE6fLxM= 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=HgG7CZpG; arc=none smtp.client-ip=217.72.192.78 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="HgG7CZpG" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=web.de; s=s29768273; t=1717576717; x=1718181517; i=l.s.r@web.de; bh=6LJnKN4mRnaiH0MHZf/sdWqAZzW0Q/7AEjEqTMPDyUA=; 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=HgG7CZpGrECnL8/8PH3/vsKDepnN1xzNBJw67p89ZTHWYkZr0HDvuUFiCBxs/4+3 ftr+JW6UeU2jf+1V7ogSV93LZ1pvZbJRpCjTV/DKrQfOSptstVBrOQc4B19/4x5B+ gPZNAZhLvj6uNGtyD24SvVnlOoFJvrXQYSifLKF0AnbbhABW8g+IiGiW1ZvJp8yvF xDOyZ4FoXdwiFKnzKLYZNFJP2HIDAtXch34oDOFWXix2/qGxRa0K5ao95KmXvju91 xBOxDAHxwK1zBjiQ1/WqNnOO/jJjT72eArrwHOS2cufc7i/grnCMvCdA/WpqcKYD7 yv2GZF4ujb+F8MXSzw== X-UI-Sender-Class: 814a7b36-bfc1-4dae-8640-3722d8ec6cd6 Received: from [192.168.178.29] ([91.47.153.5]) by smtp.web.de (mrweb106 [213.165.67.124]) with ESMTPSA (Nemesis) id 1MrwsB-1sl4Bs26B3-00nwvs; Wed, 05 Jun 2024 10:38:37 +0200 Message-ID: <99a99e5c-4fe4-413a-9281-363e280716b8@web.de> Date: Wed, 5 Jun 2024 10:38:36 +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 v2 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 References: <82561c70-ec33-41bf-b036-52310ffc1926@web.de> <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> Content-Language: en-US In-Reply-To: <6fa51a62-5dc1-4865-9f79-eaf7d65189ab@web.de> X-Provags-ID: V03:K1:Ce+A1KTLH04F+a/+UruzBFm/0tGMJS7wsGB3R9WeICLcSPASjN5 KfRblfiXVj5ZIa+YcRtnMCqsVGhQqf5pEk3i4WHrc2dF4PiLz7WMBbLUj1zdjDJbPsV/rVi tjttDx1MT2txpqwe7FkRv7CpuDiVwQBvfWMTWgcVmkbGHHiHPxKastlRZPGbEJlCHxezWIo siS9EwEiGCVb+AxpX+Zpw== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:/xzsbpSc/7k=;CGrZ5M9vRt2S1L56om2N7zcdKav Cv2bA6SaSBpWRmvzQISEuByJzEv9l+Kf1EBfgDQDAIPFZ9CU3b4p9hg2MrFRzQ+D0uElNgKpF slcrtcf1Tqy9TG1AteUyVWxlsBrMQdVGZzViZOkfhr3XRd3+0l0x0pmY1+/DEykAoqsRrknVR dmLWFpvTTXR7Tb9Ukb26kbsKtQ+/GIfWgKAYfT4ZNqkiL3v5u1FZBQKH45hcJGBFNI8RcrNZ9 5+pRtqSrAV6u/qcNn/ljjQ+UNkuioUdSPJwJ2Ef11QvRrG//6f5SjxEuXodIz2XLoSZecySif iWKphc2j/nNla1rX06VJce2BXBk8KY6rt9EMzm3hocNA0TOtUISEBvriBg67FtswXh0a9DAYx P/Dx6PakhQpP0Fv2KtocMKqDzxFKbR6ifJNGVGqiklDExmaX40QfObewNBpefx5HOG+h0Y80g R3x64eL+tLsAPe2SJbHh6c+uxt7LxlLoGQIGiFvgkLgv++Xx7pnn7bKO4ybe9aNEIRNNPNxk1 IgGnoSNoMzz6ZDwli4dbrs4AwBCgJ+DZeNu5e9Fieaa6lUr3FSr5VUJDSDkHcSymAX/yLfR3X s6cDBGeZ597N2apIOgI/T6qeRL+W9wwDeo8k0uXIDsqJnkZ4zeTie4AEBs/LGx5v7eFyxQkJ2 xabC8uzlmMUa3GUM0Q18KC9LK4qrEQhEkdhutl+qlygBt+P3cvstzahd5LJ3eBkENAreWonoa LeoxOTcB7sAfEiZcCGv4c+8Y/j6xUiUDxzIjkJpIjIbZ02BCzwLOUYpoOuODpu5QNxyzgkUn3 OYAHNxRgNM6notVKatxCw69K77dKDU697VbJNQJQzkmYE= 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(). 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 | 14 ++++++++++++++ Documentation/git.txt | 7 +++++++ Documentation/gitattributes.txt | 5 +++++ diff.c | 30 +++++++++++++++++++++++++++++- t/t4020-diff-external.sh | 23 +++++++++++++++++------ userdiff.c | 4 ++++ userdiff.h | 1 + 7 files changed, 77 insertions(+), 7 deletions(-) diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt index 5ce7b91f1d..485a13236d 100644 --- a/Documentation/config/diff.txt +++ b/Documentation/config/diff.txt @@ -79,6 +79,13 @@ 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 1 if it finds + significant changes and 0 if it doesn't, like diff(1). If it's + false then the `diff.external` command is expected to always + return exit code 0. Defaults to false. + diff.ignoreSubmodules:: Sets the default value of --ignore-submodules. Note that this affects only 'git diff' Porcelain, and not lower level 'diff' @@ -164,6 +171,13 @@ 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 + 1 if it finds significant changes and 0 if it doesn't, like + diff(1). If it's false then the `diff.external` command is + expected to always return exit code 0. Defaults to false. + 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/git.txt b/Documentation/git.txt index a31a70acca..81b806416c 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -644,6 +644,13 @@ 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`:: + Setting this environment variable indicates the the program + specified with `GIT_EXTERNAL_DIFF` returns exit code 1 if it + finds significant changes and 0 if it doesn't, like diff(1). + If it's not set, the program is expected to always return + exit code 0. + `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 286d093bfa..3dff81ec7d 100644 --- a/diff.c +++ b/diff.c @@ -430,6 +430,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")) @@ -554,6 +558,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) @@ -4385,6 +4391,18 @@ 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 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 (!(o->output_format & DIFF_FORMAT_PATCH) && !pgm->trust_exit_code) { + o->found_changes = 1; + return; + } strvec_push(&cmd.args, pgm->cmd); strvec_push(&cmd.args, name); @@ -4406,7 +4424,10 @@ 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)) + rc = run_command(&cmd); + if ((!pgm->trust_exit_code && !rc) || (pgm->trust_exit_code && rc == 1)) + o->found_changes = 1; + else if (!pgm->trust_exit_code || rc) die(_("external diff died, stopping at %s"), name); remove_tempfile(); @@ -4924,6 +4945,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 bed640b2b1..bbb49b8a0a 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -175,19 +175,22 @@ test_expect_success 'no diff with -diff' ' check_external_exit_code () { expect_code=$1 command_code=$2 - option=$3 + trust_exit_code=$3 + option=$4 command="exit $command_code;" - desc="external diff '$command'" + desc="external diff '$command' with trustExitCode=$trust_exit_code" test_expect_success "$desc via attribute with $option" " 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 $option " test_expect_success "$desc via diff.external with $option" " test_config diff.external \"$command\" && + test_config diff.trustExitCode $trust_exit_code && >.gitattributes && test_expect_code $expect_code git diff $option " @@ -196,14 +199,22 @@ check_external_exit_code () { >.gitattributes && test_expect_code $expect_code env \ GIT_EXTERNAL_DIFF=\"$command\" \ + GIT_EXTERNAL_DIFF_TRUST_EXIT_CODE=$trust_exit_code \ git diff $option " } -check_external_exit_code 1 0 --exit-code -check_external_exit_code 1 0 --quiet -check_external_exit_code 128 1 --exit-code -check_external_exit_code 1 1 --quiet # we don't even call the program +check_external_exit_code 1 0 off --exit-code +check_external_exit_code 1 0 off --quiet +check_external_exit_code 128 1 off --exit-code +check_external_exit_code 1 1 off --quiet # we don't even call the program + +check_external_exit_code 0 0 on --exit-code +check_external_exit_code 0 0 on --quiet +check_external_exit_code 1 1 on --exit-code +check_external_exit_code 1 1 on --quiet +check_external_exit_code 128 2 on --exit-code +check_external_exit_code 128 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 7e2b28e88d..5f1c8cc49c 100644 --- a/userdiff.h +++ b/userdiff.h @@ -13,6 +13,7 @@ struct userdiff_funcname { struct external_diff { const char *cmd; + unsigned trust_exit_code:1; }; struct userdiff_driver {