Hi Peff,
On Thu, May 07, 2020 at 12:20:11PM -0400, Jeff King wrote:
> On Tue, May 05, 2020 at 05:04:51PM -0400, Jeff King wrote:
>
> > Subject: [PATCH] ci: allow per-branch config for GitHub Actions
>
> Here's a "v2" of that patch based on the discussion.
I really like this direction. I think that it's a good mix of
flexibility and convenience. I'm happy to push a one-time 'ci-config'
branch to 'ttaylorr/git' and forget about it.
> I think it smooths some of the rough edges of the orphan-branch
> approach, while still having a cost on par with other suggestions (or at
> least ones that truly allow any config; we can check for "for-ci/**" or
> something very cheaply, but that implies hard-coding it for everybody).
> I think the cost here is acceptable, and it gives us room to add more
> features in the future.
>
> If Actions eventually adds per-repo variable storage that can be used in
> "if:" conditionals, then we could eventually switch to that. :)
>
> The documentation here should be enough to let people work with it. But
> we'd probably want to take Danh's patch to mention Actions in
> SubmittingPatches on top (and it possibly could be modified to point to
> the ci/config directory).
>
> -- >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 branch within the
> repository for CI config, and then runs a shell script it finds there to
> decide whether to skip the rest of the tests. The default will continue
> to run tests for all refs if that branch or script is missing.
>
> 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.
>
> There were a few design decisions that came out of on-list discussion.
> I'll summarize here:
>
> - we could use GitHub's API to retrieve the config ref, rather than a
> real checkout (and then just operate on it via some javascript). We
> still have to spin up a VM and contact GitHub over the network from
> it either way, so it ends up not being much faster. I opted to go
> with shell to keep things similar to our other tools (and really
> could implement allow-refs in any language you want). This also makes
> it easy to test your script locally, and to modify it within the
> context of a normal git.git tree.
>
> - we could keep the well-known refname out of refs/heads/ to avoid
> cluttering the branch namespace. But that makes it awkward to
> manipulate. By contrast, you can just "git checkout ci-config" to
> make changes.
>
> - we could assume the ci-config ref has nothing in it except config
> (i.e., a branch unrelated to the rest of git.git). But dealing with
> orphan branches is awkward. Instead, we'll do our best to efficiently
> check out only the ci/config directory using a shallow partial clone,
> which allows your ci-config branch to be just a normal branch, with
> your config changes on top.
>
> - we could provide a simpler interface, like a static list of ref
> patterns. But we can't get out of spinning up a whole VM anyway, so
> we might as well use that feature to make the config as flexible as
> possible. If we add more config, we should be able to reuse our
> partial-clone to set more outputs.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> .github/workflows/main.yml | 42 +++++++++++++++++++++++++++++++++++++
> ci/config/allow-refs.sample | 26 +++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
> create mode 100755 ci/config/allow-refs.sample
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index fd4df939b5..802a4bf7cd 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -6,7 +6,39 @@ env:
> DEVELOPER: 1
>
> jobs:
> + ci-config:
> + runs-on: ubuntu-latest
> + outputs:
> + enabled: ${{ steps.check-ref.outputs.enabled }}
> + steps:
> + - name: try to clone ci-config branch
> + continue-on-error: true
> + run: |
> + git -c protocol.version=2 clone \
> + --no-tags \
> + --single-branch \
> + -b ci-config \
> + --depth 1 \
> + --no-checkout \
> + --filter=blob:none \
> + https://github.com/${{ github.repository }} \
> + config-repo &&
> + cd config-repo &&
> + git checkout HEAD -- ci/config
> + - id: check-ref
> + name: check whether CI is enabled for ref
> + run: |
> + enabled=yes
> + if test -x config-repo/ci/config/allow-ref &&
> + ! config-repo/ci/config/allow-ref '${{ github.ref }}'
> + then
> + enabled=no
> + fi
> + echo "::set-output name=enabled::$enabled"
> +
> windows-build:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
One thing I wonder is whether the downstream 'windows-test' partitions.
I think that it should be fine, since we won't run the dependent
'windows-build', and then 'windows-test' won't have all of its
prerequisites filled.
> runs-on: windows-latest
> steps:
> - uses: actions/checkout@v1
> @@ -70,6 +102,8 @@ jobs:
> name: failed-tests-windows
> path: ${{env.FAILED_TEST_ARTIFACTS}}
> vs-build:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> env:
> MSYSTEM: MINGW64
> NO_PERL: 1
> @@ -154,6 +188,8 @@ jobs:
> ${{matrix.nr}} 10 t[0-9]*.sh)
> "@
> regular:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> strategy:
> matrix:
> vector:
> @@ -189,6 +225,8 @@ jobs:
> name: failed-tests-${{matrix.vector.jobname}}
> path: ${{env.FAILED_TEST_ARTIFACTS}}
> dockerized:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> strategy:
> matrix:
> vector:
> @@ -213,6 +251,8 @@ jobs:
> name: failed-tests-${{matrix.vector.jobname}}
> path: ${{env.FAILED_TEST_ARTIFACTS}}
> static-analysis:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> env:
> jobname: StaticAnalysis
> runs-on: ubuntu-latest
> @@ -221,6 +261,8 @@ jobs:
> - run: ci/install-dependencies.sh
> - run: ci/run-static-analysis.sh
> documentation:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> env:
> jobname: Documentation
> runs-on: ubuntu-latest
> diff --git a/ci/config/allow-refs.sample b/ci/config/allow-refs.sample
> new file mode 100755
> index 0000000000..f157f1945a
> --- /dev/null
> +++ b/ci/config/allow-refs.sample
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +#
> +# Sample script for enabling/disabling GitHub Actions CI runs on
> +# particular refs. By default, CI is run for all branches pushed to
> +# GitHub. You can override this by dropping the ".sample" from the script,
> +# editing it, committing, and pushing the result to the "ci-config" branch of
> +# your repository:
> +#
> +# git checkout -b ci-config
Should we be recommending '--orphan' instead of '-b' here? It looks
like when you clone this branch down that you try to get as few bytes as
possible, so I figure it may be easier to have this be a orphaned
branch.
> +# cp allow-refs.sample allow-refs
> +# $EDITOR allow-refs
> +# git commit -am "implement my ci preferences"
> +# git push
> +#
> +# This script will then be run when any refs are pushed to that repository. It
> +# gets the fully qualified refname as the first argument, and should exit with
> +# success only for refs for which you want to run CI.
> +
> +case "$1" in
> +# allow one-off tests by pushing to "for-ci" or "for-ci/mybranch"
> +refs/heads/for-ci*) true ;;
> +# always build your integration branch
> +refs/heads/my-integration-branch) true ;;
> +# don't build any other branches or tags
> +*) false ;;
> +esac
> --
> 2.26.2.1005.ge383644752
>
Thanks,
Taylor
