{"thread":{"id":"63672","subject":"[RFC PATCH] coccicheck: fail \"make\" when it fails","startedAt":"2025-06-19T15:35:38Z","lastAt":"2025-06-23T16:14:14Z","messageCount":6,"participants":["Junio C Hamano","Jeff King","Carlo Marcelo Arenas Belón","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"520412","messageId":"xmqqwm97bv7c.fsf@gitster.g","threadId":"63672","inReplyTo":null,"subject":"[RFC PATCH] coccicheck: fail \"make\" when it fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-19T15:35:35Z","receivedAt":"2025-06-19T15:35:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With \"make coccicheck\", we generate contrib/coccinelle/*.cocci.patch\nfiles that contain changes suggested by semantic patches, but \"make\"\nsucceeds. Admittedly, not many developers may run \"make coccicheck\"\nin the first place, but it makes it harder to notice when they do\nrun it after they introduced an iffy piece of code.\n\nCheck that the resulting cocci.patch files are all empty.\n\nSigned-off-by: Junio C Hamano \n---\n Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git c/Makefile w/Makefile\nindex 70d1543b6b..2c17868a24 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -3475,8 +3475,10 @@ coccicheck-test: $(COCCI_TEST_RES_GEN)\n coccicheck: coccicheck-test\n ifdef SPATCH_CONCAT_COCCI\n coccicheck: contrib/coccinelle/ALL.cocci.patch\n+\t! test -s contrib/coccinelle/ALL.cocci.patch\n else\n coccicheck: $(COCCICHECK_PATCHES_INTREE)\n+\ttest $$(cat $(COCCICHECK_PATCHES_INTREE) | wc -c) = 0\n endif\n \n # See contrib/coccinelle/README\n"},{"id":"520529","messageId":"20250621051651.GB3002138@coredump.intra.peff.net","threadId":"63672","inReplyTo":"xmqqwm97bv7c.fsf@gitster.g","subject":"Re: [RFC PATCH] coccicheck: fail \"make\" when it fails","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-06-21T05:16:51Z","receivedAt":"2025-06-21T05:16:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 19, 2025 at 08:35:35AM -0700, Junio C Hamano wrote:\n\n> With \"make coccicheck\", we generate contrib/coccinelle/*.cocci.patch\n> files that contain changes suggested by semantic patches, but \"make\"\n> succeeds. Admittedly, not many developers may run \"make coccicheck\"\n> in the first place, but it makes it harder to notice when they do\n> run it after they introduced an iffy piece of code.\n\nI admit that I do not run coccicheck very often. But I have definitely\nbeen surprised by it not triggering a make failure when there are\nchanges to be made. So I'd be in favor of this patch.\n\n-Peff\n"},{"id":"520530","messageId":"jyof56xql2c23gyckxzmry4smkt2uk64lyofqxk34olzn7evgf@ltq5tiqo4qyg","threadId":"63672","inReplyTo":"xmqqwm97bv7c.fsf@gitster.g","subject":"Re: [RFC PATCH] coccicheck: fail \"make\" when it fails","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2025-06-21T06:29:34Z","receivedAt":"2025-06-21T06:29:36Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Thu, Jun 19, 2025 at 08:35:35AM -0800, Junio C Hamano wrote:\n> \n> diff --git c/Makefile w/Makefile\n> index 70d1543b6b..2c17868a24 100644\n> --- c/Makefile\n> +++ w/Makefile\n> @@ -3475,8 +3475,10 @@ coccicheck-test: $(COCCI_TEST_RES_GEN)\n> coccicheck: coccicheck-test\n> ifdef SPATCH_CONCAT_COCCI\n> coccicheck: contrib/coccinelle/ALL.cocci.patch\n> +\t! test -s contrib/coccinelle/ALL.cocci.patch\n> else\n> coccicheck: $(COCCICHECK_PATCHES_INTREE)\n> +\ttest $$(cat $(COCCICHECK_PATCHES_INTREE) | wc -c) = 0\n\nDefinitely a nitpick, but I think using `-eq` instead of `=`\nand might even come handy from a portability point of view.\n\nJust like Peff, agree would be a big improvement to have\nthis changed for usability.\n\nCarlo\n"},{"id":"520531","messageId":"CAPig+cRtSzjA=P_-WhRw9rRmTmFMGibKuHYn_ZuN935fcffveg@mail.gmail.com","threadId":"63672","inReplyTo":"jyof56xql2c23gyckxzmry4smkt2uk64lyofqxk34olzn7evgf@ltq5tiqo4qyg","subject":"Re: [RFC PATCH] coccicheck: fail \"make\" when it fails","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-21T08:09:40Z","receivedAt":"2025-06-21T08:09:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 21, 2025 at 2:29 AM Carlo Marcelo Arenas Belón\n wrote:\n> On Thu, Jun 19, 2025 at 08:35:35AM -0800, Junio C Hamano wrote:\n> > coccicheck: $(COCCICHECK_PATCHES_INTREE)\n> > + test $$(cat $(COCCICHECK_PATCHES_INTREE) | wc -c) = 0\n>\n> Definitely a nitpick, but I think using `-eq` instead of `=`\n> and might even come handy from a portability point of view.\n\nJunio has previously expressed[*] a preference for `=` over `-eq` in\nthis sort of situation.\n\n[*]: https://lore.kernel.org/git/7v8v3m50ds.fsf@alter.siamese.dyndns.org/\n"},{"id":"520539","messageId":"xmqqa5608l3y.fsf@gitster.g","threadId":"63672","inReplyTo":"CAPig+cRtSzjA=P_-WhRw9rRmTmFMGibKuHYn_ZuN935fcffveg@mail.gmail.com","subject":"Re: [RFC PATCH] coccicheck: fail \"make\" when it fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-22T04:18:41Z","receivedAt":"2025-06-22T04:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Sat, Jun 21, 2025 at 2:29 AM Carlo Marcelo Arenas Belón\n> wrote:\n>> On Thu, Jun 19, 2025 at 08:35:35AM -0800, Junio C Hamano wrote:\n>> > coccicheck: $(COCCICHECK_PATCHES_INTREE)\n>> > + test $$(cat $(COCCICHECK_PATCHES_INTREE) | wc -c) = 0\n>>\n>> Definitely a nitpick, but I think using `-eq` instead of `=`\n>> and might even come handy from a portability point of view.\n>\n> Junio has previously expressed[*] a preference for `=` over `-eq` in\n> this sort of situation.\n>\n> [*]: https://lore.kernel.org/git/7v8v3m50ds.fsf@alter.siamese.dyndns.org/\n\nHeh, you have an infinite more memory than I have ;-) \n\nThat one is 12 years ago or so, isn't it? Not that I changed my\nstance.\n\nHaving said that, I suspect\n\n ! grep -q ^ $(COCCICHECK_PATCHES_INTREE)\n\nmight have been easier to understand. We do not want any file with\neven a single line in it, and \"grep -q\" knows to short-circuit\n(i.e. if the first file in the COCCICHECK_PATCHES_INTREE list is not\nempty, it does not even bother to open the rest of the files).\n\n"},{"id":"520572","messageId":"xmqqbjqe77vw.fsf@gitster.g","threadId":"63672","inReplyTo":"xmqqwm97bv7c.fsf@gitster.g","subject":"[PATCH v2] coccicheck: fail \"make\" when it fails","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-23T16:14:11Z","receivedAt":"2025-06-23T16:14:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With \"make coccicheck\", we generate contrib/coccinelle/*.cocci.patch\nfiles that contain changes suggested by semantic patches, but \"make\"\nsucceeds. Admittedly, not many developers may run \"make coccicheck\"\nin the first place, but it makes it harder to notice when they do\nrun it after they introduced an iffy piece of code.\n\nCheck that the resulting cocci.patch files are all empty.\n\nSigned-off-by: Junio C Hamano \n---\n Makefile | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 97e8385b66..8f1e9424a7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3422,11 +3422,14 @@ endif\n coccicheck-test: $(COCCI_TEST_RES_GEN)\n \n coccicheck: coccicheck-test\n+\n ifdef SPATCH_CONCAT_COCCI\n-coccicheck: contrib/coccinelle/ALL.cocci.patch\n+COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = contrib/coccinelle/ALL.cocci.patch\n else\n-coccicheck: $(COCCICHECK_PATCHES_INTREE)\n+COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = $(COCCICHECK_PATCHES_INTREE)\n endif\n+coccicheck: $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES)\n+\t! grep -q ^ $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) /dev/null\n \n # See contrib/coccinelle/README\n coccicheck-pending: coccicheck-test\n\nRange-diff against v1:\n1: 163a63f2cf ! 1: dee873e931 coccicheck: fail \"make\" when it fails\n @@ Commit message\n Signed-off-by: Junio C Hamano \n \n ## Makefile ##\n -@@ Makefile: coccicheck-test: $(COCCI_TEST_RES_GEN)\n +@@ Makefile: endif\n + coccicheck-test: $(COCCI_TEST_RES_GEN)\n + \n coccicheck: coccicheck-test\n ++\n ifdef SPATCH_CONCAT_COCCI\n - coccicheck: contrib/coccinelle/ALL.cocci.patch\n -+\t! test -s contrib/coccinelle/ALL.cocci.patch\n +-coccicheck: contrib/coccinelle/ALL.cocci.patch\n ++COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = contrib/coccinelle/ALL.cocci.patch\n else\n - coccicheck: $(COCCICHECK_PATCHES_INTREE)\n -+\ttest $$(cat $(COCCICHECK_PATCHES_INTREE) | wc -c) = 0\n +-coccicheck: $(COCCICHECK_PATCHES_INTREE)\n ++COCCICHECK_PATCH_MUST_BE_EMPTY_FILES = $(COCCICHECK_PATCHES_INTREE)\n endif\n ++coccicheck: $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES)\n ++\t! grep -q ^ $(COCCICHECK_PATCH_MUST_BE_EMPTY_FILES) /dev/null\n \n # See contrib/coccinelle/README\n + coccicheck-pending: coccicheck-test\n-- \n2.50.0-243-g67b26bc0ed\n\n"}]}