{"thread":{"id":"66066","subject":"[PATCH 0/2] bump static-analysis ci image version","startedAt":"2026-07-26T08:32:55Z","lastAt":"2026-09-05T13:53:01Z","messageCount":13,"participants":["Jeff King","Junio C Hamano","Patrick Steinhardt","Elijah Newren","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"548988","messageId":"20260726083254.GA3528497@coredump.intra.peff.net","threadId":"66066","inReplyTo":null,"subject":"[PATCH 0/2] bump static-analysis ci image version","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-26T08:32:54Z","receivedAt":"2026-07-26T08:32:55Z","isPatch":true,"body":"This is another way to fix the slow coccinelle run discussed in:\n\n  https://lore.kernel.org/git/20260724091152.27794-2-tnyman@openai.com/\n\nby using a newer version of coccinelle.\n\nWe tweaked the code there to avoid the problem, so this isn't urgent.\nBut it is worth doing to avoid running into the same problem again (and\nbecause in general I think it makes sense to run newer versions of our\ndev tools than older ones).\n\nThe second patch is the interesting one. The first is a necessary\nclean-up (+cc Elijah as the relevant author there).\n\n  [1/2]: bloom: silence CHECK_ASSERTION_SIDE_EFFECTS false positive\n  [2/2]: ci: bump ubuntu image version for static-analysis job\n\n .github/workflows/main.yml | 4 ++--\n .gitlab-ci.yml             | 2 +-\n bloom.c                    | 6 +++---\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\n-Peff\n"},{"id":"548990","messageId":"20260726083727.GA3529069@coredump.intra.peff.net","threadId":"66066","inReplyTo":"20260726083254.GA3528497@coredump.intra.peff.net","subject":"[PATCH 1/2] bloom: silence CHECK_ASSERTION_SIDE_EFFECTS false positive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-26T08:37:27Z","receivedAt":"2026-07-26T08:37:29Z","isPatch":true,"body":"Using gcc 15, compiling with CHECK_ASSERTION_SIDE_EFFECTS=1 causes a\ncomplaint about this line in bloom.c having a side effect:\n\n\tassert(version == 1 || version == 2);\n\nI think this is pretty clearly a false positive, as those comparisons\nshould not have side effects. The side-effect checker uses a magic\ndefinition of assert() that relies on the compiler's optimizer to drop a\nreference to an otherwise unused variable. And for whatever reason, gcc\nchooses not to do so here under -O2 (side note: if you have -O0 in your\nCFLAGS, that naturally creates many more false positives!).\n\nThis code has been around for a while, but nobody seems to have noticed\nbecause we use an older version of the compiler in our static-analysis\nci job, and it does not complain. Presumably very few people run this\ncheck locally on their more modern compilers.\n\nLet's silence the false positive to avoid confusion for anyone running\nlocally, and to make it possible to upgrade the image we use for our\nstatic-analysis job.\n\nWe could just switch to our custom ASSERT() here, but I think we can\nimprove the code by integrating the assertion into the if/else cascade.\nThat avoids repeating the logic about which versions are acceptable.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nBuilding with clang or with \"gcc -flto\" seems to also silence the false\npositive. We might consider using those for the static-analysis job.\nBut since in this instance we can both silence it and (IMHO) make the\ncode nicer to read, I think it's reasonable to do so. We can leave\ntinkering with the assert() magic as a separate topic for anyone\ninterested.\n\n bloom.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/bloom.c b/bloom.c\nindex c98d1672ad..caf22f9831 100644\n--- a/bloom.c\n+++ b/bloom.c\n@@ -610,10 +610,10 @@ int bloom_filter_contains_vec(const struct bloom_filter *filter,\n uint32_t test_bloom_murmur3_seeded(uint32_t seed, const char *data, size_t len,\n \t\t\t\t   int version)\n {\n-\tassert(version == 1 || version == 2);\n-\n \tif (version == 2)\n \t\treturn murmur3_seeded_v2(seed, data, len);\n-\telse\n+\telse if (version == 1)\n \t\treturn murmur3_seeded_v1(seed, data, len);\n+\telse\n+\t\tBUG(\"unexpected bloom version: %d\", version);\n }\n-- \n2.55.0.742.gf2bff09aa6\n\n"},{"id":"548991","messageId":"20260726083905.GB3529069@coredump.intra.peff.net","threadId":"66066","inReplyTo":"20260726083254.GA3528497@coredump.intra.peff.net","subject":"[PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-07-26T08:39:05Z","receivedAt":"2026-07-26T08:39:07Z","isPatch":true,"body":"We recently ran into a case[1] where old versions of coccinelle ran very\nslowly, but newer ones are fine. The version we use in GitHub's CI was\nthe old slow version, leading to timeouts of the static-analysis job.\n\nWe get the old version because we ask for the ubuntu-22.04 image. That\nhas coccinelle 1.1.1, but the \"fast\" improvement is in coccinelle 1.3.0,\nspecifically their 58619b8fe (break up envs for e1 & e2, 2024-08-18).\n\nBumping to ubuntu-25.10 would be enough to get that new version. But I\ndon't see any need to ask for a specific version at all. We originally\nused a specific version because coccinelle wasn't available in ubuntu\n20.04, so we pinned to 18.04 in d051ed77ee (.github/workflows/main.yml:\nrun static-analysis on bionic, 2021-02-08). Later that got bumped in\nef46584831 (ci: update 'static-analysis' to Ubuntu 22.04, 2022-08-23)\nwhen 18.04 support was dropped.\n\nIt seems like the absence of coccinelle was a blip in 20.04, and we can\njust stick with \"latest\" going forward.\n\nI tested the result on GitHub's CI. I bumped the matching line in the\nGitLab definition, but didn't have a simple means of testing (but it's\nsuch a trivial change nothing could go wrong, right?).\n\n[1] https://lore.kernel.org/git/20260724091152.27794-2-tnyman@openai.com/\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n .github/workflows/main.yml | 4 ++--\n .gitlab-ci.yml             | 2 +-\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 85cfedf5b0..205325eb33 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -460,8 +460,8 @@ jobs:\n     if: needs.ci-config.outputs.enabled == 'yes'\n     env:\n       jobname: StaticAnalysis\n-      CI_JOB_IMAGE: ubuntu-22.04\n-    runs-on: ubuntu-22.04\n+      CI_JOB_IMAGE: ubuntu-latest\n+    runs-on: ubuntu-latest\n     concurrency:\n       group: static-analysis-${{ github.ref }}\n       cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex 1c4d04da9d..0242283c3c 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -227,7 +227,7 @@ test:fuzz-smoke-tests:\n     - ./ci/run-build-and-minimal-fuzzers.sh\n \n static-analysis:\n-  image: ubuntu:22.04\n+  image: ubuntu:latest\n   stage: analyze\n   needs: [ ]\n   variables:\n-- \n2.55.0.742.gf2bff09aa6\n"},{"id":"549026","messageId":"xmqqo6ftvhed.fsf@gitster.g","threadId":"66066","inReplyTo":"20260726083254.GA3528497@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] bump static-analysis ci image version","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-26T16:34:50Z","receivedAt":"2026-07-26T16:34:53Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> This is another way to fix the slow coccinelle run discussed in:\n>\n>   https://lore.kernel.org/git/20260724091152.27794-2-tnyman@openai.com/\n>\n> by using a newer version of coccinelle.\n>\n> We tweaked the code there to avoid the problem, so this isn't urgent.\n> But it is worth doing to avoid running into the same problem again (and\n> because in general I think it makes sense to run newer versions of our\n> dev tools than older ones).\n>\n> The second patch is the interesting one. The first is a necessary\n> clean-up (+cc Elijah as the relevant author there).\n\nBoth are interesting.  Thanks for doing this (with relevant\narchaeology as usual).\n"},{"id":"549970","messageId":"anWyV9Q4Cmsa5AoT@pks.im","threadId":"66066","inReplyTo":"20260726083905.GB3529069@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T10:24:23Z","receivedAt":"2026-08-07T10:24:31Z","isPatch":true,"body":"On Sun, Jul 26, 2026 at 04:39:05AM -0400, Jeff King wrote:\n> We recently ran into a case[1] where old versions of coccinelle ran very\n> slowly, but newer ones are fine. The version we use in GitHub's CI was\n> the old slow version, leading to timeouts of the static-analysis job.\n> \n> We get the old version because we ask for the ubuntu-22.04 image. That\n> has coccinelle 1.1.1, but the \"fast\" improvement is in coccinelle 1.3.0,\n> specifically their 58619b8fe (break up envs for e1 & e2, 2024-08-18).\n\nI have been wondering about slow Coccinelle for a while now. Making\nthings faster via a simple version upgrade is great, as it comes almost\nfor free.\n\nBut that being said, we also have a bunch of Coccinelle rules nowadays,\nand my gut feeling tells me that there's a bunch of them that aren't\nuseful anymore. \"refs\", \"object_id\", \"the_repository\",\n\"git_config_number\", \"index-compatibility\" and \"context_fn_ctx\" all look\nlike files that we could probably just get rid of because we have long\ndone the migrations, and it's unlikely anybody still has patches that\nuse the pre-migration variants.\n\nThey'd of course require a bit of a deeper look, but that could be\nanother way to speed up Coccinelle for us. Even though I cannot say for\nsure by how much, I didn't give it a test.\n\nThanks!\n\nPatrick\n"},{"id":"550027","messageId":"xmqq8q6hgb2m.fsf@gitster.g","threadId":"66066","inReplyTo":"anWyV9Q4Cmsa5AoT@pks.im","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-07T16:16:49Z","receivedAt":"2026-08-07T16:16:56Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> They'd of course require a bit of a deeper look, but that could be\n> another way to speed up Coccinelle for us. Even though I cannot say for\n> sure by how much, I didn't give it a test.\n\nAnother benefit is that it would reduce the programmer's burden, as\nit is not immediately apparent which rules are still relevant.\n\nI wonder if we can easily define the exit criteria when we introduce\na new rule and document them, immediately next to the rules.\n\nYou said \"refs, object_id, the_repository, ... all look like we have\nlong done with the migrations\"; in retrospect, would it have been\neasily doable for those who introduced these rules to describe how\nwe would declare \"now migration is done\"?  If so, perhaps a good\nstep forward may be to update tools/coccinelle/README to add such a\nrule.\n\n    ... goes and looks ...\n\nThe readme file clearly states that transformations needed for\nmigrations are *not* regularly run.  Is it possible that we have\nthese rules you mentioned misclassified?\n"},{"id":"550033","messageId":"CABPp-BEURtn+yh_m=DX1dUe5CY5mzpdmzpqOeZdOQ14sKw43FQ@mail.gmail.com","threadId":"66066","inReplyTo":"20260726083905.GB3529069@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-07T16:47:41Z","receivedAt":"2026-08-07T16:47:53Z","isPatch":true,"body":"On Sun, Jul 26, 2026 at 1:39 AM Jeff King <peff@peff.net> wrote:\n>\n> We recently ran into a case[1] where old versions of coccinelle ran very\n> slowly, but newer ones are fine. The version we use in GitHub's CI was\n> the old slow version, leading to timeouts of the static-analysis job.\n>\n> We get the old version because we ask for the ubuntu-22.04 image. That\n> has coccinelle 1.1.1, but the \"fast\" improvement is in coccinelle 1.3.0,\n> specifically their 58619b8fe (break up envs for e1 & e2, 2024-08-18).\n>\n> Bumping to ubuntu-25.10 would be enough to get that new version. But I\n> don't see any need to ask for a specific version at all. We originally\n> used a specific version because coccinelle wasn't available in ubuntu\n> 20.04, so we pinned to 18.04 in d051ed77ee (.github/workflows/main.yml:\n> run static-analysis on bionic, 2021-02-08). Later that got bumped in\n> ef46584831 (ci: update 'static-analysis' to Ubuntu 22.04, 2022-08-23)\n> when 18.04 support was dropped.\n>\n> It seems like the absence of coccinelle was a blip in 20.04, and we can\n> just stick with \"latest\" going forward.\n>\n> I tested the result on GitHub's CI. I bumped the matching line in the\n> GitLab definition, but didn't have a simple means of testing (but it's\n> such a trivial change nothing could go wrong, right?).\n>\n> [1] https://lore.kernel.org/git/20260724091152.27794-2-tnyman@openai.com/\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  .github/workflows/main.yml | 4 ++--\n>  .gitlab-ci.yml             | 2 +-\n>  2 files changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 85cfedf5b0..205325eb33 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -460,8 +460,8 @@ jobs:\n>      if: needs.ci-config.outputs.enabled == 'yes'\n>      env:\n>        jobname: StaticAnalysis\n> -      CI_JOB_IMAGE: ubuntu-22.04\n> -    runs-on: ubuntu-22.04\n> +      CI_JOB_IMAGE: ubuntu-latest\n> +    runs-on: ubuntu-latest\n>      concurrency:\n>        group: static-analysis-${{ github.ref }}\n>        cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}\n> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\n> index 1c4d04da9d..0242283c3c 100644\n> --- a/.gitlab-ci.yml\n> +++ b/.gitlab-ci.yml\n> @@ -227,7 +227,7 @@ test:fuzz-smoke-tests:\n>      - ./ci/run-build-and-minimal-fuzzers.sh\n>\n>  static-analysis:\n> -  image: ubuntu:22.04\n> +  image: ubuntu:latest\n>    stage: analyze\n>    needs: [ ]\n>    variables:\n> --\n> 2.55.0.742.gf2bff09aa6\n\nMakes sense to me; looks good.\n"},{"id":"550103","messageId":"andoDRDn5RvgNHrl@szeder.dev","threadId":"66066","inReplyTo":"20260726083905.GB3529069@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2026-08-08T17:31:57Z","receivedAt":"2026-08-08T17:32:01Z","isPatch":true,"body":"On Sun, Jul 26, 2026 at 04:39:05AM -0400, Jeff King wrote:\n> We recently ran into a case[1] where old versions of coccinelle ran very\n> slowly, but newer ones are fine. The version we use in GitHub's CI was\n> the old slow version, leading to timeouts of the static-analysis job.\n> \n> We get the old version because we ask for the ubuntu-22.04 image. That\n> has coccinelle 1.1.1, but the \"fast\" improvement is in coccinelle 1.3.0,\n> specifically their 58619b8fe (break up envs for e1 & e2, 2024-08-18).\n\nI've built Docker images of various Coccinelle versions [1] years ago,\nand seeing this issue I've updated those images with more recent base\nimage and Coccinelle versions.\n\nUsing these to run 'make coccicheck' on 630cf86933, i.e. 'seen' on or\naround 2026-07-14, which contained a024a5818c (branch: add\n--delete-merged <branch>, 2026-07-14) with those problematic loop\ncounter variables I got the following results:\n\n  - 1.1.1: 1437.78user 56.66system 2:10.29elapsed 1146%CPU (0avgtext+0avgdata 223896maxresident)k\n\n  - 1.2.0: ctrl-c after 2.5h.  The bulk of the work was done in about\n           10 minutes, but processing 'builtin/branch.c' seemed to\n           hang forever.\n\n  - 1.3.1: 6532.81user 106.75system 9:35.04elapsed 1154%CPU (0avgtext+0avgdata 635592maxresident)k\n\nSo my Coccinelle 1.1.1 didn't hang, moreover, it was about 4.5 times\nfaster than 1.3.1.  I got similar runtime differences between 1.1.1\nand 1.3.1 when checking e.g. v2.55.0 or current master; in these cases\n1.2.0 didn't hang, but took about the same time as 1.3.1.\n\nAm I doing something wrong?   Or is everyone else is doing something\nwrong? :)\n\n[1] https://hub.docker.com/r/szeder/coccinelle/tags\n\n\nOn a somewhat related note, for a while now we've been unnecessarily\ninstalling all the dependencies of the \"build and test\" jobs\n(compiler, build systems, apache, p4, jgit, etc.) for the various\nstatic analysis and the 'documentation' CI jobs as well.\n\nI think this is because 707d2f2fe8 (CI: use \"$runs_on_pool\", not\n\"$jobname\" to select packages & config, 2021-11-23) started installing\nall those dependencies for jobs using 'ubuntu-latest', including the\n'documentation' job as well, though this side-effect was not mentioned\nin the commit message.  The 'StaticAnalysis' and 'sparse' jobs were\nnot affected at the time, because they were using a specific Ubuntu\nversion, but then 0178420b9c (github-actions: run gcc-8 on\nubuntu-20.04 image, 2022-11-25) came along and changed the pattern\nmatching $runs_on_pool from 'ubuntu-latest' to 'ubuntu-*'.\n\n\n"},{"id":"550154","messageId":"anlj3kdAfOh8OnNR@pks.im","threadId":"66066","inReplyTo":"xmqq8q6hgb2m.fsf@gitster.g","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-10T05:38:38Z","receivedAt":"2026-08-10T05:38:45Z","isPatch":true,"body":"On Fri, Aug 07, 2026 at 09:16:49AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > They'd of course require a bit of a deeper look, but that could be\n> > another way to speed up Coccinelle for us. Even though I cannot say for\n> > sure by how much, I didn't give it a test.\n> \n> Another benefit is that it would reduce the programmer's burden, as\n> it is not immediately apparent which rules are still relevant.\n> \n> I wonder if we can easily define the exit criteria when we introduce\n> a new rule and document them, immediately next to the rules.\n> \n> You said \"refs, object_id, the_repository, ... all look like we have\n> long done with the migrations\"; in retrospect, would it have been\n> easily doable for those who introduced these rules to describe how\n> we would declare \"now migration is done\"?  If so, perhaps a good\n> step forward may be to update tools/coccinelle/README to add such a\n> rule.\n> \n>     ... goes and looks ...\n> \n> The readme file clearly states that transformations needed for\n> migrations are *not* regularly run.  Is it possible that we have\n> these rules you mentioned misclassified?\n\nFor all I can see, both our Makefile and Meson simply take all\nCoccinelle files we have, concatenate and run those rules against our\nwhole codebase. So I don't see any kind of classification at all?\n\nAh, no, you're right. We have the \".pending\" suffix that we do treat\nspecial. We only have a single one of those with \"config_fn_ctx\".\nArguably, many of the others should've been classified as pending, too.\nBut I think it's quite easy to miss that we even treat these kinds of\nfiles special.\n\nTaking a step back, I do have to wonder whether the Cocci files have\nbeen adding any kind of value in the first place. I myself introduced\nsome of them in contexts where I made sweeping changes to our APIs, so\nthat any in-flight topics can be trivially adjusted via Coccinelle. But\nI very much doubt that anyone ever used those to adapt their in-flight\npatch series at all.\n\nSo maybe we should just not do that anymore?\n\nPatrick\n"},{"id":"550210","messageId":"xmqq7blx7tii.fsf@gitster.g","threadId":"66066","inReplyTo":"anlj3kdAfOh8OnNR@pks.im","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-10T17:52:21Z","receivedAt":"2026-08-10T17:52:24Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Taking a step back, I do have to wonder whether the Cocci files have\n> been adding any kind of value in the first place. I myself introduced\n> some of them in contexts where I made sweeping changes to our APIs, so\n> that any in-flight topics can be trivially adjusted via Coccinelle. But\n> I very much doubt that anyone ever used those to adapt their in-flight\n> patch series at all.\n>\n> So maybe we should just not do that anymore?\n\nWe still do catch when somebody writes \"if (a == NULL)\", no?\n"},{"id":"550238","messageId":"anqs8mT78znJmUwJ@pks.im","threadId":"66066","inReplyTo":"xmqq7blx7tii.fsf@gitster.g","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-11T05:02:42Z","receivedAt":"2026-08-11T05:02:50Z","isPatch":true,"body":"On Mon, Aug 10, 2026 at 10:52:21AM -0700, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Taking a step back, I do have to wonder whether the Cocci files have\n> > been adding any kind of value in the first place. I myself introduced\n> > some of them in contexts where I made sweeping changes to our APIs, so\n> > that any in-flight topics can be trivially adjusted via Coccinelle. But\n> > I very much doubt that anyone ever used those to adapt their in-flight\n> > patch series at all.\n> >\n> > So maybe we should just not do that anymore?\n> \n> We still do catch when somebody writes \"if (a == NULL)\", no?\n\nYes! What I was trying to say is that we maybe shouldn't add Cocci files\nfor temporary migrations anymore, but still keep (and extend) them for\nevertyhing where we want to consistently catch antipatterns going\nforward.\n\nOverall I have a feeling that I'm overthinking this though :) Maybe it\nultimately doesn't matter too much and we just continue what we're doing\nand then clean up every once in a while when too much cruft has\naccumulated.\n\nPatrick\n"},{"id":"552027","messageId":"20260905134424.GA3914039@coredump.intra.peff.net","threadId":"66066","inReplyTo":"anqs8mT78znJmUwJ@pks.im","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-05T13:44:24Z","receivedAt":"2026-09-05T13:51:07Z","isPatch":true,"body":"On Tue, Aug 11, 2026 at 07:02:42AM +0200, Patrick Steinhardt wrote:\n\n> On Mon, Aug 10, 2026 at 10:52:21AM -0700, Junio C Hamano wrote:\n> > Patrick Steinhardt <ps@pks.im> writes:\n> > \n> > > Taking a step back, I do have to wonder whether the Cocci files have\n> > > been adding any kind of value in the first place. I myself introduced\n> > > some of them in contexts where I made sweeping changes to our APIs, so\n> > > that any in-flight topics can be trivially adjusted via Coccinelle. But\n> > > I very much doubt that anyone ever used those to adapt their in-flight\n> > > patch series at all.\n> > >\n> > > So maybe we should just not do that anymore?\n> > \n> > We still do catch when somebody writes \"if (a == NULL)\", no?\n> \n> Yes! What I was trying to say is that we maybe shouldn't add Cocci files\n> for temporary migrations anymore, but still keep (and extend) them for\n> evertyhing where we want to consistently catch antipatterns going\n> forward.\n> \n> Overall I have a feeling that I'm overthinking this though :) Maybe it\n> ultimately doesn't matter too much and we just continue what we're doing\n> and then clean up every once in a while when too much cruft has\n> accumulated.\n\nFWIW, I'd be happy to avoid coccinelle for transitions. The most\nimportant thing is for transitions to be brought to the developer's\nattention at all, so we don't quietly produce broken programs or\ncontinue adding callers of interfaces we're trying to get rid of.\n\nBut bringing attention is often done trivially via the compiler (e.g.,\nchanging names or interfaces). Coccinelle can further suggest the actual\nfix, but most of the time that fix is either obvious, or easily\nexplained in the commit message (and I feel like if any project can do\nso, we should be able to assume people can use pickaxe/blame to find the\nsource of a change).\n\nSo coccinelle can save some work in these cases, but I think it is a net\nloss overall compared to both the effort in writing the semantic\npatches, as well as the operational headaches.\n\nI do think there's still enough value in the enforcement of rules that\ncan't easily be caught by the compiler. Style bits like \"a == NULL\" are\nan obvious example, but I think we have some \"we offer functions X and\nY, but you should usually use X unless you have a good reason\". Though\nmaybe even some of those can be simplified (stuff like oidclr() should\nbe preferred over hashclr(), but maybe we are at a point where hashclr()\ncan become a private function?).\n\n-Peff\n"},{"id":"552028","messageId":"20260905135259.GB3914039@coredump.intra.peff.net","threadId":"66066","inReplyTo":"andoDRDn5RvgNHrl@szeder.dev","subject":"Re: [PATCH 2/2] ci: bump ubuntu image version for static-analysis job","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-09-05T13:52:59Z","receivedAt":"2026-09-05T13:53:01Z","isPatch":true,"body":"On Sat, Aug 08, 2026 at 07:31:57PM +0200, SZEDER Gábor wrote:\n\n> Using these to run 'make coccicheck' on 630cf86933, i.e. 'seen' on or\n> around 2026-07-14, which contained a024a5818c (branch: add\n> --delete-merged <branch>, 2026-07-14) with those problematic loop\n> counter variables I got the following results:\n> \n>   - 1.1.1: 1437.78user 56.66system 2:10.29elapsed 1146%CPU (0avgtext+0avgdata 223896maxresident)k\n> \n>   - 1.2.0: ctrl-c after 2.5h.  The bulk of the work was done in about\n>            10 minutes, but processing 'builtin/branch.c' seemed to\n>            hang forever.\n> \n>   - 1.3.1: 6532.81user 106.75system 9:35.04elapsed 1154%CPU (0avgtext+0avgdata 635592maxresident)k\n> \n> So my Coccinelle 1.1.1 didn't hang, moreover, it was about 4.5 times\n> faster than 1.3.1.  I got similar runtime differences between 1.1.1\n> and 1.3.1 when checking e.g. v2.55.0 or current master; in these cases\n> 1.2.0 didn't hang, but took about the same time as 1.3.1.\n> \n> Am I doing something wrong?   Or is everyone else is doing something\n> wrong? :)\n\nI'd meant to circle back to this and get an answer, but ultimately...I\ndon't have one. I was easily able to reproduce the forever-hang behavior\nbuilding locally, and even bisected it. However IIRC I couldn't get\n1.1.1 to build at all, so my bisect started a bit forward of that.\n\nSo I'm a little curious why we get different results, but not enough to\nsink a bunch more time into building and timing coccinelle myself.\n\nUltimately I think we'll end up on newer versions in the long run as old\nversions eventually become unavailable / uncompilable on newer\nplatforms. So given mixed signals about timing, I think I'd still prefer\nmoving forward in time as a general tie-breaker.\n\n> On a somewhat related note, for a while now we've been unnecessarily\n> installing all the dependencies of the \"build and test\" jobs\n> (compiler, build systems, apache, p4, jgit, etc.) for the various\n> static analysis and the 'documentation' CI jobs as well.\n> \n> I think this is because 707d2f2fe8 (CI: use \"$runs_on_pool\", not\n> \"$jobname\" to select packages & config, 2021-11-23) started installing\n> all those dependencies for jobs using 'ubuntu-latest', including the\n> 'documentation' job as well, though this side-effect was not mentioned\n> in the commit message.  The 'StaticAnalysis' and 'sparse' jobs were\n> not affected at the time, because they were using a specific Ubuntu\n> version, but then 0178420b9c (github-actions: run gcc-8 on\n> ubuntu-20.04 image, 2022-11-25) came along and changed the pattern\n> matching $runs_on_pool from 'ubuntu-latest' to 'ubuntu-*'.\n\nIt has always felt a little nuts to me that all of these CI jobs start\nwith a vanilla base image and then \"apt install\" a bunch of packages.\nSurely there is some mechanism for caching that intermediate state as an\nimage, at which point it is \"free\" to use it as the base for all of the\njobs, whether they need all of it or not (modulo some extra bytes in the\nimage, but to me that is way cheaper than the run-time cost of\ndownloading and installing packages).\n\nI know Docker has some support for automatically caching intermediate\nimage states, but I don't think any of that applies here. From its\nperspective, the all of our ci scripts are running and mutating the\ncontainer.\n\n-Peff\n"}]}