{"thread":{"id":"58741","subject":"[PATCH] ci: avoid unnecessary builds","startedAt":"2022-11-03T13:34:25Z","lastAt":"2022-11-10T02:40:34Z","messageCount":20,"participants":["Johannes Schindelin via GitGitGadget","Ævar Arnfjörð Bjarmason","Taylor Blau","Jeff King","Derrick Stolee","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"466401","messageId":"pull.1404.git.1667482458622.gitgitgadget@gmail.com","threadId":"58741","inReplyTo":null,"subject":"[PATCH] ci: avoid unnecessary builds","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-11-03T13:34:18Z","receivedAt":"2022-11-03T13:34:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhenever a branch is pushed to a repository which has GitHub Actions\nenabled, a bunch of new workflow runs are started.\n\nWe sometimes see contributors push multiple branch updates in rapid\nsuccession, which in conjunction with the impressive time swallowed by\neven just a single CI build frequently leads to many queued-up runs.\n\nThis is particularly problematic in the case of Pull Requests where a\nsingle contributor can easily (inadvertently) prevent timely builds for\nother contributors.\n\nTo help with this situation, let's use the `concurrency` feature of\nGitHub workflows, essentially canceling GitHub workflow runs that are\nobsoleted by more recent runs:\nhttps://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci: avoid unnecessary builds\n    \n    Just something I noticed recently and only got around to implement\n    today.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1404%2Fdscho%2Favoid-unnecessary-ci-builds-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1404/dscho/avoid-unnecessary-ci-builds-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1404\n\n .github/workflows/check-whitespace.yml | 5 +++++\n .github/workflows/l10n.yml             | 5 +++++\n .github/workflows/main.yml             | 5 +++++\n 3 files changed, 15 insertions(+)\n\ndiff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml\nindex ad3466ad16e..8a4c4bfbb93 100644\n--- a/.github/workflows/check-whitespace.yml\n+++ b/.github/workflows/check-whitespace.yml\n@@ -9,6 +9,11 @@ on:\n   pull_request:\n     types: [opened, synchronize]\n \n+# Avoid unnecessary builds\n+concurrency:\n+  group: ${{ github.workflow }}-${{ github.ref }}\n+  cancel-in-progress: true\n+\n jobs:\n   check-whitespace:\n     runs-on: ubuntu-latest\ndiff --git a/.github/workflows/l10n.yml b/.github/workflows/l10n.yml\nindex 27f72f0ff34..77d9a416289 100644\n--- a/.github/workflows/l10n.yml\n+++ b/.github/workflows/l10n.yml\n@@ -2,6 +2,11 @@ name: git-l10n\n \n on: [push, pull_request_target]\n \n+# Avoid unnecessary builds\n+concurrency:\n+  group: ${{ github.workflow }}-${{ github.ref }}\n+  cancel-in-progress: true\n+\n jobs:\n   git-po-helper:\n     if: >-\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 831f4df56c5..cf47f0ccfed 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -2,6 +2,11 @@ name: CI\n \n on: [push, pull_request]\n \n+# Avoid unnecessary builds\n+concurrency:\n+  group: ${{ github.workflow }}-${{ github.ref }}\n+  cancel-in-progress: true\n+\n env:\n   DEVELOPER: 1\n \n\nbase-commit: e7e5c6f715b2de7bea0d39c7d2ba887335b40aa0\n-- \ngitgitgadget\n"},{"id":"466475","messageId":"221104.86bkpnzdqi.gmgdl@evledraar.gmail.com","threadId":"58741","inReplyTo":"pull.1404.git.1667482458622.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-04T01:46:23Z","receivedAt":"2022-11-04T01:54:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 03 2022, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> Whenever a branch is pushed to a repository which has GitHub Actions\n> enabled, a bunch of new workflow runs are started.\n>\n> We sometimes see contributors push multiple branch updates in rapid\n> succession, which in conjunction with the impressive time swallowed by\n> even just a single CI build frequently leads to many queued-up runs.\n>\n> This is particularly problematic in the case of Pull Requests where a\n> single contributor can easily (inadvertently) prevent timely builds for\n> other contributors.\n\nThe \"timely\" being an issue in git/git and/or gitgitgadget where CI time\nis a shared resource, but not in a <someuser>/git running CI just for\n<someuser>?\n\n> To help with this situation, let's use the `concurrency` feature of\n> GitHub workflows, essentially canceling GitHub workflow runs that are\n> obsoleted by more recent runs:\n> https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency\n\nIn my own fork I very much use this concurrency not-cancel-in-progress\nintentionally.\n\nI.e. I'll run local tests, but also occasionally push to CI\n(particularly when I know I have bad local coverage).\n\nBut that's very much an async process, sometimes I'll look at the\n(hopefully finished by then) CI in the morning.\n\nE.g. now I have a re-pushed branch where the last tip it was at is still\nrunning CI, but it's waiting just the ASAN job.\n\nHaving that breadcrumb trail of \"green\" CI is very helpful, i.e. push\nevery hour or so with something WIP, and be able to eventually see when\nthings went wrong, if they went wrong.\n\nWe have CI config for other such stuff, but I think it's probably hard\nto use in this case, as this involves cancelling the *other* job, not\ncancelling ourselves. So by the time we're past the config stage is when\nwe might want to cancel.\n\nBut doesn't this support the \"if\" syntax to narrow this down to the\nrelevant repos where this would help, while leaving those forks where it\nisn't an issue to enact their own policy?\n\n"},{"id":"466476","messageId":"Y2R0YrQzKaUZzaPB@nand.local","threadId":"58741","inReplyTo":"pull.1404.git.1667482458622.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-04T02:09:38Z","receivedAt":"2022-11-04T02:10:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Nov 03, 2022 at 01:34:18PM +0000, Johannes Schindelin via GitGitGadget wrote:\n>  .github/workflows/check-whitespace.yml | 5 +++++\n>  .github/workflows/l10n.yml             | 5 +++++\n>  .github/workflows/main.yml             | 5 +++++\n>  3 files changed, 15 insertions(+)\n\nVery nice. For my own workflow, I'll often do something like:\n\n  - work on a topic\n  - decide that the topic is ready for submission, so push it to GitHub\n  - while running `git rebase master -x 'make -j40 DEVELOPER=1 test'`, I\n    notice that an earlier patch is broken, so fix it\n  - push the (updated) topic out\n\n...and before preparing the rest of the series for submission, I'll\ncancel any in-progress runs, the results of which I no longer care\nabout.\n\nIt sounds like this does exactly that automatically, making older runs\nobsolete by newer ones.\n\nThanks,\nTaylor\n"},{"id":"466477","messageId":"Y2R3vJf1A2KOZwA7@nand.local","threadId":"58741","inReplyTo":"221104.86bkpnzdqi.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-04T02:23:56Z","receivedAt":"2022-11-04T02:24:04Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Fri, Nov 04, 2022 at 02:46:23AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > This is particularly problematic in the case of Pull Requests where a\n> > single contributor can easily (inadvertently) prevent timely builds for\n> > other contributors.\n>\n> The \"timely\" being an issue in git/git and/or gitgitgadget where CI time\n> is a shared resource, but not in a <someuser>/git running CI just for\n> <someuser>?\n\nYup, agreed.\n\n> > To help with this situation, let's use the `concurrency` feature of\n> > GitHub workflows, essentially canceling GitHub workflow runs that are\n> > obsoleted by more recent runs:\n> > https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency\n>\n> In my own fork I very much use this concurrency not-cancel-in-progress\n> intentionally.\n\nInteresting. I noted basically the opposite in my earlier reply[1] to\nJohannes, where the behavior I want is that newer pushes of the same\ntopic supersede older ones that are currently in progress.\n\nBut I think you make a compelling point (which doesn't match my own\nworkflow, but I can see the utility of nonetheless).\n\nI was thinking that we could rely on something similar to the ci-config\nref stuff from back in e76eec35540 (ci: allow per-branch config for\nGitHub Actions, 2020-05-07), but it looks like it'll be a little\ntrickier than that, maybe impossible.\n\nWe need to know about the state of the ci-config branch before we set\nthe concurrency bits. So I think you *could* do something like:\n\n--- >8 ---\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 4fdf4d3552..f1ca364f96 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -2,11 +2,6 @@ name: CI\n\n on: [push, pull_request]\n\n-# Avoid unnecessary builds\n-concurrency:\n-  group: ${{ github.workflow }}-${{ github.ref }}\n-  cancel-in-progress: true\n-\n env:\n   DEVELOPER: 1\n\n@@ -39,7 +34,14 @@ jobs:\n           then\n             enabled=no\n           fi\n+          skip_concurrent=yes\n+          if test -x config-repo/ci/config/skip-concurrent &&\n+             ! config-repo/ci/config/skip-concurrent '${{ github.ref }}'\n+          then\n+            skip_concurrent=no\n+          fi\n           echo \"::set-output name=enabled::$enabled\"\n+          echo \"::set-output name=skip_concurrent::$skip_concurrent\"\n       - name: skip if the commit or tree was already tested\n         id: skip-if-redundant\n         uses: actions/github-script@v3\n@@ -86,6 +88,9 @@ jobs:\n     name: win build\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n+    concurrency:\n+      group: ${{ github.workflow }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent = 'yes'\n     runs-on: windows-latest\n     steps:\n     - uses: actions/checkout@v2\n--- 8< ---\n\n...and similar \"concurrency\" blocks in each of the other jobs to define\nthe settings at the job level instead of at the workflow level.\n\nSo, it's doable, but a little gross. At the very least, it would satisfy\nÆvar's workflow requirements, too, since he could write a script that\nexits with non-zero status to avoid the new behavior.\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/Y2R0YrQzKaUZzaPB@nand.local/\n"},{"id":"466482","messageId":"Y2SFGmQnx7CXtTEI@coredump.intra.peff.net","threadId":"58741","inReplyTo":"Y2R3vJf1A2KOZwA7@nand.local","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-11-04T03:20:58Z","receivedAt":"2022-11-04T03:25:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 03, 2022 at 10:23:56PM -0400, Taylor Blau wrote:\n\n> But I think you make a compelling point (which doesn't match my own\n> workflow, but I can see the utility of nonetheless).\n> \n> I was thinking that we could rely on something similar to the ci-config\n> ref stuff from back in e76eec35540 (ci: allow per-branch config for\n> GitHub Actions, 2020-05-07), but it looks like it'll be a little\n> trickier than that, maybe impossible.\n> \n> We need to know about the state of the ci-config branch before we set\n> the concurrency bits. So I think you *could* do something like:\n\nAs an aside, I wish there was a way to interpret per-repo environment\nvariables in the actual action config. The current ci-config stuff\nworks, but it's pretty horrible because (if I understand correctly) it\nspins up a VM just to evaluate a glob and say \"nope, no CI needed on\nthis branch\". So:\n\n  1. It's wasteful of resources, compared to a system where the Actions\n     parser can evaluate a variable.\n\n  2. It makes the Actions results page for a repo ugly, because skipped\n     branches clutter the output with \"yes, I passed CI\" even though all\n     they passed was a trivial job to say \"don't bother running more\n     CI\".\n\n  3. The problem you mention: it happens too late to affect the overall\n     Actions flow, and instead individual jobs have to take it into\n     account.\n\nWhen I wrote the original ci-config stuff I looked for an alternative.\nYou _can_ set per-repo variables in the form of secrets, but I couldn't\nfind a way to evaluate them at the top-level of the yaml file. But maybe\nthat has improved in the meantime. I had looked against as recently as a\nmonth or two ago and didn't find anything, but I'm far from an expert on\nActions.\n\n-Peff\n"},{"id":"466724","messageId":"f975f57e-71e2-3227-8039-14dff82f04db@github.com","threadId":"58741","inReplyTo":"pull.1404.git.1667482458622.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-07T19:45:24Z","receivedAt":"2022-11-07T19:45:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> Whenever a branch is pushed to a repository which has GitHub Actions\n> enabled, a bunch of new workflow runs are started.\n> \n> We sometimes see contributors push multiple branch updates in rapid\n> succession, which in conjunction with the impressive time swallowed by\n> even just a single CI build frequently leads to many queued-up runs.\n> \n> This is particularly problematic in the case of Pull Requests where a\n> single contributor can easily (inadvertently) prevent timely builds for\n> other contributors.\n\nAs someone who is both the cause and the victim of this, I\nthank you for finding a way to reduce wasted CPU time. This\npatch looks good to me, though I'll need to trust the docs\nand your testing to be sure it will work. We will definitely\nsee it in place as it merges into 'next' and 'main'.\n\nThanks,\n-Stolee\n"},{"id":"466727","messageId":"Y2liOzOFLyz4uzd0@nand.local","threadId":"58741","inReplyTo":"f975f57e-71e2-3227-8039-14dff82f04db@github.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-07T19:53:31Z","receivedAt":"2022-11-07T19:53:36Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Nov 07, 2022 at 02:45:24PM -0500, Derrick Stolee wrote:\n> On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > Whenever a branch is pushed to a repository which has GitHub Actions\n> > enabled, a bunch of new workflow runs are started.\n> >\n> > We sometimes see contributors push multiple branch updates in rapid\n> > succession, which in conjunction with the impressive time swallowed by\n> > even just a single CI build frequently leads to many queued-up runs.\n> >\n> > This is particularly problematic in the case of Pull Requests where a\n> > single contributor can easily (inadvertently) prevent timely builds for\n> > other contributors.\n>\n> As someone who is both the cause and the victim of this, I\n> thank you for finding a way to reduce wasted CPU time. This\n> patch looks good to me, though I'll need to trust the docs\n> and your testing to be sure it will work. We will definitely\n> see it in place as it merges into 'next' and 'main'.\n\nI wonder how we should treat Ævar's concerns in this thread. I suspect\nthat the vast majority of workflows wouldn't be affected, but I don't\nwant to completely break Ævar's workflow, either ;-).\n\nSome kind of configuration mechanism like I proposed might be nice.\nThoughts?\n\nThanks,\nTaylor\n"},{"id":"466731","messageId":"85b30a45-e00e-44b5-ae4a-b09542a9fb19@github.com","threadId":"58741","inReplyTo":"Y2liOzOFLyz4uzd0@nand.local","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-07T20:08:46Z","receivedAt":"2022-11-07T20:08:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/7/22 2:53 PM, Taylor Blau wrote:\n> On Mon, Nov 07, 2022 at 02:45:24PM -0500, Derrick Stolee wrote:\n>> On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>>\n>>> Whenever a branch is pushed to a repository which has GitHub Actions\n>>> enabled, a bunch of new workflow runs are started.\n>>>\n>>> We sometimes see contributors push multiple branch updates in rapid\n>>> succession, which in conjunction with the impressive time swallowed by\n>>> even just a single CI build frequently leads to many queued-up runs.\n>>>\n>>> This is particularly problematic in the case of Pull Requests where a\n>>> single contributor can easily (inadvertently) prevent timely builds for\n>>> other contributors.\n>>\n>> As someone who is both the cause and the victim of this, I\n>> thank you for finding a way to reduce wasted CPU time. This\n>> patch looks good to me, though I'll need to trust the docs\n>> and your testing to be sure it will work. We will definitely\n>> see it in place as it merges into 'next' and 'main'.\n> \n> I wonder how we should treat Ævar's concerns in this thread. I suspect\n> that the vast majority of workflows wouldn't be affected, but I don't\n> want to completely break Ævar's workflow, either ;-).\n> \n> Some kind of configuration mechanism like I proposed might be nice.\n> Thoughts?\n\nTaking a look at that sub-thread, I have two thoughts:\n\n1. I don't think supporting a \"multiple pushes of WIP work\"\n   scenario is a good use of \"free\" resources. If you want to\n   test multiple versions of something, then use multiple\n   branches (and I think Johannes's patch allows concurrent\n   builds for distinct branch names).\n\n2. The change you recommend requires running the job and\n   deciding at runtime whether to do the actual build\n   (unless I'm mistaken). It is better to let the workflow\n   coordinator decide on concurrency before the stage where\n   worker VMs are engaged.\n\nEither of these points may have an incorrect assumption, so\nI'm prepared to be wrong.\n\nThanks,\n-Stolee\n"},{"id":"466739","messageId":"221107.865yfqv58k.gmgdl@evledraar.gmail.com","threadId":"58741","inReplyTo":"85b30a45-e00e-44b5-ae4a-b09542a9fb19@github.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-07T21:03:23Z","receivedAt":"2022-11-07T21:16:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 07 2022, Derrick Stolee wrote:\n\n> On 11/7/22 2:53 PM, Taylor Blau wrote:\n>> On Mon, Nov 07, 2022 at 02:45:24PM -0500, Derrick Stolee wrote:\n>>> On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n>>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>>>\n>>>> Whenever a branch is pushed to a repository which has GitHub Actions\n>>>> enabled, a bunch of new workflow runs are started.\n>>>>\n>>>> We sometimes see contributors push multiple branch updates in rapid\n>>>> succession, which in conjunction with the impressive time swallowed by\n>>>> even just a single CI build frequently leads to many queued-up runs.\n>>>>\n>>>> This is particularly problematic in the case of Pull Requests where a\n>>>> single contributor can easily (inadvertently) prevent timely builds for\n>>>> other contributors.\n>>>\n>>> As someone who is both the cause and the victim of this, I\n>>> thank you for finding a way to reduce wasted CPU time. This\n>>> patch looks good to me, though I'll need to trust the docs\n>>> and your testing to be sure it will work. We will definitely\n>>> see it in place as it merges into 'next' and 'main'.\n>> \n>> I wonder how we should treat Ævar's concerns in this thread. I suspect\n>> that the vast majority of workflows wouldn't be affected, but I don't\n>> want to completely break Ævar's workflow, either ;-).\n>> \n>> Some kind of configuration mechanism like I proposed might be nice.\n>> Thoughts?\n>\n> Taking a look at that sub-thread, I have two thoughts:\n>\n> 1. I don't think supporting a \"multiple pushes of WIP work\"\n>    scenario is a good use of \"free\" resources. If you want to\n>    test multiple versions of something, then use multiple\n>    branches (and I think Johannes's patch allows concurrent\n>    builds for distinct branch names).\n\nThe setting Taylor proposed in\nhttps://lore.kernel.org/git/Y2R3vJf1A2KOZwA7@nand.local/ is off by\ndefault, i.e. it would behave the same way as what Johannes is\nproposing, just give you (well, me) an opt-out from the default, without\npatching main.yml on every branch.\n\nSo it seems like a win-win, why force others to change their workflow?\nSure, I could push multiple branches, but you could also manually cancel\nyour outstanding jobs before re-pushing...\n\nI agree that cancelling the outstanding job is a better default, and if\nwe had to pick one or the other I'd say \"sure\", but if we can have\nboth...\n\n> 2. The change you recommend requires running the job and\n>    deciding at runtime whether to do the actual build\n>    (unless I'm mistaken). It is better to let the workflow\n>    coordinator decide on concurrency before the stage where\n>    worker VMs are engaged.\n\nDoesn't the \"concurrency\" setting say \"I am a job thaht's prepared to\nhave its run cancelled if another job shows up with this <key>\".\n\nI.e. we'd already be running the VM by definition if we're going to\nbenefit from it (very tight races aside), and in any case getting to the\n\"configure\" stage doesn't require much in the way of resources, a few\nseconds of VM time...\n\nWhereas the \"this tree already ran\" *would* benefit from not starting a\nVM, but that's \"I'm a new push, and maybe I shouldn't run at all\",\nwhereas this feature is \"I always want to run, but someone might cancel\nme later\".\n\n> Either of these points may have an incorrect assumption, so\n> I'm prepared to be wrong.\n\nI *think* you're wrong about #2, but I'm not sure either.\n\nI don't think you can be wrong about #1, \"others should change their\nworkflow to fit a new worldview\" is more of a value-judgment :)\n"},{"id":"466760","messageId":"114d4a72-1a75-71f3-8af6-6e82cd4fd54b@github.com","threadId":"58741","inReplyTo":"221107.865yfqv58k.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-07T21:59:12Z","receivedAt":"2022-11-07T21:59:55Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/7/22 4:03 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Nov 07 2022, Derrick Stolee wrote:\n> \n>> On 11/7/22 2:53 PM, Taylor Blau wrote:\n\n>>> I wonder how we should treat Ævar's concerns in this thread. I suspect\n>>> that the vast majority of workflows wouldn't be affected, but I don't\n>>> want to completely break Ævar's workflow, either ;-).\n>>>\n>>> Some kind of configuration mechanism like I proposed might be nice.\n>>> Thoughts?\n>>\n>> Taking a look at that sub-thread, I have two thoughts:\n>>\n>> 1. I don't think supporting a \"multiple pushes of WIP work\"\n>>    scenario is a good use of \"free\" resources. If you want to\n>>    test multiple versions of something, then use multiple\n>>    branches (and I think Johannes's patch allows concurrent\n>>    builds for distinct branch names).\n> \n> The setting Taylor proposed in\n> https://lore.kernel.org/git/Y2R3vJf1A2KOZwA7@nand.local/ is off by\n> default, i.e. it would behave the same way as what Johannes is\n> proposing, just give you (well, me) an opt-out from the default, without\n> patching main.yml on every branch.\n> \n> So it seems like a win-win, why force others to change their workflow?\n> Sure, I could push multiple branches, but you could also manually cancel\n> your outstanding jobs before re-pushing...\n> \n> I agree that cancelling the outstanding job is a better default, and if\n> we had to pick one or the other I'd say \"sure\", but if we can have\n> both...\n\n>> Either of these points may have an incorrect assumption, so\n>> I'm prepared to be wrong.\n> \n> I *think* you're wrong about #2, but I'm not sure either.\n\nAt the very least, the configurable option requires fetching the\nrepo and checking out at least one file. I don't know how much it\nactually saves one way or another.\n \n> I don't think you can be wrong about #1, \"others should change their\n> workflow to fit a new worldview\" is more of a value-judgment :)\n\nTrue, but I think that the workflow you are trying to keep valid\nis also indistinguishable to the typical flow of force-pushing\nduring incremental rewrites, so preserving your workflow will\ncontinue to add costs to that behavior.\n\nMy value judgement is that experts can adapt their workflows as\nsituations change for the better of the group.\n\nIf the cost of doing the config option version is minimal over\nthe global concurrency issue, then I say we should go that route.\nI just expect it to take up more resources than the strategy\nproposed in the initial patch.\n\nI wonder how we could determine this. Should we run a few CI\njobs with some force-pushes in either approach (config turned\noff) so we know that cost?\n\nThanks,\n-Stolee\n"},{"id":"466762","messageId":"Y2mKY+rE6X/Lu4pb@nand.local","threadId":"58741","inReplyTo":"114d4a72-1a75-71f3-8af6-6e82cd4fd54b@github.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-07T22:44:51Z","receivedAt":"2022-11-07T22:44:56Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Nov 07, 2022 at 04:59:12PM -0500, Derrick Stolee wrote:\n> On 11/7/22 4:03 PM, Ævar Arnfjörð Bjarmason wrote:\n> >\n> > On Mon, Nov 07 2022, Derrick Stolee wrote:\n> >\n> >> On 11/7/22 2:53 PM, Taylor Blau wrote:\n>\n> >>> I wonder how we should treat Ævar's concerns in this thread. I suspect\n> >>> that the vast majority of workflows wouldn't be affected, but I don't\n> >>> want to completely break Ævar's workflow, either ;-).\n> >>>\n> >>> Some kind of configuration mechanism like I proposed might be nice.\n> >>> Thoughts?\n> >>\n> >> Taking a look at that sub-thread, I have two thoughts:\n> >>\n> >> 1. I don't think supporting a \"multiple pushes of WIP work\"\n> >>    scenario is a good use of \"free\" resources. If you want to\n> >>    test multiple versions of something, then use multiple\n> >>    branches (and I think Johannes's patch allows concurrent\n> >>    builds for distinct branch names).\n> >\n> > The setting Taylor proposed in\n> > https://lore.kernel.org/git/Y2R3vJf1A2KOZwA7@nand.local/ is off by\n> > default, i.e. it would behave the same way as what Johannes is\n> > proposing, just give you (well, me) an opt-out from the default, without\n> > patching main.yml on every branch.\n> >\n> > So it seems like a win-win, why force others to change their workflow?\n> > Sure, I could push multiple branches, but you could also manually cancel\n> > your outstanding jobs before re-pushing...\n> >\n> > I agree that cancelling the outstanding job is a better default, and if\n> > we had to pick one or the other I'd say \"sure\", but if we can have\n> > both...\n>\n> >> Either of these points may have an incorrect assumption, so\n> >> I'm prepared to be wrong.\n> >\n> > I *think* you're wrong about #2, but I'm not sure either.\n>\n> At the very least, the configurable option requires fetching the\n> repo and checking out at least one file. I don't know how much it\n> actually saves one way or another.\n\nTo be clear, I think the savings here are minimal from a pure CPU-usage\nperspective. I'm more concerned with the expense of running a job which\ncounts double-digit minutes against your available Actions runtime.\n\nI played around with the following, but I can't quite get Actions to\nlike it. The error message I get (ref[1]) is something like:\n\n    The workflow is not valid. .github/workflows/main.yml (Line: 96, Col:\n    27): Unexpected value 'needs.ci-config.outputs.skip_concurrent == 'yes''\n    .github/workflows/main.yml (Line: 123, Col: 27): Unexpected value\n    'needs.ci-config.outputs.skip_concurrent == 'yes''\n\nBut I think the patch below should more-or-less be what we're looking\nfor:\n\n--- >8 ---\nSubject: [PATCH] ci: avoid unnecessary builds\n\nWhenever a branch is pushed to a repository which has GitHub Actions\nenabled, a bunch of new workflow runs are started.\n\nWe sometimes see contributors push multiple branch updates in rapid\nsuccession, which in conjunction with the impressive time swallowed by\neven just a single CI build frequently leads to many queued-up runs.\n\nThis is particularly problematic in the case of Pull Requests where a\nsingle contributor can easily (inadvertently) prevent timely builds for\nother contributors when using a shared repository.\n\nTo help with this situation, let's use the `concurrency` feature of\nGitHub workflows, essentially canceling GitHub workflow runs that are\nobsoleted by more recent runs:\n\n  https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency\n\nFor workflows that *do* want the behavior in the pre-image of this\npatch, they can use the ci-config feature to disable the new behavior by\nadding an executable script on the ci-config branch called\n'skip-concurrent' which terminates with a non-zero exit code.\n\nOriginal-patch-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n .github/workflows/check-whitespace.yml |  6 ++++\n .github/workflows/l10n.yml             |  6 ++++\n .github/workflows/main.yml             | 40 ++++++++++++++++++++++++--\n 3 files changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml\nindex ad3466ad16..0bcc9cffbd 100644\n--- a/.github/workflows/check-whitespace.yml\n+++ b/.github/workflows/check-whitespace.yml\n@@ -9,6 +9,12 @@ on:\n   pull_request:\n     types: [opened, synchronize]\n\n+# Avoid unnecessary builds. Unlike the main CI jobs, these are not\n+# ci-configurable (but could be).\n+concurrency:\n+  group: ${{ github.workflow }}-${{ github.ref }}\n+  cancel-in-progress: true\n+\n jobs:\n   check-whitespace:\n     runs-on: ubuntu-latest\ndiff --git a/.github/workflows/l10n.yml b/.github/workflows/l10n.yml\nindex 27f72f0ff3..51fd46e6af 100644\n--- a/.github/workflows/l10n.yml\n+++ b/.github/workflows/l10n.yml\n@@ -2,6 +2,12 @@ name: git-l10n\n\n on: [push, pull_request_target]\n\n+# Avoid unnecessary builds. Unlike the main CI jobs, these are not\n+# ci-configurable (but could be).\n+concurrency:\n+  group: ${{ github.workflow }}-${{ github.ref }}\n+  cancel-in-progress: true\n+\n jobs:\n   git-po-helper:\n     if: >-\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex bd6f75b8e0..87b5b369e1 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -11,6 +11,7 @@ jobs:\n     runs-on: ubuntu-latest\n     outputs:\n       enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}\n+      skip_concurrent: ${{ steps.check-ref.outputs.skip_concurrent }}\n     steps:\n       - name: try to clone ci-config branch\n         run: |\n@@ -34,7 +35,15 @@ jobs:\n           then\n             enabled=no\n           fi\n+\n+          skip_concurrent=yes\n+          if test -x config-repo/ci/config/skip-concurrent &&\n+             ! config-repo/ci/config/skip-concurrent '${{ github.ref }}'\n+          then\n+            skip_concurrent=no\n+          fi\n           echo \"::set-output name=enabled::$enabled\"\n+          echo \"::set-output name=skip_concurrent::$skip_concurrent\"\n       - name: skip if the commit or tree was already tested\n         id: skip-if-redundant\n         uses: actions/github-script@v3\n@@ -82,6 +91,9 @@ jobs:\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n     runs-on: windows-latest\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - uses: actions/checkout@v2\n     - uses: git-for-windows/setup-git-for-windows-sdk@v1\n@@ -101,11 +113,14 @@ jobs:\n   windows-test:\n     name: win test\n     runs-on: windows-latest\n-    needs: [windows-build]\n+    needs: [ci-config, windows-build]\n     strategy:\n       fail-fast: false\n       matrix:\n         nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - name: download tracked files and build artifacts\n       uses: actions/download-artifact@v2\n@@ -137,6 +152,9 @@ jobs:\n       NO_PERL: 1\n       GIT_CONFIG_PARAMETERS: \"'user.name=CI' 'user.email=ci@git'\"\n     runs-on: windows-latest\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - uses: actions/checkout@v2\n     - uses: git-for-windows/setup-git-for-windows-sdk@v1\n@@ -184,11 +202,14 @@ jobs:\n   vs-test:\n     name: win+VS test\n     runs-on: windows-latest\n-    needs: vs-build\n+    needs: [ci-config, vs-build]\n     strategy:\n       fail-fast: false\n       matrix:\n         nr: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - uses: git-for-windows/setup-git-for-windows-sdk@v1\n     - name: download tracked files and build artifacts\n@@ -218,6 +239,9 @@ jobs:\n     name: ${{matrix.vector.jobname}} (${{matrix.vector.pool}})\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     strategy:\n       fail-fast: false\n       matrix:\n@@ -281,6 +305,9 @@ jobs:\n     name: ${{matrix.vector.jobname}} (${{matrix.vector.image}})\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     strategy:\n       fail-fast: false\n       matrix:\n@@ -316,6 +343,9 @@ jobs:\n     env:\n       jobname: StaticAnalysis\n     runs-on: ubuntu-22.04\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - uses: actions/checkout@v2\n     - run: ci/install-dependencies.sh\n@@ -327,6 +357,9 @@ jobs:\n     env:\n       jobname: sparse\n     runs-on: ubuntu-20.04\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     steps:\n     - name: Download a current `sparse` package\n       # Ubuntu's `sparse` version is too old for us\n@@ -345,6 +378,9 @@ jobs:\n     name: documentation\n     needs: ci-config\n     if: needs.ci-config.outputs.enabled == 'yes'\n+    concurrency:\n+      group: ${{ github.job.name }}-${{ github.ref }}\n+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent == 'yes'\n     env:\n       jobname: Documentation\n     runs-on: ubuntu-latest\n--\n2.38.0.16.g393fd4c6db\n\n--- 8< ---\n\nThanks,\nTaylor\n\n[1]: https://github.com/ttaylorr/git/actions/runs/3414594555\n"},{"id":"466765","messageId":"221108.8635autlha.gmgdl@evledraar.gmail.com","threadId":"58741","inReplyTo":"114d4a72-1a75-71f3-8af6-6e82cd4fd54b@github.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-07T22:56:30Z","receivedAt":"2022-11-07T23:04:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 07 2022, Derrick Stolee wrote:\n\n> On 11/7/22 4:03 PM, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Mon, Nov 07 2022, Derrick Stolee wrote:\n>> \n>>> On 11/7/22 2:53 PM, Taylor Blau wrote:\n>\n>>>> I wonder how we should treat Ævar's concerns in this thread. I suspect\n>>>> that the vast majority of workflows wouldn't be affected, but I don't\n>>>> want to completely break Ævar's workflow, either ;-).\n>>>>\n>>>> Some kind of configuration mechanism like I proposed might be nice.\n>>>> Thoughts?\n>>>\n>>> Taking a look at that sub-thread, I have two thoughts:\n>>>\n>>> 1. I don't think supporting a \"multiple pushes of WIP work\"\n>>>    scenario is a good use of \"free\" resources. If you want to\n>>>    test multiple versions of something, then use multiple\n>>>    branches (and I think Johannes's patch allows concurrent\n>>>    builds for distinct branch names).\n>> \n>> The setting Taylor proposed in\n>> https://lore.kernel.org/git/Y2R3vJf1A2KOZwA7@nand.local/ is off by\n>> default, i.e. it would behave the same way as what Johannes is\n>> proposing, just give you (well, me) an opt-out from the default, without\n>> patching main.yml on every branch.\n>> \n>> So it seems like a win-win, why force others to change their workflow?\n>> Sure, I could push multiple branches, but you could also manually cancel\n>> your outstanding jobs before re-pushing...\n>> \n>> I agree that cancelling the outstanding job is a better default, and if\n>> we had to pick one or the other I'd say \"sure\", but if we can have\n>> both...\n>\n>>> Either of these points may have an incorrect assumption, so\n>>> I'm prepared to be wrong.\n>> \n>> I *think* you're wrong about #2, but I'm not sure either.\n>\n> At the very least, the configurable option requires fetching the\n> repo and checking out at least one file. I don't know how much it\n> actually saves one way or another.\n\nIt's already fetching the ci-config repo, so we're talking about the\nmarginal cost of running the bit of shellscript to check if\nconfig-repo/ci/config/skip-concurrent is executable, and if not keeping\nthe default config.\n\n>> I don't think you can be wrong about #1, \"others should change their\n>> workflow to fit a new worldview\" is more of a value-judgment :)\n>\n> True, but I think that the workflow you are trying to keep valid\n> is also indistinguishable to the typical flow of force-pushing\n> during incremental rewrites, so preserving your workflow will\n> continue to add costs to that behavior.\n\nI don't think it will, per the above. I mean, pedantically yes: But the\ncost of that \"test -x and variable setting\" is so trivial that it's not\nworth worrying about.\n\n> My value judgement is that experts can adapt their workflows as\n> situations change for the better of the group.\n\nSure, I agree with that in zero-sum scenarios, or where it's a hassle to\nprovide two things, and we need to pick one etc. I just don't see that\nbeing the case here.\n\n> If the cost of doing the config option version is minimal over\n> the global concurrency issue, then I say we should go that route.\n> I just expect it to take up more resources than the strategy\n> proposed in the initial patch.\n\nBased on what? That you read it as us cloning the ci-config repo just\nfor this new proposed config, and missed that we're doing it already, or\n...?\n\n> I wonder how we could determine this. Should we run a few CI\n> jobs with some force-pushes in either approach (config turned\n> off) so we know that cost?\n\nThe incremental cost of that \"test -x\", or...? I'm not sure what you\nmean here.\n"},{"id":"466769","messageId":"e185dc68-4a01-da57-8c68-a14448f9173f@github.com","threadId":"58741","inReplyTo":"221108.8635autlha.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-08T00:02:01Z","receivedAt":"2022-11-08T00:02:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/7/2022 5:56 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Nov 07 2022, Derrick Stolee wrote:\n> \n>> On 11/7/22 4:03 PM, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>> On Mon, Nov 07 2022, Derrick Stolee wrote:\n\n>>>> Either of these points may have an incorrect assumption, so\n>>>> I'm prepared to be wrong.\n>>>\n>>> I *think* you're wrong about #2, but I'm not sure either.\n>>\n>> At the very least, the configurable option requires fetching the\n>> repo and checking out at least one file. I don't know how much it\n>> actually saves one way or another.\n> \n> It's already fetching the ci-config repo, so we're talking about the\n> marginal cost of running the bit of shellscript to check if\n> config-repo/ci/config/skip-concurrent is executable, and if not keeping\n> the default config.\n\n>> I wonder how we could determine this. Should we run a few CI\n>> jobs with some force-pushes in either approach (config turned\n>> off) so we know that cost?\n> \n> The incremental cost of that \"test -x\", or...? I'm not sure what you\n> mean here.\n\nThe difference is that setting the concurrency globally allows\nActions to decide the concurrency from the workflow file alone,\nbefore any jobs are run. This is done without a clone.\n\nThe method introduced in e76eec35540 (ci: allow per-branch config\nfor GitHub Actions, 2020-05-07) requires running the ci-config\njob at minimum to set the concurrency value, which involves doing\na (very small, shallow) clone of the ci-config branch to determine\nif that file exists.\n\nSince this is the first job to run in the workflow, the global\nconcurrency will only reduces the amount of compute consumed\nwhen pushes happen close together, but that is included for \"oops\nI forgot to --amend\" and other common cases.\n\nAt the very least, the difference between the two mechanisms is\ngreater than a 'test -x'.\n\nThanks,\n-Stolee\n"},{"id":"466773","messageId":"xmqqk046cmmv.fsf@gitster.g","threadId":"58741","inReplyTo":"f975f57e-71e2-3227-8039-14dff82f04db@github.com","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-08T00:31:36Z","receivedAt":"2022-11-08T00:31:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee <derrickstolee@github.com> writes:\n\n> On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n>> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>> \n>> Whenever a branch is pushed to a repository which has GitHub Actions\n>> enabled, a bunch of new workflow runs are started.\n>> \n>> We sometimes see contributors push multiple branch updates in rapid\n>> succession, which in conjunction with the impressive time swallowed by\n>> even just a single CI build frequently leads to many queued-up runs.\n>> \n>> This is particularly problematic in the case of Pull Requests where a\n>> single contributor can easily (inadvertently) prevent timely builds for\n>> other contributors.\n>\n> As someone who is both the cause and the victim of this, I\n> thank you for finding a way to reduce wasted CPU time. This\n> patch looks good to me, though I'll need to trust the docs\n> and your testing to be sure it will work. We will definitely\n> see it in place as it merges into 'next' and 'main'.\n\nWhen I see breakages of 'seen' only at the CI, perhaps because it\nmanifests only on macOS, I manually \"bisected\" by pushing various\ncombinations of topics merged on top of 'master' and pushing the\nresult out as 'seen' only to the GitHub repository, and not having\nto wait one to finish before pushing out another was a really nice\nfeature.  Of course, I could wait before pushing another out, but\nafter seeing the last one close to successful completion in a few\nminutes and being able to push out the next one was a great\ntimesaver, not only for the \"few minutes\", but for the countless\nminutes because I will have to concentrate on more than just a \"few\nminutes\" on another task if I have to switch to another task in\norder to wait for just a \"few minutes\" before pushing the trial\nmerge out.\n\nSo, that is the only concern I have with this change, but in\ngeneral, not running jobs whose results are clearly not needed is a\ngood idea.  It just is \"clearly\" is hard to determine automatically.\n"},{"id":"466794","messageId":"75ono097-16nr-nno4-rqoq-rrn79spps249@tzk.qr","threadId":"58741","inReplyTo":"Y2mKY+rE6X/Lu4pb@nand.local","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-08T08:18:21Z","receivedAt":"2022-11-08T08:18:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Taylor,\n\nOn Mon, 7 Nov 2022, Taylor Blau wrote:\n\n> I'm more concerned with the expense of running a job which counts\n> double-digit minutes against your available Actions runtime.\n\nMe, too. The fact that we used up 50.000 build minutes before we were able\nto finalize v2.38.1 is quite concerning.\n\n> I played around with the following, but I can't quite get Actions to\n> like it. The error message I get (ref[1]) is something like:\n>\n>     The workflow is not valid. .github/workflows/main.yml (Line: 96, Col:\n>     27): Unexpected value 'needs.ci-config.outputs.skip_concurrent == 'yes''\n>     .github/workflows/main.yml (Line: 123, Col: 27): Unexpected value\n>     'needs.ci-config.outputs.skip_concurrent == 'yes''\n\nThe reason is that what you are trying to do simply cannot work.\n\nThe `concurrency` setting is evaluated _before_ running the build. Yet you\nwant to make it contingent on the output of a job that only runs _as part_\nof the build. Catch-22.\n\nReading through the thread, my assessment is that the original patch is\nstill the best we can do.\n\nI also want to point out that the strategy described by Ævar to push every\nhour and see a \"breadcrumb trail\" of green builds is quite wasteful. Just\nbecause somebody else pays for it does not mean that it is free of cost.\nIt very much looks like wasting resources unnecessarily to me, and I do\nnot condone this, and I believe that the Git project should neither\nencourage nor support this.\n\nIf you _must_ have concurrent builds of your branch (i.e. if you want to\nuse that many resources despite the planet literally burning already from\nwasteful use of resources), you can always start your branch with a patch\nthat comments out the `cancel-in-progress` line, without forcing the\ncomplexity of a more generic support for this behavior into Git's code\nbase.\n\nCiao,\nDscho\n"},{"id":"466797","messageId":"oo9ssp5n-6ors-n309-2r2n-3q43rq7pn89q@tzk.qr","threadId":"58741","inReplyTo":"Y2SFGmQnx7CXtTEI@coredump.intra.peff.net","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-08T09:16:15Z","receivedAt":"2022-11-08T09:16:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 3 Nov 2022, Jeff King wrote:\n\n> On Thu, Nov 03, 2022 at 10:23:56PM -0400, Taylor Blau wrote:\n>\n> > But I think you make a compelling point (which doesn't match my own\n> > workflow, but I can see the utility of nonetheless).\n> >\n> > I was thinking that we could rely on something similar to the ci-config\n> > ref stuff from back in e76eec35540 (ci: allow per-branch config for\n> > GitHub Actions, 2020-05-07), but it looks like it'll be a little\n> > trickier than that, maybe impossible.\n> >\n> > We need to know about the state of the ci-config branch before we set\n> > the concurrency bits. So I think you *could* do something like:\n>\n> As an aside, I wish there was a way to interpret per-repo environment\n> variables in the actual action config.\n\nThere kind of is. \"Kind of\" because it is not _really_ a per-repo variable\n(those do not exist on GitHub), but there are topics you can set. These\nare relatively free-form labels you can attach to _your_ fork, and these\nlabels show up below the \"About\" section and the link to the home page (if\nany) on the right side of your respository. AFAICT these topics are not\ninherited automatically when forking a repository, which is precisely what\nwe want. See\nhttps://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/classifying-your-repository-with-topics\nfor more details on that.\n\nA GitHub workflow can be made conditional on the presence of such a topic.\nI use this, for example, for an opt-in build of the InnoSetup installer:\nhttps://github.com/jrsoftware/issrc/blob/is-6_2_1/.github/workflows/build.yml#L11-L12\nThe build is opt-in because it requires a non-free build environment,\nconfigured via two repository secrets containing a URL pointing to a\n`.zip` file and a password to extract said `.zip`. As you say, I cannot\nmake the build opt-in based on the presence of secrets, but I can use the\npresence a repository topic as the knob to enable the workflow.\n\n> The current ci-config stuff works, but it's pretty horrible because (if\n> I understand correctly) it spins up a VM just to evaluate a glob and say\n> \"nope, no CI needed on this branch\". So:\n>\n>   1. It's wasteful of resources, compared to a system where the Actions\n>      parser can evaluate a variable.\n\nIndeed. It might look like the job only takes a few seconds (at least that\nwas the argument that got the `ci-config` patch accepted), but that misses\nthe queue time, which turns this more into several dozens of seconds, and\nthe recorded total duration is much longer than that. In\nhttps://github.com/gitgitgadget/git/actions/runs/3412982102 for example,\nthe `ci-config` job only took 6 seconds, according to the page, but the\ntotal duration of the build was 6 minutes and 56 seconds.\n\n>   2. It makes the Actions results page for a repo ugly, because skipped\n>      branches clutter the output with \"yes, I passed CI\" even though all\n>      they passed was a trivial job to say \"don't bother running more\n>      CI\".\n>\n>   3. The problem you mention: it happens too late to affect the overall\n>      Actions flow, and instead individual jobs have to take it into\n>      account.\n\nAnd\n\n    4. The way `ci-config` is configured is sufficiently \"magic\" that it\n       only benefits very, very few users, at the price of adding to\n       everybody's build time. To see what I mean, look at\n       https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685867765#step:1:1\n       and at\n       https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685879914#step:1:1\n       turning on the timestamps (i.e. click on the sprocket wheel on the\n       upper right side of the log and select \"Show timestamps\"). You will\n       see that the `ci-config` job started at 3:22:05 UTC and the next\n       job, `win-build`, started only at 4:16:03 UTC.\n\n    5. There is official support for the desired behavior that comes\n       without any magic branch with special content that users somehow\n       need to learn about: If you push a branch with commit messages that\n       contain `[skip ci]`, the build will be skipped, and no time is\n       wasted on running any job. For full details, see\n       https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/\n\nHaving said that, the `ci-config` job is used for something I find much\nmore useful than that magic `ci-config` branch handling: to skip builds\nwhen there are already successful builds of the same commit or at least\ntree. Sadly, that logic fails sometimes because of an unresilient REST API\ncall: in case of network errors, we fall back to not skipping the build\n_even if_ there has been a previously-successful build of that tree.\n\nIf it was up to me, I'd simply rip out the `ci-config` branch (`ci/config`\nfile) handling and tell users to mark up branches that need to be skipped\nwith `[skip ci]`. That has a further advantage of being actually portable\nacross a wider range of CI systems (for example, CircleCI supports it,\ntoo: https://circleci.com/docs/skip-build/).\n\nBut then, it would not save us a whole lot because the `ci-config` job\n_still_ does something useful (i.e. checking for previously successful\nbuilds of the same tree), so that time is still spent. But how knows,\nmaybe there will be out-of-the-box support for that at some stage. 🤷\n\nCiao,\nDscho\n"},{"id":"466800","messageId":"2p770o19-s3o3-o6r9-18r5-25n91r1r2210@tzk.qr","threadId":"58741","inReplyTo":"xmqqk046cmmv.fsf@gitster.g","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-11-08T09:51:45Z","receivedAt":"2022-11-08T09:52:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 7 Nov 2022, Junio C Hamano wrote:\n\n> Derrick Stolee <derrickstolee@github.com> writes:\n>\n> > On 11/3/22 9:34 AM, Johannes Schindelin via GitGitGadget wrote:\n> >> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >>\n> >> Whenever a branch is pushed to a repository which has GitHub Actions\n> >> enabled, a bunch of new workflow runs are started.\n> >>\n> >> We sometimes see contributors push multiple branch updates in rapid\n> >> succession, which in conjunction with the impressive time swallowed by\n> >> even just a single CI build frequently leads to many queued-up runs.\n> >>\n> >> This is particularly problematic in the case of Pull Requests where a\n> >> single contributor can easily (inadvertently) prevent timely builds for\n> >> other contributors.\n> >\n> > As someone who is both the cause and the victim of this, I\n> > thank you for finding a way to reduce wasted CPU time. This\n> > patch looks good to me, though I'll need to trust the docs\n> > and your testing to be sure it will work. We will definitely\n> > see it in place as it merges into 'next' and 'main'.\n>\n> When I see breakages of 'seen' only at the CI, perhaps because it\n> manifests only on macOS, I manually \"bisected\" by pushing various\n> combinations of topics merged on top of 'master' and pushing the\n> result out as 'seen' only to the GitHub repository, and not having\n> to wait one to finish before pushing out another was a really nice\n> feature.  Of course, I could wait before pushing another out, but\n> after seeing the last one close to successful completion in a few\n> minutes and being able to push out the next one was a great\n> timesaver, not only for the \"few minutes\", but for the countless\n> minutes because I will have to concentrate on more than just a \"few\n> minutes\" on another task if I have to switch to another task in\n> order to wait for just a \"few minutes\" before pushing the trial\n> merge out.\n\nI often find myself with similar problems where I have to test a couple of\nrevisions in order to pinpoint regressions that do not reproduce locally.\n\nOne of my power tools to figure these things out is\nhttps://github.com/mxschmitt/action-tmate, allowing me to SSH into the\nbuild agent where the failure happens (note: do use with prudence lest your\naccount gets flagged for potential mining).\n\nI find that much more surgical a tool than having multiple builds run\nconcurrently, not the least reason being that keeping track of which\nbuilds correspond to which step of the bisection can be very, very\nconfusing.\n\nAlso, one of my core values is to use up only the resources that I need\nto. And using up a lot of build minutes is contrary to that value.\n\nSo what I end up doing in similar situations is:\n\n- first of all, switch to a temporary branch\n\n- then, add a TO-DROP commit that rips out _all_ of the unnecessary\n  builds. Typically the first to go are check-whitespace and git-l10n, and\n  then I liberally delete jobs (including the `ci-config` job that's\n  totally useless in this use case) and restrict the test suite to running\n  just the failing script by editing the `T = [...]` line in `t/Makefile`,\n  often even adding a `--run=[... only the minimal amount of test\n  cases...]` (after figuring out which ones are needed which is\n  unfortunately quite hard due to the abundance of side effects our test\n  suite relies on) to the `GIT_TEST_OPTS` in `ci/`.\n\n- then, if I need to test multiple revisions, I _create_ new branches for\n  each bisection point (typically encoding information in the branch name\n  that will help me with book-keeping), cherry-picking the just-created\n  TO-DROP commit before pushing.\n\nSince I want to minimize my footprint when it comes to using resources, I\ntypically am very judicious about what revisions I test, and how many I\nrun concurrently.\n\nIt is a bit sad that doing this is currently very much involved and that\nit is so much easier to just go ahead and run the entire test suite on\nevery available platform even if the test failures one wishes to diagnose\nhappen on but one platform in but one test script. I.e. in the current\nshape, our code base encourages wasting of resources.\n\nThat is a situation I would like to see improved. If I was not committed\nto other work streams, I would work on it because I find it that\nimportant.\n\n> So, that is the only concern I have with this change, but in\n> general, not running jobs whose results are clearly not needed is a\n> good idea.  It just is \"clearly\" is hard to determine automatically.\n\nRight. It is also very subjective what \"clearly\" should mean in this\ncontext. For example, I find it clearly undesirable just how long our test\nsuite takes (and not for any good reason, really) and how inflexible it is\nwhen it comes to things like Test Impact Analysis (i.e. running only tests\ncovering the code modified in a given PR, which would speed up everything\n_tremendously_). At the same time I am fully aware that I find myself\npretty alone in this assessment, otherwise other contributors would\nclearly be more interested in fixing these issues than they are.\n\nCiao,\nDscho\n"},{"id":"466870","messageId":"Y2qgPKRW1+9BNkIp@nand.local","threadId":"58741","inReplyTo":"75ono097-16nr-nno4-rqoq-rrn79spps249@tzk.qr","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-08T18:30:20Z","receivedAt":"2022-11-08T18:30:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Nov 08, 2022 at 09:18:21AM +0100, Johannes Schindelin wrote:\n> > I played around with the following, but I can't quite get Actions to\n> > like it. The error message I get (ref[1]) is something like:\n> >\n> >     The workflow is not valid. .github/workflows/main.yml (Line: 96, Col:\n> >     27): Unexpected value 'needs.ci-config.outputs.skip_concurrent == 'yes''\n> >     .github/workflows/main.yml (Line: 123, Col: 27): Unexpected value\n> >     'needs.ci-config.outputs.skip_concurrent == 'yes''\n>\n> The reason is that what you are trying to do simply cannot work.\n\nI was surprised that I couldn't get this to work, because to me it\nseemed like the sort of thing that *should* be possible to do.\n\nIndeed, it is, and I made a couple of mistakes in writing the workflow\nfile:\n\n  - The expression for 'skip-in-progress' needed to be enclosed in\n    '${{}}' markers.\n\n  - It also needed to take into account the job name (and matrix\n    information!) where relevant. And here we can't just use\n    ${{github.job}}, since that is only available inside of the job\n    steps.\n\nTo the last bullet point there, we unfortunately have to copy and paste\nthe job name, which seems like a limitation of the Actions workflow\nparser to me.\n\nI posted an alternative approach to this patch in [1], and I would be\nvery curious to hear your thoughts, if you have time!\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/cover.1667931937.git.me@ttaylorr.com/T/#t\n"},{"id":"466949","messageId":"Y2uygXD7R+fsC0as@coredump.intra.peff.net","threadId":"58741","inReplyTo":"oo9ssp5n-6ors-n309-2r2n-3q43rq7pn89q@tzk.qr","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-11-09T14:00:33Z","receivedAt":"2022-11-09T14:00:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 08, 2022 at 10:16:15AM +0100, Johannes Schindelin wrote:\n\n> > As an aside, I wish there was a way to interpret per-repo environment\n> > variables in the actual action config.\n> \n> There kind of is. \"Kind of\" because it is not _really_ a per-repo variable\n> (those do not exist on GitHub), but there are topics you can set. These\n> are relatively free-form labels you can attach to _your_ fork, and these\n> labels show up below the \"About\" section and the link to the home page (if\n> any) on the right side of your respository. AFAICT these topics are not\n> inherited automatically when forking a repository, which is precisely what\n> we want. See\n> https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/classifying-your-repository-with-topics\n> for more details on that.\n\nAh, that's very clever, thank you!\n\nFor the original problem that motivated me adding ci-config in the first\nplace, branch selection, I think we could do this:\n\n  if: |\n    !contains(join(github.event.repository.topics), 'ci-only-') ||\n    contains(github.event.repository.topics, format('ci-only-{0}', github.ref_name))\n\nand then by default we'd continue to build for all pushes, but if you\nadd ci-only-foo as a repo topic, then we'd build only refs/heads/foo.\n\nI may see if I can work this into our workflow file. Even if we can't\nget rid of ci-config for the skip-successful-build feature, this would\nstill save CPU by dropping even ci-config when the branch should be\nskipped entirely.\n\n> > The current ci-config stuff works, but it's pretty horrible because (if\n> > I understand correctly) it spins up a VM just to evaluate a glob and say\n> > \"nope, no CI needed on this branch\". So:\n> >\n> >   1. It's wasteful of resources, compared to a system where the Actions\n> >      parser can evaluate a variable.\n> \n> Indeed. It might look like the job only takes a few seconds (at least that\n> was the argument that got the `ci-config` patch accepted), but that misses\n> the queue time, which turns this more into several dozens of seconds, and\n> the recorded total duration is much longer than that. In\n> https://github.com/gitgitgadget/git/actions/runs/3412982102 for example,\n> the `ci-config` job only took 6 seconds, according to the page, but the\n> total duration of the build was 6 minutes and 56 seconds.\n\nYes, but I don't think that's wasting 6 minutes of resources if we're\njust sitting in a queue. It may increase the end-to-end latency of\ngetting the CI result, of course.\n\n>     4. The way `ci-config` is configured is sufficiently \"magic\" that it\n>        only benefits very, very few users, at the price of adding to\n>        everybody's build time. To see what I mean, look at\n>        https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685867765#step:1:1\n>        and at\n>        https://github.com/gitgitgadget/git/actions/runs/3416084640/jobs/5685879914#step:1:1\n>        turning on the timestamps (i.e. click on the sprocket wheel on the\n>        upper right side of the log and select \"Show timestamps\"). You will\n>        see that the `ci-config` job started at 3:22:05 UTC and the next\n>        job, `win-build`, started only at 4:16:03 UTC.\n\nOuch. Though I wonder in practice how fast that would have gone without\nci-config. It is just asking for a generic ubuntu vm, which many of\nother jobs would. Was there a shortage of those vms at 3:22, and by the\ntime it finally ran that shortage was over? Or is there constant\ncontention, and it is increasing the end-to-end latency by asking for a\nqueue slot, running, and then asking for more queue slots?\n\n>     5. There is official support for the desired behavior that comes\n>        without any magic branch with special content that users somehow\n>        need to learn about: If you push a branch with commit messages that\n>        contain `[skip ci]`, the build will be skipped, and no time is\n>        wasted on running any job. For full details, see\n>        https://github.blog/changelog/2021-02-08-github-actions-skip-pull-request-and-push-workflows-with-skip-ci/\n\nThis existed back when I added ci-config originally, and I tried it, but\nthe results are quite painful to use. Because \"skip ci\" is really a\nproperty of a branch that a commit is pushed to, and not a branch. So\nyou have to sprinkle these \"skip ci\" markers all over the branch tips,\nbut then remember to remove them when you actually want to do useful\nthings with the branches, like merge them or send them out.\n\n-Peff\n"},{"id":"467027","messageId":"Y2xknh1TqMTdpIxE@nand.local","threadId":"58741","inReplyTo":"Y2uygXD7R+fsC0as@coredump.intra.peff.net","subject":"Re: [PATCH] ci: avoid unnecessary builds","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-11-10T02:40:30Z","receivedAt":"2022-11-10T02:40:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Nov 09, 2022 at 09:00:33AM -0500, Jeff King wrote:\n> On Tue, Nov 08, 2022 at 10:16:15AM +0100, Johannes Schindelin wrote:\n>\n> > > As an aside, I wish there was a way to interpret per-repo environment\n> > > variables in the actual action config.\n> >\n> > There kind of is. \"Kind of\" because it is not _really_ a per-repo variable\n> > (those do not exist on GitHub), but there are topics you can set. These\n> > are relatively free-form labels you can attach to _your_ fork, and these\n> > labels show up below the \"About\" section and the link to the home page (if\n> > any) on the right side of your respository. AFAICT these topics are not\n> > inherited automatically when forking a repository, which is precisely what\n> > we want. See\n> > https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/classifying-your-repository-with-topics\n> > for more details on that.\n>\n> Ah, that's very clever, thank you!\n\nVery cute.\n\n> For the original problem that motivated me adding ci-config in the first\n> place, branch selection, I think we could do this:\n>\n>   if: |\n>     !contains(join(github.event.repository.topics), 'ci-only-') ||\n>     contains(github.event.repository.topics, format('ci-only-{0}', github.ref_name))\n>\n> and then by default we'd continue to build for all pushes, but if you\n> add ci-only-foo as a repo topic, then we'd build only refs/heads/foo.\n>\n> I may see if I can work this into our workflow file. Even if we can't\n> get rid of ci-config for the skip-successful-build feature, this would\n> still save CPU by dropping even ci-config when the branch should be\n> skipped entirely.\n\nYeah. I'd be hesitant to see a bunch of CI configuration knobs get\nfolded into the repository topics feature, but a clear \"save CI minutes\nif we don't care about building these branch(es)\" is useful.\n\nThanks,\nTaylor\n"}]}