All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] ci: avoid unnecessary builds
Date: Thu, 3 Nov 2022 22:23:56 -0400	[thread overview]
Message-ID: <Y2R3vJf1A2KOZwA7@nand.local> (raw)
In-Reply-To: <221104.86bkpnzdqi.gmgdl@evledraar.gmail.com>

On Fri, Nov 04, 2022 at 02:46:23AM +0100, Ævar Arnfjörð Bjarmason wrote:
> > This is particularly problematic in the case of Pull Requests where a
> > single contributor can easily (inadvertently) prevent timely builds for
> > other contributors.
>
> The "timely" being an issue in git/git and/or gitgitgadget where CI time
> is a shared resource, but not in a <someuser>/git running CI just for
> <someuser>?

Yup, agreed.

> > To help with this situation, let's use the `concurrency` feature of
> > GitHub workflows, essentially canceling GitHub workflow runs that are
> > obsoleted by more recent runs:
> > https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency
>
> In my own fork I very much use this concurrency not-cancel-in-progress
> intentionally.

Interesting. I noted basically the opposite in my earlier reply[1] to
Johannes, where the behavior I want is that newer pushes of the same
topic supersede older ones that are currently in progress.

But I think you make a compelling point (which doesn't match my own
workflow, but I can see the utility of nonetheless).

I was thinking that we could rely on something similar to the ci-config
ref stuff from back in e76eec35540 (ci: allow per-branch config for
GitHub Actions, 2020-05-07), but it looks like it'll be a little
trickier than that, maybe impossible.

We need to know about the state of the ci-config branch before we set
the concurrency bits. So I think you *could* do something like:

--- >8 ---
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 4fdf4d3552..f1ca364f96 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -2,11 +2,6 @@ name: CI

 on: [push, pull_request]

-# Avoid unnecessary builds
-concurrency:
-  group: ${{ github.workflow }}-${{ github.ref }}
-  cancel-in-progress: true
-
 env:
   DEVELOPER: 1

@@ -39,7 +34,14 @@ jobs:
           then
             enabled=no
           fi
+          skip_concurrent=yes
+          if test -x config-repo/ci/config/skip-concurrent &&
+             ! config-repo/ci/config/skip-concurrent '${{ github.ref }}'
+          then
+            skip_concurrent=no
+          fi
           echo "::set-output name=enabled::$enabled"
+          echo "::set-output name=skip_concurrent::$skip_concurrent"
       - name: skip if the commit or tree was already tested
         id: skip-if-redundant
         uses: actions/github-script@v3
@@ -86,6 +88,9 @@ jobs:
     name: win build
     needs: ci-config
     if: needs.ci-config.outputs.enabled == 'yes'
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.ref }}
+      cancel-in-progress: needs.ci-config.outputs.skip_concurrent = 'yes'
     runs-on: windows-latest
     steps:
     - uses: actions/checkout@v2
--- 8< ---

...and similar "concurrency" blocks in each of the other jobs to define
the settings at the job level instead of at the workflow level.

So, it's doable, but a little gross. At the very least, it would satisfy
Ævar's workflow requirements, too, since he could write a script that
exits with non-zero status to avoid the new behavior.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/Y2R0YrQzKaUZzaPB@nand.local/

  reply	other threads:[~2022-11-04  2:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 13:34 [PATCH] ci: avoid unnecessary builds Johannes Schindelin via GitGitGadget
2022-11-04  1:46 ` Ævar Arnfjörð Bjarmason
2022-11-04  2:23   ` Taylor Blau [this message]
2022-11-04  3:20     ` Jeff King
2022-11-08  9:16       ` Johannes Schindelin
2022-11-09 14:00         ` Jeff King
2022-11-10  2:40           ` Taylor Blau
2022-11-04  2:09 ` Taylor Blau
2022-11-07 19:45 ` Derrick Stolee
2022-11-07 19:53   ` Taylor Blau
2022-11-07 20:08     ` Derrick Stolee
2022-11-07 21:03       ` Ævar Arnfjörð Bjarmason
2022-11-07 21:59         ` Derrick Stolee
2022-11-07 22:44           ` Taylor Blau
2022-11-08  8:18             ` Johannes Schindelin
2022-11-08 18:30               ` Taylor Blau
2022-11-07 22:56           ` Ævar Arnfjörð Bjarmason
2022-11-08  0:02             ` Derrick Stolee
2022-11-08  0:31   ` Junio C Hamano
2022-11-08  9:51     ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y2R3vJf1A2KOZwA7@nand.local \
    --to=me@ttaylorr.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.