git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Taylor Blau" <me@ttaylorr.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	git@vger.kernel.org, "Jeff Hostetler" <jeffhost@microsoft.com>,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches
Date: Tue, 5 May 2020 17:04:51 -0400	[thread overview]
Message-ID: <20200505210451.GA645290@coredump.intra.peff.net> (raw)
In-Reply-To: <20200505182418.GA66702@coredump.intra.peff.net>

On Tue, May 05, 2020 at 02:24:18PM -0400, Jeff King wrote:

> But _if_ we can read from other refs in the repository, I would be very
> happy if we parsed config out of refs/ci/branches or something. It feels
> like that's something that ought to be possible, but I haven't quite
> figured out a way to do it.

OK, I finally figured this out. The result is the patch below, which I
think should make everybody happy. Or at least has the ability to do so
if they're willing to push a config ref. ;)

-- >8 --
Subject: [PATCH] ci: allow per-branch config for GitHub Actions

Depending on the workflows of individual developers, it can either be
convenient or annoying that our GitHub Actions CI jobs are run on every
branch. As an example of annoying: if you carry many half-finished
work-in-progress branches and rebase them frequently against master,
you'd get tons of failure reports that aren't interesting (not to
mention the wasted CPU).

This commit adds a new job which checks a special ref within the
repository for CI config, and (optionally) causes all of the other jobs
to be skipped.

There have been a few alternatives discussed:

One option is to carry information in the commit itself about whether it
should be tested, either in the tree itself (changing the workflow YAML
file) or in the commit message (a "[skip ci]" flag or similar). But
these are frustrating and error-prone to use:

  - you have to manually apply them to each branch that you want to mark

  - it's easy for them to leak into other workflows, like emailing patches

We could likewise try to get some information from the branch name. But
that leads to debates about whether the default should be "off" or "on",
and overriding still ends up somewhat awkward. If we default to "on",
you have to remember to name your branches appropriately to skip CI. And
if "off", you end up having to contort your branch names or duplicate
your pushes with an extra refspec.

By comparison, this commit's solution lets you specify your config once
and forget about it, and all of the data is off in its own ref, where it
can be changed by individual forks without touching the main tree.

I used refs/ci/config as the config ref, which should be a commit whose
tree contains various config files (right now the only one is
"ref-whitelist"). It was intentional to avoid refs/heads/ here so we
don't conflict with any branch workflows. But it does make it a little
awkward to edit, since you can't check it out directly.

Right now the logic is to run CI for all branches by default, unless a
whitelist exists, in which case the branch must be mentioned there
(using its fully qualified ref name). We could easily add in a
blacklist, as well. Or since we're running a shell in a VM, we really
could just run "./allow-ref $refname" and let individual forks specify
whatever shell code they like.

Signed-off-by: Jeff King <peff@peff.net>
---
After writing that, I think we probably ought to just do the allow-ref
thing from the start, and skip this whitelist logic. Then we should
never need to change this workflow file again. People can implement
whatever weird custom logic they want to.

 .github/workflows/main.yml | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index fd4df939b5..51f4ff6e89 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -6,7 +6,29 @@ env:
   DEVELOPER: 1
 
 jobs:
+  check-ci:
+      runs-on: ubuntu-latest
+      outputs:
+        enabled: ${{ steps.check-ref.outputs.enabled }}
+      steps:
+        - uses: actions/checkout@v2
+          continue-on-error: true
+          with:
+            ref: refs/ci/config
+        - id: check-ref
+          name: check whether CI is enabled for ref
+          run: |
+            enabled=yes
+            if test -e ref-whitelist &&
+               ! grep '^${{ github.ref }}$' ref-whitelist
+            then
+              enabled=no
+            fi
+            echo "::set-output name=enabled::$enabled"
+
   windows-build:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     runs-on: windows-latest
     steps:
     - uses: actions/checkout@v1
@@ -70,6 +92,8 @@ jobs:
         name: failed-tests-windows
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   vs-build:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     env:
       MSYSTEM: MINGW64
       NO_PERL: 1
@@ -154,6 +178,8 @@ jobs:
                           ${{matrix.nr}} 10 t[0-9]*.sh)
         "@
   regular:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     strategy:
       matrix:
         vector:
@@ -189,6 +215,8 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   dockerized:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     strategy:
       matrix:
         vector:
@@ -213,6 +241,8 @@ jobs:
         name: failed-tests-${{matrix.vector.jobname}}
         path: ${{env.FAILED_TEST_ARTIFACTS}}
   static-analysis:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     env:
       jobname: StaticAnalysis
     runs-on: ubuntu-latest
@@ -221,6 +251,8 @@ jobs:
     - run: ci/install-dependencies.sh
     - run: ci/run-static-analysis.sh
   documentation:
+    needs: check-ci
+    if: needs.check-ci.outputs.enabled == 'yes'
     env:
       jobname: Documentation
     runs-on: ubuntu-latest
-- 
2.26.2.962.g87ee755179


  reply	other threads:[~2020-05-05 21:04 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-02 15:08 [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" Johannes Schindelin via GitGitGadget
2020-05-03  9:36 ` Jeff King
2020-05-03 12:05   ` Danh Doan
2020-05-04 15:01     ` Jeff King
2020-05-04 15:49       ` [PATCH v2 0/2] Limit GitHub Actions to designated branches Đoàn Trần Công Danh
2020-05-04 15:49         ` [PATCH v2 1/2] CI: limit " Đoàn Trần Công Danh
2020-05-04 16:23           ` Jeff King
2020-05-04 21:58             ` Taylor Blau
2020-05-04 22:52               ` Junio C Hamano
2020-05-04 23:15                 ` Taylor Blau
2020-05-04 23:35                   ` Jeff King
2020-05-05  0:24                     ` Junio C Hamano
2020-05-04 23:36               ` Jeff King
2020-05-05  0:20                 ` Taylor Blau
2020-05-05 16:43                   ` Jeff King
2020-05-05 17:57                     ` Junio C Hamano
2020-05-05 18:24                       ` Jeff King
2020-05-05 21:04                         ` Jeff King [this message]
2020-05-05 21:29                           ` Junio C Hamano
2020-05-05 21:58                             ` Jeff King
2020-05-05 22:28                               ` Junio C Hamano
2020-05-06 15:09                             ` Johannes Schindelin
2020-05-06 16:26                               ` Junio C Hamano
2020-05-07 12:17                                 ` Jeff King
2020-05-07 14:02                                   ` Jeff King
2020-05-07 18:17                                     ` Junio C Hamano
2020-05-07 12:01                               ` Đoàn Trần Công Danh
2020-05-07 12:47                                 ` Đoàn Trần Công Danh
2020-05-06  0:46                           ` Đoàn Trần Công Danh
2020-05-06  3:56                             ` Junio C Hamano
2020-05-06 14:25                               ` Đoàn Trần Công Danh
2020-05-06 16:31                                 ` Junio C Hamano
2020-05-07 12:25                                   ` Jeff King
2020-05-07 18:29                                     ` Junio C Hamano
2020-05-07 18:54                                       ` Jeff King
2020-05-07 19:33                                         ` Junio C Hamano
2020-05-07 16:20                           ` [PATCH v2] ci: allow per-branch config for GitHub Actions Jeff King
2020-05-07 17:00                             ` Taylor Blau
2020-05-07 17:18                               ` Jeff King
2020-05-07 19:53                             ` Junio C Hamano
2020-05-07 20:46                               ` Jeff King
2020-05-07 21:58                                 ` Junio C Hamano
2020-05-08 18:00                                   ` Jeff King
2020-05-09  1:23                                     ` Đoàn Trần Công Danh
2020-05-05  0:34             ` [PATCH v2 1/2] CI: limit GitHub Actions to designated branches Đoàn Trần Công Danh
2020-05-04 15:49         ` [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI Đoàn Trần Công Danh
2020-05-04 16:37           ` Junio C Hamano
2020-05-05  0:46             ` Đoàn Trần Công Danh
2020-05-05 16:26         ` [PATCH v3 0/3] Provide option to opt in/out GitHub Actions Đoàn Trần Công Danh
2020-05-05 16:26           ` [PATCH v3 1/3] SubmittingPatches: advertise GitHub Actions CI Đoàn Trần Công Danh
2020-05-05 16:47             ` Jeff King
2020-05-05 16:59               ` Đoàn Trần Công Danh
2020-05-05 17:07                 ` Jeff King
2020-05-05 16:26           ` [PATCH v3 2/3] CI: limit GitHub Actions to designated branches Đoàn Trần Công Danh
2020-05-05 16:51             ` Jeff King
2020-05-05 17:05               ` Đoàn Trần Công Danh
2020-05-05 17:11                 ` Jeff King
2020-05-05 18:49             ` Junio C Hamano
2020-05-05 16:26           ` [PATCH v3 3/3] fixup! " Đoàn Trần Công Danh
2020-05-05 18:59             ` Junio C Hamano
2020-05-05 17:01           ` [PATCH v3 0/3] Provide option to opt in/out GitHub Actions Jeff King
2020-05-03 16:46   ` [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20200505210451.GA645290@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=me@ttaylorr.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).