* [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" @ 2020-05-02 15:08 Johannes Schindelin via GitGitGadget 2020-05-03 9:36 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2020-05-02 15:08 UTC (permalink / raw) To: git; +Cc: Jeff King, Jeff Hostetler, Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> It might have been invented by Travis CI (at least it is described here: https://docs.travis-ci.com/user/customizing-the-build/#skipping-a-build) to avoid unnecessary builds: if the tip commit message (or for PR builds, the PR description) contains the needle `[skip ci]`, then the build is skipped. Unlike Azure Pipelines, GitHub workflows does not support that feature out of the box (at least not at the time of writing), but we can reinstate it manually. Note: GitHub workflows might implement this at some stage. In the least, the desire for this feature is expressed, together with some history, at https://github.community/t5/GitHub-Actions/GitHub-Actions-does-not-respect-skip-ci/m-p/42834 While it is the number five in the kudo ranking of the GitHub Actions posts, it might take a while given the current state of the world. In the meantime let's do the manual thing. Following Travis CI's example, we also add special handling to exclude the PR build when the PR title or description (or any other part of the pull request information provided by the GitHub event) contains the needle `[skip pr]`. Note: for technical reasons, a PR build will still run if a commit message contains the needle `[skip ci]` but neither PR title nor description contain the needle `[skip pr]`: for PR builds, the commit messages are not part of the payload delivered via the event. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- ci: allow skipping CI/PR builds on GitHub It was mentioned to me that it might not be totally helpful to run all the builds whenever an in-progress branch is pushed to GitHub. For example, if a contributor has dozens of topic branches in flight, they might not want to have all of them built whenever a new end-of-day push happens. This patch tries to address that, by following the convention that I believe Travis CI invented: if a commit message contains the needle [skip ci], any CI build (i.e. a build triggered by a push) is skipped. Likewise, PR builds are skipped when the PR title and/or description contains [skip pr]. Ideally, GitHub workflows will support this feature at some stage, but until then, we could have this (admittedly not very elegant) workaround. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-776%2Fdscho%2Fskip-ci-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-776/dscho/skip-ci-v1 Pull-Request: https://github.com/git/git/pull/776 .github/workflows/main.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b50..0e4a280d309 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -7,6 +7,7 @@ env: jobs: windows-build: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" runs-on: windows-latest steps: - uses: actions/checkout@v1 @@ -70,6 +71,7 @@ jobs: name: failed-tests-windows path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: MSYSTEM: MINGW64 NO_PERL: 1 @@ -154,6 +156,7 @@ jobs: ${{matrix.nr}} 10 t[0-9]*.sh) "@ regular: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" strategy: matrix: vector: @@ -189,6 +192,7 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} dockerized: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" strategy: matrix: vector: @@ -213,6 +217,7 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: jobname: StaticAnalysis runs-on: ubuntu-latest @@ -221,6 +226,7 @@ jobs: - run: ci/install-dependencies.sh - run: ci/run-static-analysis.sh documentation: + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" env: jobname: Documentation runs-on: ubuntu-latest base-commit: b34789c0b0d3b137f0bb516b417bd8d75e0cb306 -- gitgitgadget ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" 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-03 16:46 ` [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" Junio C Hamano 0 siblings, 2 replies; 62+ messages in thread From: Jeff King @ 2020-05-03 9:36 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget Cc: git, Jeff Hostetler, Johannes Schindelin On Sat, May 02, 2020 at 03:08:07PM +0000, Johannes Schindelin via GitGitGadget wrote: > It was mentioned to me that it might not be totally helpful to run all > the builds whenever an in-progress branch is pushed to GitHub. For > example, if a contributor has dozens of topic branches in flight, they > might not want to have all of them built whenever a new end-of-day push > happens. As I was probably (one of) the mentioners here, thank you for looking into this. This definitely _helps_, and if you don't want to go further I could certainly make do with it[1]. But here are some thoughts on what a more ideal solution would look like (to me, anyway). What I'd _most_ like is a separately-maintained list of branches (or branch patterns) on which to run CI. I mostly care about just testing my personal equivalent of "next" (which is really "next" plus my stable topics), and I'd probably only list that branch. Plus potentially a special CI branch that I'd use when chasing down a failure that I can't reproduce locally. But I don't think there's any good way to implement that via GitHub Actions. I thought perhaps we could pull data from a different branch in the same repo, but referring to external "Actions" seems to require a full repo name (and obviously putting git/git in that name doesn't help anybody who's trying to override something in a fork). Another alternative: could we trigger CI based on branch-names? Locally, I call my unstable branches "jk/something-wip", and I'd like to skip all of the "-wip" branches. That's basically equivalent to "[skip ci]" in that I could just amend the tip commit of the -wip branches to say "skip ci". But what if we flipped the default to _not_ build? I.e.: - continue to build all pull requests; if you opened one, you're serious enough to have other people look and should get CI feedback - build branches with a few well-known names or patterns. Maybe names like "master"? Or maybe "refs/heads/build-ci/*"? Or maybe anything in "refs/heads/<initials>/*"? We'd have to decide on a convention as a community. - do not build anything else; we have no idea if somebody in a fork is just pushing up a work in progress or not, and they may be surprised to get a CI failure notification back. I'm not sure whether we want to be building all of the individual topics in gitster/git or not. In theory that provides more information, but I'm not sure if anybody is looking at them (and all of the notifications would go to Junio anyway). My ideas aren't really developed, but I guess what I'm wondering foremost is whether other people are thinking along the same lines. > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index fd4df939b50..0e4a280d309 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -7,6 +7,7 @@ env: > > jobs: > windows-build: > + if: "!contains(toJSON(github.event.commits.*.message), '[skip ci]') && !contains(toJSON(github.event.pull_request), '[skip pr]')" It's unfortunate to have to repeat this in every job, as opposed to in the "on" event block, but I don't think that block is flexible enough to do what we want here. I don't know very much about Actions, but the code looks correct to me. -Peff [1] My current workaround is even more horrendous: I've turned off Actions entirely in peff/git, and then I separately push branches I want CI on into a separate repository. But that repo can't be a fork of git/git, because I already have one! So I now have an extra "git-ci" repo that isn't connected to anything. Yuck. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" 2020-05-03 9:36 ` Jeff King @ 2020-05-03 12:05 ` Danh Doan 2020-05-04 15:01 ` Jeff King 2020-05-03 16:46 ` [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" Junio C Hamano 1 sibling, 1 reply; 62+ messages in thread From: Danh Doan @ 2020-05-03 12:05 UTC (permalink / raw) To: Jeff King Cc: Johannes Schindelin via GitGitGadget, git, Jeff Hostetler, Johannes Schindelin On 2020-05-03 05:36:46-0400, Jeff King <peff@peff.net> wrote: > On Sat, May 02, 2020 at 03:08:07PM +0000, Johannes Schindelin via GitGitGadget wrote: > > > It was mentioned to me that it might not be totally helpful to run all > > the builds whenever an in-progress branch is pushed to GitHub. For > > example, if a contributor has dozens of topic branches in flight, they > > might not want to have all of them built whenever a new end-of-day push > > happens. > > As I was probably (one of) the mentioners here, thank you for looking > into this. This definitely _helps_, and if you don't want to go further > I could certainly make do with it[1]. > > But here are some thoughts on what a more ideal solution would look > like (to me, anyway). > > What I'd _most_ like is a separately-maintained list of branches (or > branch patterns) on which to run CI. I mostly care about just testing > my personal equivalent of "next" (which is really "next" plus my stable > topics), and I'd probably only list that branch. Plus potentially a > special CI branch that I'd use when chasing down a failure that I can't > reproduce locally. > > But I don't think there's any good way to implement that via GitHub > Actions. I thought perhaps we could pull data from a different branch in > the same repo, but referring to external "Actions" seems to require a > full repo name (and obviously putting git/git in that name doesn't help > anybody who's trying to override something in a fork). > > Another alternative: could we trigger CI based on branch-names? Locally, > I call my unstable branches "jk/something-wip", and I'd like to skip all > of the "-wip" branches. That's basically equivalent to "[skip ci]" in > that I could just amend the tip commit of the -wip branches to say "skip > ci". But what if we flipped the default to _not_ build? I.e.: > > - continue to build all pull requests; if you opened one, you're > serious enough to have other people look and should get CI feedback > > - build branches with a few well-known names or patterns. Maybe > names like "master"? Or maybe "refs/heads/build-ci/*"? Or maybe > anything in "refs/heads/<initials>/*"? We'd have to decide on a > convention as a community. I think this is better approach for Junio, and we can help people opt in by asking them to push into "for-ci/<some-name>. This is my proposal for it: ------------------8<------------- From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= <congdanhqx@gmail.com> Date: Sun, 3 May 2020 18:56:50 +0700 Subject: [PATCH] CI: GitHub: limit GitHub Action to intergration branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b5..303393ba73 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,19 @@ name: CI/PR -on: [push, pull_request] +on: + pull_request: + branches: + - '*' + push: + branches: + - maint + - master + - next + - jch + - pu + - 'for-ci/**' + tags: + - '*' env: DEVELOPER: 1 -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" 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 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-04 15:01 UTC (permalink / raw) To: Danh Doan Cc: Johannes Schindelin via GitGitGadget, git, Jeff Hostetler, Johannes Schindelin On Sun, May 03, 2020 at 07:05:46PM +0700, Danh Doan wrote: > This is my proposal for it: > [...] > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index fd4df939b5..303393ba73 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -1,6 +1,19 @@ > name: CI/PR > > -on: [push, pull_request] > +on: > + pull_request: > + branches: > + - '*' > + push: > + branches: > + - maint > + - master > + - next > + - jch > + - pu > + - 'for-ci/**' > + tags: > + - '*' Yeah, this seems quite reasonable to me. I'd just add a refspec to push a duplicate of branches I'm interested in running CI on to for-ci. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v2 0/2] Limit GitHub Actions to designated branches 2020-05-04 15:01 ` Jeff King @ 2020-05-04 15:49 ` Đ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 ` (2 more replies) 0 siblings, 3 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-04 15:49 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King It seems like there's at least one contributor like this proposal. > Yeah, this seems quite reasonable to me. I'd just add a refspec to push > a duplicate of branches I'm interested in running CI on to for-ci. I've tweaked the commit message for the proposal and added a new patch to advertise GitHub Actions to our friends. With this series applied, we will trigger GitHub Actions CI for those branches only: - 5 integration branches: maint, master, next, jch, and pu - Contributors' opted-in branches: 'for-ci/**' Đoàn Trần Công Danh (2): CI: limit GitHub Actions to designated branches SubmittingPatches: advertise GitHub Actions CI .github/workflows/main.yml | 14 +++++++++++++- Documentation/SubmittingPatches | 5 +++++ 2 files changed, 18 insertions(+), 1 deletion(-) Range-diff against v1: 1: 106620b2d8 ! 1: 73de97dfeb CI: GitHub: limit GitHub Action to intergration branches @@ Metadata Author: Đoàn Trần Công Danh <congdanhqx@gmail.com> ## Commit message ## - CI: GitHub: limit GitHub Action to intergration branches + CI: limit GitHub Actions to designated branches + + Git's maintainer usually don't have enough time to debug the failure of + invidual feature branch, they usually want to look at intergration + branches. + + Contributors now can have GitHub Actions as an opt-in option, + should they want to check their code, they will push into designated + branch. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> @@ .github/workflows/main.yml +on: + pull_request: + branches: -+ - '*' ++ - '**' + push: + branches: + - maint -: ---------- > 2: 24a8fefe5a SubmittingPatches: advertise GitHub Actions CI -- 2.26.2.672.g232c24e857 ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 ` Đoàn Trần Công Danh 2020-05-04 16:23 ` Jeff King 2020-05-04 15:49 ` [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI Đ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 2 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-04 15:49 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King Git's maintainer usually don't have enough time to debug the failure of invidual feature branch, they usually want to look at intergration branches. Contributors now can have GitHub Actions as an opt-in option, should they want to check their code, they will push into designated branch. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b5..ea43b03092 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,18 @@ name: CI/PR -on: [push, pull_request] +on: + pull_request: + branches: + - '**' + push: + branches: + - maint + - master + - next + - jch + - pu + tags: + - '*' env: DEVELOPER: 1 -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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-05 0:34 ` [PATCH v2 1/2] CI: limit GitHub Actions to designated branches Đoàn Trần Công Danh 0 siblings, 2 replies; 62+ messages in thread From: Jeff King @ 2020-05-04 16:23 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 10:49:31PM +0700, Đoàn Trần Công Danh wrote: > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index fd4df939b5..ea43b03092 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -1,6 +1,18 @@ > name: CI/PR > > -on: [push, pull_request] > +on: > + pull_request: > + branches: > + - '**' Doing "**" here makes sense to catch everything (it would be even better if we could just say "everything with a pull request" by omitting the branch filter entirely, but maybe that's not possible). > + tags: > + - '*' Would we want that here, too? I guess nobody is likely to push "foo/v1.2.3". Or on the flip side, would we want to tighten this? If I push a tag "wip", I probably don't want it built. Probably the right rule is "annotated tags only", but I suspect that's not possible. > + push: > + branches: > + - maint > + - master > + - next > + - jch > + - pu What happened to "for-ci" (presumably "for-ci/**")? -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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:36 ` Jeff King 2020-05-05 0:34 ` [PATCH v2 1/2] CI: limit GitHub Actions to designated branches Đoàn Trần Công Danh 1 sibling, 2 replies; 62+ messages in thread From: Taylor Blau @ 2020-05-04 21:58 UTC (permalink / raw) To: Jeff King Cc: Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 12:23:11PM -0400, Jeff King wrote: > On Mon, May 04, 2020 at 10:49:31PM +0700, Đoàn Trần Công Danh wrote: > > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > > index fd4df939b5..ea43b03092 100644 > > --- a/.github/workflows/main.yml > > +++ b/.github/workflows/main.yml > > @@ -1,6 +1,18 @@ > > name: CI/PR > > > > -on: [push, pull_request] > > +on: > > + pull_request: > > + branches: > > + - '**' > > Doing "**" here makes sense to catch everything (it would be even better > if we could just say "everything with a pull request" by omitting the > branch filter entirely, but maybe that's not possible). > > > + tags: > > + - '*' > > Would we want that here, too? I guess nobody is likely to push > "foo/v1.2.3". > > Or on the flip side, would we want to tighten this? If I push a tag > "wip", I probably don't want it built. Probably the right rule is > "annotated tags only", but I suspect that's not possible. > > > + push: > > + branches: > > + - maint > > + - master > > + - next > > + - jch > > + - pu > > What happened to "for-ci" (presumably "for-ci/**")? Huh; I'm not sure that I'm sold on the idea of a 'for-ci' namespace here. In addition to running 'make test' on patches locally before I send them, I find it tremendously convenient for GitHub to run them for me when I push 'tb/' branches up to 'ttaylorr/git'. So, while the above is more-or-less what I'd expect the monitored list of branches to look like (at least, ignoring the missing 'for-ci/**' bits), I wish that I could also build every branch that I push up to my fork. Of course, I don't want to maintain a one-patch difference between ttaylorr/git@master and git/git@master, so I wonder if we could get a little more creative with these rules and actually run Actions on *every* branch, but introduce a new first step which stops the rest of the actions run (so that in practice we're not running CI on non-integration branches in Junio's tree). I figure that we need something more flexible than the 'push.branches' list, but I'd be very curious to hear if something like what I'm describing is possible. > -Peff Thanks, Taylor ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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:36 ` Jeff King 1 sibling, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-04 22:52 UTC (permalink / raw) To: Taylor Blau Cc: Jeff King, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Taylor Blau <me@ttaylorr.com> writes: > Huh; I'm not sure that I'm sold on the idea of a 'for-ci' namespace > here. In addition to running 'make test' on patches locally before I > send them, I find it tremendously convenient for GitHub to run them for > me when I push 'tb/' branches up to 'ttaylorr/git'. > > So, while the above is more-or-less what I'd expect the monitored list > of branches to look like (at least, ignoring the missing 'for-ci/**' > bits), I wish that I could also build every branch that I push up to my > fork. > > Of course, I don't want to maintain a one-patch difference between > ttaylorr/git@master and git/git@master, so I wonder if we could get a > little more creative with these rules and actually run Actions on > *every* branch, but introduce a new first step which stops the rest of > the actions run (so that in practice we're not running CI on > non-integration branches in Junio's tree). Hmph, what are we trying to avoid by using the for-ci/ convention? If this is only a reaction to what I said earlier (i.e. "building everything in github.com/gitster/git/ has no value to me"), then I suspect it may be an over-engineered solution to a problem that does not exist, and harms people like you. I could just go there and turn off GitHub Actions for that repository instead. Or are there more issues being addressed with the "testing branches are opt-in, unless a pull request against git/git explicitly says it is ready to be tested" approach? ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 22:52 ` Junio C Hamano @ 2020-05-04 23:15 ` Taylor Blau 2020-05-04 23:35 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Taylor Blau @ 2020-05-04 23:15 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Jeff King, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 03:52:44PM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > Huh; I'm not sure that I'm sold on the idea of a 'for-ci' namespace > > here. In addition to running 'make test' on patches locally before I > > send them, I find it tremendously convenient for GitHub to run them for > > me when I push 'tb/' branches up to 'ttaylorr/git'. > > > > So, while the above is more-or-less what I'd expect the monitored list > > of branches to look like (at least, ignoring the missing 'for-ci/**' > > bits), I wish that I could also build every branch that I push up to my > > fork. > > > > Of course, I don't want to maintain a one-patch difference between > > ttaylorr/git@master and git/git@master, so I wonder if we could get a > > little more creative with these rules and actually run Actions on > > *every* branch, but introduce a new first step which stops the rest of > > the actions run (so that in practice we're not running CI on > > non-integration branches in Junio's tree). > > Hmph, what are we trying to avoid by using the for-ci/ convention? > > If this is only a reaction to what I said earlier (i.e. "building > everything in github.com/gitster/git/ has no value to me"), then I > suspect it may be an over-engineered solution to a problem that does > not exist, and harms people like you. I could just go there and > turn off GitHub Actions for that repository instead. It is a reaction to that, yes. It would be nice to have a middle-ground where you can run Actions on the official integration branches, but contributors such as myself run Actions on *every* branch of their fork. It does feel over-engineered, yes. I would not be surprised if Actions supports something like this more directly, and I just don't know about it. > Or are there more issues being addressed with the "testing branches > are opt-in, unless a pull request against git/git explicitly says it > is ready to be tested" approach? Thanks, Taylor ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 23:15 ` Taylor Blau @ 2020-05-04 23:35 ` Jeff King 2020-05-05 0:24 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-04 23:35 UTC (permalink / raw) To: Taylor Blau Cc: Junio C Hamano, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 05:15:44PM -0600, Taylor Blau wrote: > > If this is only a reaction to what I said earlier (i.e. "building > > everything in github.com/gitster/git/ has no value to me"), then I > > suspect it may be an over-engineered solution to a problem that does > > not exist, and harms people like you. I could just go there and > > turn off GitHub Actions for that repository instead. > > It is a reaction to that, yes. It would be nice to have a middle-ground > where you can run Actions on the official integration branches, but > contributors such as myself run Actions on *every* branch of their fork. The problem is that there is no way for people to _not_ run on every branch of their fork, then. It is either every branch or no branches. By choosing a prefix, it gives people the option to choose. But it does place the burden on them of adding an extra refspec: refs/heads/*:refs/heads/for-ci/* or similar. I don't like it, but I don't see any other way to let people choose some branches but not others (that doesn't involve a one-line yaml change at the beginning of every branch, which is an even higher burden). -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 23:35 ` Jeff King @ 2020-05-05 0:24 ` Junio C Hamano 0 siblings, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 0:24 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > On Mon, May 04, 2020 at 05:15:44PM -0600, Taylor Blau wrote: > >> > If this is only a reaction to what I said earlier (i.e. "building >> > everything in github.com/gitster/git/ has no value to me"), then I >> > suspect it may be an over-engineered solution to a problem that does >> > not exist, and harms people like you. I could just go there and >> > turn off GitHub Actions for that repository instead. >> >> It is a reaction to that, yes. It would be nice to have a middle-ground >> where you can run Actions on the official integration branches, but >> contributors such as myself run Actions on *every* branch of their fork. > > The problem is that there is no way for people to _not_ run on every > branch of their fork, then. It is either every branch or no branches. OK. > .... I don't like it, but I don't see any other way to let people > choose some branches but not others (that doesn't involve a one-line > yaml change at the beginning of every branch, which is an even higher > burden). True, and having to mark a commit object with "do not run CI on this", which is what I heard Travis does, is even worse. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 21:58 ` Taylor Blau 2020-05-04 22:52 ` Junio C Hamano @ 2020-05-04 23:36 ` Jeff King 2020-05-05 0:20 ` Taylor Blau 1 sibling, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-04 23:36 UTC (permalink / raw) To: Taylor Blau Cc: Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 03:58:24PM -0600, Taylor Blau wrote: > Huh; I'm not sure that I'm sold on the idea of a 'for-ci' namespace > here. In addition to running 'make test' on patches locally before I > send them, I find it tremendously convenient for GitHub to run them for > me when I push 'tb/' branches up to 'ttaylorr/git'. > > So, while the above is more-or-less what I'd expect the monitored list > of branches to look like (at least, ignoring the missing 'for-ci/**' > bits), I wish that I could also build every branch that I push up to my > fork. > > Of course, I don't want to maintain a one-patch difference between > ttaylorr/git@master and git/git@master, so I wonder if we could get a > little more creative with these rules and actually run Actions on > *every* branch, but introduce a new first step which stops the rest of > the actions run (so that in practice we're not running CI on > non-integration branches in Junio's tree). I don't understand what that would accomplish. If we ran the actions on every branch but stopped the run, then you wouldn't get the CI results you want. What am I missing? -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 23:36 ` Jeff King @ 2020-05-05 0:20 ` Taylor Blau 2020-05-05 16:43 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Taylor Blau @ 2020-05-05 0:20 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 07:36:34PM -0400, Jeff King wrote: > On Mon, May 04, 2020 at 03:58:24PM -0600, Taylor Blau wrote: > > > Huh; I'm not sure that I'm sold on the idea of a 'for-ci' namespace > > here. In addition to running 'make test' on patches locally before I > > send them, I find it tremendously convenient for GitHub to run them for > > me when I push 'tb/' branches up to 'ttaylorr/git'. > > > > So, while the above is more-or-less what I'd expect the monitored list > > of branches to look like (at least, ignoring the missing 'for-ci/**' > > bits), I wish that I could also build every branch that I push up to my > > fork. > > > > Of course, I don't want to maintain a one-patch difference between > > ttaylorr/git@master and git/git@master, so I wonder if we could get a > > little more creative with these rules and actually run Actions on > > *every* branch, but introduce a new first step which stops the rest of > > the actions run (so that in practice we're not running CI on > > non-integration branches in Junio's tree). > > I don't understand what that would accomplish. If we ran the actions on > every branch but stopped the run, then you wouldn't get the CI results > you want. What am I missing? That on forks of git/git we *would't* stop the run for non-integration branches, i.e., that we'd have something like: * Actions is running on all branches, of all forks, all the time. * If a branch is pushed to git/git, and it's not an integration branch, the run is aborted early. * If a branch is pushed to a fork of git/git, it's allowed to run no matter what. Maybe it's an over-engineered solution to a problem that we don't have, though. Junio seems happy to simply disable Actions on git/git. OTOH, I think it would be OK to skip the second bullet point, and have Junio disable Actions notifications. /shrug > -Peff Thanks, Taylor ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 0:20 ` Taylor Blau @ 2020-05-05 16:43 ` Jeff King 2020-05-05 17:57 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-05 16:43 UTC (permalink / raw) To: Taylor Blau Cc: Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Mon, May 04, 2020 at 06:20:55PM -0600, Taylor Blau wrote: > > > Of course, I don't want to maintain a one-patch difference between > > > ttaylorr/git@master and git/git@master, so I wonder if we could get a > > > little more creative with these rules and actually run Actions on > > > *every* branch, but introduce a new first step which stops the rest of > > > the actions run (so that in practice we're not running CI on > > > non-integration branches in Junio's tree). > > > > I don't understand what that would accomplish. If we ran the actions on > > every branch but stopped the run, then you wouldn't get the CI results > > you want. What am I missing? > > That on forks of git/git we *would't* stop the run for non-integration > branches, i.e., that we'd have something like: Ah, so the first step is "if we are gitster/git, then do not run", not "do not run". But that is missing the point of the exercise, no? The question of what gitster/git should do was a side conversation. The purpose of Dscho's original patch and Danh's followup was to allow anybody to choose which branches in their own fork. I.e.: > * Actions is running on all branches, of all forks, all the time. This is how it works now, and is the exact thing we are trying to fix. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 16:43 ` Jeff King @ 2020-05-05 17:57 ` Junio C Hamano 2020-05-05 18:24 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 17:57 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > But that is missing the point of the exercise, no? The question of what > gitster/git should do was a side conversation. The purpose of Dscho's > original patch and Danh's followup was to allow anybody to choose which > branches in their own fork. I.e.: > >> * Actions is running on all branches, of all forks, all the time. > > This is how it works now, and is the exact thing we are trying to fix. Thanks for clarifying and refocusing the discussion. I am onboard. It seems to me that there are only two and half approaches, then: - Branch-build is opt-in; only branches that match selected, known, and fixed patterns will be built (e.g. 'maint', 'maint-*', 'master', 'next', 'pu', and 'for-ci/*'). - Branch-build is opt-out; branches that match selected, known, and fixed patterns will be excluded (e.g., '*-wip'). - If you do not want your branches want to be skipped, you need to tweak the commit at the tip (e.g. mark with '[skip ci]' log message, munging .github/workflows/ in the tree). The last one is only there for completeness. I do not think mucking with the objects recorded in the history, whether it is a tweaked log message or tweaked tree contents, is a good way to do this. More random ideas... Would it be too much hassle to use notes for a thing like this? Perhaps push out with refs/notes/skip-ci note attached to a commit you do not want to be built? I have a feeling that it gives way overkill flexibility with little gain (probably too cumbersome to manage). Does push into GitHub repository offer an ability to pass arbitrary push option, to which actions that trigger "on: push" event can react? ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 17:57 ` Junio C Hamano @ 2020-05-05 18:24 ` Jeff King 2020-05-05 21:04 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-05 18:24 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Tue, May 05, 2020 at 10:57:39AM -0700, Junio C Hamano wrote: > Would it be too much hassle to use notes for a thing like this? > Perhaps push out with refs/notes/skip-ci note attached to a commit > you do not want to be built? I have a feeling that it gives way > overkill flexibility with little gain (probably too cumbersome to > manage). I think using notes would be a hassle. This config is really associated with a branch, not a particular config (so you'd have to make sure they propagate across rebases, etc). 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. Really all we want is some kind of per-repo variable storage where the values aren't baked into the tree. There is a "secrets" system that can be used for this, though it kind of feels like an abuse of the concept. > Does push into GitHub repository offer an ability to pass arbitrary > push option, to which actions that trigger "on: push" event can > react? No, I don't think so. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 18:24 ` Jeff King @ 2020-05-05 21:04 ` Jeff King 2020-05-05 21:29 ` Junio C Hamano ` (2 more replies) 0 siblings, 3 replies; 62+ messages in thread From: Jeff King @ 2020-05-05 21:04 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 21:04 ` Jeff King @ 2020-05-05 21:29 ` Junio C Hamano 2020-05-05 21:58 ` Jeff King 2020-05-06 15:09 ` Johannes Schindelin 2020-05-06 0:46 ` Đoàn Trần Công Danh 2020-05-07 16:20 ` [PATCH v2] ci: allow per-branch config for GitHub Actions Jeff King 2 siblings, 2 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 21:29 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > 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. ;) That sounds good. > 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). OK. > 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. Nice---that way, all existing jobs do not even need to know about the special controlling ref. > 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). So there is no wildcard? Not really complaining, but am wondering. > We could easily add in a > blacklist, as well. OK. > 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. I presume that you are saying "checking out the tree of refs/ci/config and there is a program allow-ref that can tell which one to run ci on"? > 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. Probably. > 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 Oh, quite nice. Simple and clean. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 1 sibling, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-05 21:58 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Tue, May 05, 2020 at 02:29:09PM -0700, Junio C Hamano wrote: > > 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. > > Nice---that way, all existing jobs do not even need to know about > the special controlling ref. Yep. Also, it only has to run once. It's not too expensive, but it does seem to take a few seconds to run. I think the VM setup is usually pretty quick due to the way Actions is implemented. I made sure we avoid cloning the whole normal git.git tree, but we still have to clone the individual config branch. It seems to take ~3-5s to run that job. I'd have liked to make it even cheaper for branches which don't want CI (less for latency, but more just because I hate to waste GitHub's resources), but I think that's the best we can do. And for cases where we actually run CI, 3s is a drop in the bucket. ;) > > 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). > > So there is no wildcard? Not really complaining, but am wondering. Correct, there's no wildcard. It would be easy to add one, now that this gets us to a working shell environment with the refname available. And probably worth doing if we don't just go the "execute a program from the config tree" route. > > 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. > > I presume that you are saying "checking out the tree of refs/ci/config > and there is a program allow-ref that can tell which one to run ci on"? Yes, exactly. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 21:58 ` Jeff King @ 2020-05-05 22:28 ` Junio C Hamano 0 siblings, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 22:28 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: >> > 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. >> >> I presume that you are saying "checking out the tree of refs/ci/config >> and there is a program allow-ref that can tell which one to run ci on"? > > Yes, exactly. I guess a simple example implementation of allow-ref script, with a bit of instruction to tell people how to initialize a separate and otherwise empty repository with just a (possibly customized) copy of the script in it, and push its 'master' branch to refs/ci/config of their Git fork, would be a way to go, then? Sounds simple enough. Cycles to spin up a VM that adds 3 seconds latency, only to know that a branch won't need to be built, does sound like a bit unfortunate, though. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 21:29 ` Junio C Hamano 2020-05-05 21:58 ` Jeff King @ 2020-05-06 15:09 ` Johannes Schindelin 2020-05-06 16:26 ` Junio C Hamano 2020-05-07 12:01 ` Đoàn Trần Công Danh 1 sibling, 2 replies; 62+ messages in thread From: Johannes Schindelin @ 2020-05-06 15:09 UTC (permalink / raw) To: Junio C Hamano Cc: Jeff King, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler Hi, On Tue, 5 May 2020, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > 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 > > Oh, quite nice. Simple and clean. The idea is indeed very neat. I think we can do a bit better with resource usage by not even bothering to check this branch out. Something along those lines (sorry, I really would love to have the time to test this...): - id: check-ref name: check whether CI is enabled for ref uses: actions/github-script@0.9.0 with: script: | const req = { owner: context.repo.owner, repo: context.repo.repo, ref: "ci/config" }; try { req.tree_sha = (await github.git.getRef(req)).data.object.sha; (await github.git.getTree(req)) .tree.filter(e => e.path == 'ref-whitelist').map(e => { req.file_sha = e.sha; }); const list = Buffer.from((await github.git.getBlob(req)).data.content, 'base64').toString('UTF-8'); core.setOutput('enabled', `\n${list}`.indexOf(`\n${{github.ref}}\n`) < 0 ? 'no' : 'yes'); } catch (e) { core.setOutput('enabled', 'yes'); } Ciao, Dscho ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 12:01 ` Đoàn Trần Công Danh 1 sibling, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-06 16:26 UTC (permalink / raw) To: Johannes Schindelin Cc: Jeff King, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > The idea is indeed very neat. I think we can do a bit better with resource > usage by not even bothering to check this branch out. Something along > those lines (sorry, I really would love to have the time to test this...): Nice. I view the latest round of Peff's idea "allow-ref" as "because we are spinning a VM anyway, why not just run an end-user supplied script and let it decide?" and not as "we must have a Turing complete flexibility so let's run a script in a VM". In other words, it may be overkill and we may strike a better tradeoff by living with reduced flexibility if there is a way to avoid the associated cost. But doesn't this (i.e. uses: actions/github-script) still pay the cost of spinning up a VM? How expensive is it to check out a small tree with a single file, whether it is ref-whitelist or allow-ref? If the cost to check out a tree is dwarfed, it probably is not worth pursuing the approach to use a canned "don't customize, just use" logic in the workflow file to read a configuration file that can only list full names of allowed refs. Thanks. > - id: check-ref > name: check whether CI is enabled for ref > uses: actions/github-script@0.9.0 > with: > script: | > const req = { > owner: context.repo.owner, > repo: context.repo.repo, > ref: "ci/config" > }; > > try { > req.tree_sha = (await github.git.getRef(req)).data.object.sha; > (await github.git.getTree(req)) > .tree.filter(e => e.path == 'ref-whitelist').map(e => { > req.file_sha = e.sha; > }); > const list = Buffer.from((await github.git.getBlob(req)).data.content, 'base64').toString('UTF-8'); > core.setOutput('enabled', `\n${list}`.indexOf(`\n${{github.ref}}\n`) < 0 ? 'no' : 'yes'); > } catch (e) { > core.setOutput('enabled', 'yes'); > } > > Ciao, > Dscho ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-06 16:26 ` Junio C Hamano @ 2020-05-07 12:17 ` Jeff King 2020-05-07 14:02 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-07 12:17 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler On Wed, May 06, 2020 at 09:26:53AM -0700, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > The idea is indeed very neat. I think we can do a bit better with resource > > usage by not even bothering to check this branch out. Something along > > those lines (sorry, I really would love to have the time to test this...): > > Nice. I view the latest round of Peff's idea "allow-ref" as > "because we are spinning a VM anyway, why not just run an end-user > supplied script and let it decide?" and not as "we must have a > Turing complete flexibility so let's run a script in a VM". In > other words, it may be overkill and we may strike a better tradeoff > by living with reduced flexibility if there is a way to avoid the > associated cost. The script is just javascript, which has an eval(). We could probably let refs/ci/config:allow-ref be a snippet of javascript that we run. I do prefer the VM environment to snippets of javascript for a few reasons: - shell scripts match the rest of our Git toolbox (though that's a personal preference, and for the amount of code we're talking about even _I_ can do a little javascript) - it's easy to test your code locally by running "./allow-ref foo". Whereas emulating the environment in which those scriptlets are run is much trickier (e.g., you need a working github-api object) That said, if this is more lightweight, I think it's worth exploring. > But doesn't this (i.e. uses: actions/github-script) still pay the > cost of spinning up a VM? How expensive is it to check out a small > tree with a single file, whether it is ref-whitelist or allow-ref? I suspect this script mechanism may be much cheaper. I don't know the implementation details, but spinning up a nodejs container to run a javascript snippet should be much cheaper than a full ubuntu VM running "git clone" (the clone itself should be super cheap because it's a shallow single-branch clone of a tree with one file in it, but getting there is relatively heavy-weight). But I could be wrong. It's clear that they're not spinning up a full ubuntu vm from scratch to serve the requests (since it happens in 3s). I'll see if I can get something working and do some timings. The latency isn't incredibly important. This is all happening async, and either we'll skip CI (in which case you don't care how long it takes, since you're not waiting on the result) or CI will take many minutes, in which case 3s is nothing. But I would like to be respectful of the CPU spent running Actions and be as lightweight as possible. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 12:17 ` Jeff King @ 2020-05-07 14:02 ` Jeff King 2020-05-07 18:17 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-07 14:02 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Schindelin, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler On Thu, May 07, 2020 at 08:17:27AM -0400, Jeff King wrote: > > But doesn't this (i.e. uses: actions/github-script) still pay the > > cost of spinning up a VM? How expensive is it to check out a small > > tree with a single file, whether it is ref-whitelist or allow-ref? > > I suspect this script mechanism may be much cheaper. I don't know the > implementation details, but spinning up a nodejs container to run a > javascript snippet should be much cheaper than a full ubuntu VM running > "git clone" (the clone itself should be super cheap because it's a > shallow single-branch clone of a tree with one file in it, but getting > there is relatively heavy-weight). Sorry, this is all complete nonsense. There is no magical nodejs container in Actions. You still have to say "runs-on: ubuntu-latest". So it's still spinning up that VM and then running inside there. I just did a timing with three jobs: noop: runs-on: ubuntu-latest steps: - run: exit 0 script: runs-on: ubuntu-latest steps: - uses: actions/github-script@0.9.0 with: script: | const req = { owner: context.repo.owner, repo: context.repo.repo, ref: "refs/ci/config" }; try { req.tree_sha = (await github.git.getRef(req)).data.object.sha; (await github.git.getTree(req)) .tree.filter(e => e.path == 'ref-whitelist').map(e => { req.file_sha = e.sha; }); const list = Buffer.from((await github.git.getBlob(req)).data.content, 'base64').toString('UTF-8'); core.setOutput('enabled', `\n${list}`.indexOf(`\n${{github.ref}}\n`) < 0 ? 'no' : 'yes'); } catch (e) { core.setOutput('enabled', 'yes'); } checkout: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 continue-on-error: true with: ref: refs/ci/config - run: ./allow-ref ${{ github.ref }} and they took 1, 2, and 3 seconds respectively. They spend 2s getting the environment set up and the actions loaded. So the API one spent less than 1s on the network, but the single-file checkout spent slightly more. Given the timing variations I've seen, I wouldn't be surprised if it sometimes goes the other way. But even if those numbers are accurate, I don't think the cost difference is enough to force our hand either way. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 14:02 ` Jeff King @ 2020-05-07 18:17 ` Junio C Hamano 0 siblings, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-07 18:17 UTC (permalink / raw) To: Jeff King Cc: Johannes Schindelin, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler Jeff King <peff@peff.net> writes: > On Thu, May 07, 2020 at 08:17:27AM -0400, Jeff King wrote: > >> > But doesn't this (i.e. uses: actions/github-script) still pay the >> > cost of spinning up a VM? How expensive is it to check out a small >> > tree with a single file, whether it is ref-whitelist or allow-ref? >> >> I suspect this script mechanism may be much cheaper. I don't know the >> implementation details, but spinning up a nodejs container to run a >> javascript snippet should be much cheaper than a full ubuntu VM running >> "git clone" (the clone itself should be super cheap because it's a >> shallow single-branch clone of a tree with one file in it, but getting >> there is relatively heavy-weight). > > Sorry, this is all complete nonsense. There is no magical nodejs > container in Actions. You still have to say "runs-on: ubuntu-latest". So > it's still spinning up that VM and then running inside there. Ah, I did see "runs-on: ubuntu-latest" in the tutorial for the node thing, and was very much dissapointed, before I sent that "don't you still spin up a VM anyway?" response. Glad to know that I wasn't totally misreading the documentation, and unhappy that there wasn't a magic bullet after all X-<. > and they took 1, 2, and 3 seconds respectively. They spend 2s getting > the environment set up and the actions loaded. So the API one spent less > than 1s on the network, but the single-file checkout spent slightly > more. Given the timing variations I've seen, I wouldn't be surprised if > it sometimes goes the other way. But even if those numbers are accurate, > I don't think the cost difference is enough to force our hand either > way. Yup, the above tempts me to say "because we are spinning a VM anyway, why not just run an end-user supplied script and let it decide?" would be the best approach. Unless somebody finds a magic bullet, that is, but unfortunately the nodejs one does not seem to be one. Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-06 15:09 ` Johannes Schindelin 2020-05-06 16:26 ` 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 1 sibling, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-07 12:01 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Jeff King, Taylor Blau, git, Jeff Hostetler On 2020-05-06 17:09:39+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > The idea is indeed very neat. I think we can do a bit better with resource > usage by not even bothering to check this branch out. Something along > those lines (sorry, I really would love to have the time to test this...): While this can avoid the cost of checking out a whole branch (which can be mitigated by using an orphan branch with single file), This still spins up an VM, and actions/github-script run (I think) nodejs, which is more resource intensive than git and sh script. Above statement maybe wrong, I'm not interacting much with nodejs. > - id: check-ref > name: check whether CI is enabled for ref > uses: actions/github-script@0.9.0 > with: > script: | > const req = { > owner: context.repo.owner, > repo: context.repo.repo, > ref: "ci/config" > }; > > try { > req.tree_sha = (await github.git.getRef(req)).data.object.sha; > (await github.git.getTree(req)) > .tree.filter(e => e.path == 'ref-whitelist').map(e => { > req.file_sha = e.sha; > }); > const list = Buffer.from((await github.git.getBlob(req)).data.content, 'base64').toString('UTF-8'); > core.setOutput('enabled', `\n${list}`.indexOf(`\n${{github.ref}}\n`) < 0 ? 'no' : 'yes'); And this `indexOf` will check if our ref (exact) matchs (full line) with some white-list list, which is very limited. So people couldn't match by some pattern (grep can work). I haven't tested, but we may use part of above script to read a single file from a ref, and add another steps for "grep"/"sh" I'm not sure if that script will cost more resources than git-checkout or not. And is that solutions over-engineered? -- Danh ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 12:01 ` Đoàn Trần Công Danh @ 2020-05-07 12:47 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-07 12:47 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Jeff King, Taylor Blau, git, Jeff Hostetler On 2020-05-07 19:01:02+0700, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote: > On 2020-05-06 17:09:39+0200, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > The idea is indeed very neat. I think we can do a bit better with resource > > usage by not even bothering to check this branch out. Something along > > those lines (sorry, I really would love to have the time to test this...): > > While this can avoid the cost of checking out a whole branch (which > can be mitigated by using an orphan branch with single file), > > This still spins up an VM, and actions/github-script run (I think) > nodejs, which is more resource intensive than git and sh script. > Above statement maybe wrong, I'm not interacting much with nodejs. I was wrong, actions/checkout is also using nodejs, So, this actions/github-script actual reduces the total time for fetching the file ref-whitelist/ref-blacklist/allow-ref > > - id: check-ref > > name: check whether CI is enabled for ref > > uses: actions/github-script@0.9.0 > > with: > > script: | > > const req = { > > owner: context.repo.owner, > > repo: context.repo.repo, > > ref: "ci/config" > > }; > > > > try { > > req.tree_sha = (await github.git.getRef(req)).data.object.sha; > > (await github.git.getTree(req)) > > .tree.filter(e => e.path == 'ref-whitelist').map(e => { > > req.file_sha = e.sha; > > }); > > const list = Buffer.from((await github.git.getBlob(req)).data.content, 'base64').toString('UTF-8'); > > core.setOutput('enabled', `\n${list}`.indexOf(`\n${{github.ref}}\n`) < 0 ? 'no' : 'yes'); > > And this `indexOf` will check if our ref (exact) matchs (full line) > with some white-list list, which is very limited. > So people couldn't match by some pattern (grep can work). > > I haven't tested, but we may use part of above script to read a single > file from a ref, and add another steps for "grep"/"sh" > I'm not sure if that script will cost more resources than git-checkout > or not. And is that solutions over-engineered? But this point still hold, now, I think using part of above script to read the file, and allow more custom logic in a separated steps maybe better solutions. -- Danh ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-05 21:04 ` Jeff King 2020-05-05 21:29 ` Junio C Hamano @ 2020-05-06 0:46 ` Đoàn Trần Công Danh 2020-05-06 3:56 ` Junio C Hamano 2020-05-07 16:20 ` [PATCH v2] ci: allow per-branch config for GitHub Actions Jeff King 2 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-06 0:46 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin On 2020-05-05 17:04:51-0400, Jeff King <peff@peff.net> wrote: > 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. How about supports the best of both worlds. Let's say support wildcard 'wip/**' for opt-out. And use "./allow-ref" to filter everything that passed the wildcard. > 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. Should we go with this route, here is a fix-up patch for your, (after cherry-pick my [1/3]) --------------------8<------------------- Subject: [PATCH] fixup! ci: allow per-branch config for GitHub Actions Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 3 +-- Documentation/SubmittingPatches | 12 ++++++++++++ contrib/ci-config-allow-ref | 9 +++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100755 contrib/ci-config-allow-ref diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 51f4ff6e89..08217c5ed8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,8 +19,7 @@ jobs: name: check whether CI is enabled for ref run: | enabled=yes - if test -e ref-whitelist && - ! grep '^${{ github.ref }}$' ref-whitelist + if test -x allow-ref && ! ./allow-ref '${{ github.ref }}' then enabled=no fi diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 8686318550..8175424929 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -82,6 +82,18 @@ Alternately, you can use GitHub Actions (which supports testing your changes on Linux, macOS, and Windows) by pushing into a branch in your fork or opening a GitHub's Pull Request against https://github.com/git/git.git or a fork of that repository. +In the event that you only want to trigger GitHub Actions for specific +refname, you can create an executable file named `allow-ref` in +`refs/ci/config`. Those below steps may help you: +-------------- +$ git checkout --orphan ci-config +$ cp contrib/ci-config-allow-ref allow-ref +$ $EDITOR allow-ref +$ git rm -rf . +$ git commit allow-ref +$ git push <your-fork> HEAD:refs/ci/config +-------------- + Do not forget to update the documentation to describe the updated behavior and make sure that the resulting documentation set formats diff --git a/contrib/ci-config-allow-ref b/contrib/ci-config-allow-ref new file mode 100755 index 0000000000..b53e9ddbd0 --- /dev/null +++ b/contrib/ci-config-allow-ref @@ -0,0 +1,9 @@ +#!/bin/sh +# Sample filter for GitHub Actions +# GitHub Actions will run if and only if this script exit with zero status + +REFNAME="$1" + +case "$REFNAME" in +refs/heads/no-ci*) exit 1 ;; +esac -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-06 3:56 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: Jeff King, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches > index 8686318550..8175424929 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -82,6 +82,18 @@ Alternately, you can u > on Linux, macOS, and Windows) by pushing into a branch in your fork > or opening a GitHub's Pull Request against > https://github.com/git/git.git or a fork of that repository. > +In the event that you only want to trigger GitHub Actions for specific > +refname, you can create an executable file named `allow-ref` in > +`refs/ci/config`. Those below steps may help you: "These steps below" or "The follwoing steps", perhaps. > +-------------- > +$ git checkout --orphan ci-config > +$ cp contrib/ci-config-allow-ref allow-ref > +$ $EDITOR allow-ref > +$ git rm -rf . This sounds horrible. You just nuked the entire files in the working tree you use for your everyday Git hacking to edit a single file. > +$ git commit allow-ref If allow-ref were added to the index before this "git commit", the previous "git rm -rf ." would have removed it. Since the surviving allow-ref file must have been untracked, this "git commit" would not commit anything. Forgot to "git add"? > +$ git push <your-fork> HEAD:refs/ci/config > +-------------- > Do not forget to update the documentation to describe the updated > behavior and make sure that the resulting documentation set formats > diff --git a/contrib/ci-config-allow-ref b/contrib/ci-config-allow-ref > new file mode 100755 > index 0000000000..b53e9ddbd0 > --- /dev/null > +++ b/contrib/ci-config-allow-ref > @@ -0,0 +1,9 @@ > +#!/bin/sh > +# Sample filter for GitHub Actions > +# GitHub Actions will run if and only if this script exit with zero status s/exit/exits/; As the instruction above says, we should set the example and describe the behaviour we implemented initially. Something as basic like ... # Build any branch other than those whose name begins with "no-ci" > + > +REFNAME="$1" > + > +case "$REFNAME" in > +refs/heads/no-ci*) exit 1 ;; > +esac ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 0 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-06 14:25 UTC (permalink / raw) To: Junio C Hamano Cc: Jeff King, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin On 2020-05-05 20:56:58-0700, Junio C Hamano <gitster@pobox.com> wrote: > Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > > +-------------- > > +$ git checkout --orphan ci-config > > +$ cp contrib/ci-config-allow-ref allow-ref > > +$ $EDITOR allow-ref > > +$ git rm -rf . > > This sounds horrible. You just nuked the entire files in the > working tree you use for your everyday Git hacking to edit a > single file. It isn't that horrible as it sounds. It only removes the files that are currently added in index, which is the same with tracked files in old branch, and we can get it back by switching back to old branch. I decided to make an orphanage branch because I would like to save time and network bandwidth for the "check-ci" jobs. Since GitHub will fetch only single branch in GitHub Actions: /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/ci/config:refs/ci/config I wonder whether the "git rm -rf ." makes that block sounds horrible? If that is the case, we can use the experimental git-switch(1) instead, it's doing more-or-less the same (or is it the same?) with "git checkout --orphan" and "git rm -rf ." ----------------- $ cp contrib/ci-config-allow-ref.sample allow-ref $ git switch --orphan ci-config $ edit allow-ref $ git add allow-ref $ git commit ----------------- Note: I changed `$EDITOR` to `edit` to match other example of `edit`. > As the instruction above says, we should set the example and > describe the behaviour we implemented initially. Something as basic > like ... > > # Build any branch other than those whose name begins with "no-ci" That's definitely better. And I think we should add `.sample` suffix to this file, too If all of that's sensible enough, here is the replacement fix-up patch. --------------------8<------------------ Subject: [PATCH] fixup! ci: allow per-branch config for GitHub Actions Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 3 +-- Documentation/SubmittingPatches | 12 ++++++++++++ contrib/ci-config-allow-ref.sample | 12 ++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100755 contrib/ci-config-allow-ref.sample diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 51f4ff6e89..08217c5ed8 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -19,8 +19,7 @@ jobs: name: check whether CI is enabled for ref run: | enabled=yes - if test -e ref-whitelist && - ! grep '^${{ github.ref }}$' ref-whitelist + if test -x allow-ref && ! ./allow-ref '${{ github.ref }}' then enabled=no fi diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index b916f07f2c..bf06284f29 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -82,6 +82,18 @@ Alternately, you can use GitHub Actions (which supports testing your changes on Linux, macOS, and Windows) by pushing into a branch in your fork or opening a GitHub Pull Request against https://github.com/git/git.git or a fork of that repository. +In the event that you only want to trigger GitHub Actions for specific +refname, you can create an executable file named `allow-ref` in +`refs/ci/config`. The following steps may help you: +-------------- +$ cp contrib/ci-config-allow-ref.sample allow-ref +$ git switch --orphan <a-branch-name-of-your-choice> +$ edit allow-ref +$ git add allow-ref +$ git commit allow-ref +$ git push <your-fork> HEAD:refs/ci/config +-------------- + Do not forget to update the documentation to describe the updated behavior and make sure that the resulting documentation set formats diff --git a/contrib/ci-config-allow-ref.sample b/contrib/ci-config-allow-ref.sample new file mode 100755 index 0000000000..1973e88912 --- /dev/null +++ b/contrib/ci-config-allow-ref.sample @@ -0,0 +1,12 @@ +#!/bin/sh +# Sample filter for GitHub Actions +# GitHub Actions will run if and only if this script exits with zero status + +# This sample filter will ask GitHub Actions to build +# any branches whose name doesn't start with "no-ci" + +REFNAME="$1" + +case "$REFNAME" in +refs/heads/no-ci*) exit 1 ;; +esac -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 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 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-06 16:31 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: Jeff King, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > On 2020-05-05 20:56:58-0700, Junio C Hamano <gitster@pobox.com> wrote: >> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: >> > +-------------- >> > +$ git checkout --orphan ci-config >> > +$ cp contrib/ci-config-allow-ref allow-ref >> > +$ $EDITOR allow-ref >> > +$ git rm -rf . >> >> This sounds horrible. You just nuked the entire files in the >> working tree you use for your everyday Git hacking to edit a >> single file. > > It isn't that horrible as it sounds. It only removes the files that are > currently added in index, which is the same with tracked files in old > branch, and we can get it back by switching back to old branch. > > I decided to make an orphanage branch because I would like to save > time and network bandwidth for the "check-ci" jobs. I didn't say it is wrong to record a tree with a single file (allow-ref) in a commit that is pointed by the ci-config ref. I would have expected you to create such a commit in an otherwise empty repository, and push into your fork of Git at GitHub. That way, you won't have to checkout and/or refresh the index all of the 3800+ files. > I wonder whether the "git rm -rf ." makes that block sounds horrible? > If that is the case, we can use the experimental git-switch(1) > instead, it's doing more-or-less the same (or is it the same?) with > "git checkout --orphan" and "git rm -rf ." That does not change anything, as abuse of your primary repository is what offends me. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-06 16:31 ` Junio C Hamano @ 2020-05-07 12:25 ` Jeff King 2020-05-07 18:29 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-07 12:25 UTC (permalink / raw) To: Junio C Hamano Cc: Đoàn Trần Công Danh, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin On Wed, May 06, 2020 at 09:31:25AM -0700, Junio C Hamano wrote: > Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > > > On 2020-05-05 20:56:58-0700, Junio C Hamano <gitster@pobox.com> wrote: > >> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > >> > +-------------- > >> > +$ git checkout --orphan ci-config > >> > +$ cp contrib/ci-config-allow-ref allow-ref > >> > +$ $EDITOR allow-ref > >> > +$ git rm -rf . > >> > >> This sounds horrible. You just nuked the entire files in the > >> working tree you use for your everyday Git hacking to edit a > >> single file. > > > > It isn't that horrible as it sounds. It only removes the files that are > > currently added in index, which is the same with tracked files in old > > branch, and we can get it back by switching back to old branch. > > > > I decided to make an orphanage branch because I would like to save > > time and network bandwidth for the "check-ci" jobs. > > I didn't say it is wrong to record a tree with a single file > (allow-ref) in a commit that is pointed by the ci-config ref. > > I would have expected you to create such a commit in an otherwise > empty repository, and push into your fork of Git at GitHub. That > way, you won't have to checkout and/or refresh the index all of the > 3800+ files. Yeah, I agree that all of the mechanisms for dealing with the unrelated history are somewhat awkward. Another issue is that you can't just: git clone --single-branch -b refs/ci/config my-config to work on it, because "-b" wants only heads or tags (we could address that by putting it in refs/heads/ci-config or similar). If we do go the javascript route, perhaps it would make sense for refs/ci/config to be a single blob containing a snippet of javascript with several functions. And then we could just eval() that and call the appropriate functions (if defined). Then a sample can live in the main repo with something like: # This file contains functions which will be run by the GitHub # Actions CI script. # # You may customize it for your own fork by modifying it on any branch # you like, and installing with: # # git push <remote> $(git rev-parse HEAD:ci/config):refs/ci/config # # [allow_ref() sample definition and documentation....] That sidesteps most of those issues. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 12:25 ` Jeff King @ 2020-05-07 18:29 ` Junio C Hamano 2020-05-07 18:54 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-07 18:29 UTC (permalink / raw) To: Jeff King Cc: Đoàn Trần Công Danh, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > Yeah, I agree that all of the mechanisms for dealing with the unrelated > history are somewhat awkward. Another issue is that you can't just: > > git clone --single-branch -b refs/ci/config my-config > > to work on it, because "-b" wants only heads or tags (we could address > that by putting it in refs/heads/ci-config or similar). I somehow don't think that it is such a huge issue, because I expect anybody who has legitimate interest in refs/ci/config to have a full clone of git.git anyway. So it is more like defining another remote.origin.fetch like this: [remote "origin"] url = https://git.kernel.org/pub/scm/git/git.git/ fetch = +refs/heads/*:refs/remotes/origin/* + fetch = +refs/ci/config:refs/remotes/origin/ci-config [remote "publish"] url = https://github.com/user/git/ and then do something like: $ git worktree add -b ci-config ../git-ci-config origin/ci-config $ cd ../git-ci-config ... hack hack hack ... $ git push publish ci-config:refs/ci/config > If we do go the javascript route, perhaps it would make sense for > refs/ci/config to be a single blob containing a snippet of javascript > with several functions. And then we could just eval() that and call the > appropriate functions (if defined). Yup. > Then a sample can live in the main repo with something like: > > # This file contains functions which will be run by the GitHub > # Actions CI script. > # > # You may customize it for your own fork by modifying it on any branch > # you like, and installing with: > # > # git push <remote> $(git rev-parse HEAD:ci/config):refs/ci/config > # > # [allow_ref() sample definition and documentation....] > > That sidesteps most of those issues. Perhaps. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 18:29 ` Junio C Hamano @ 2020-05-07 18:54 ` Jeff King 2020-05-07 19:33 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-07 18:54 UTC (permalink / raw) To: Junio C Hamano Cc: Đoàn Trần Công Danh, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin On Thu, May 07, 2020 at 11:29:09AM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > Yeah, I agree that all of the mechanisms for dealing with the unrelated > > history are somewhat awkward. Another issue is that you can't just: > > > > git clone --single-branch -b refs/ci/config my-config > > > > to work on it, because "-b" wants only heads or tags (we could address > > that by putting it in refs/heads/ci-config or similar). > > I somehow don't think that it is such a huge issue, because I expect > anybody who has legitimate interest in refs/ci/config to have a full > clone of git.git anyway. So it is more like defining another > remote.origin.fetch like this: > > [remote "origin"] > url = https://git.kernel.org/pub/scm/git/git.git/ > fetch = +refs/heads/*:refs/remotes/origin/* > + fetch = +refs/ci/config:refs/remotes/origin/ci-config > > [remote "publish"] > url = https://github.com/user/git/ > > > and then do something like: > > $ git worktree add -b ci-config ../git-ci-config origin/ci-config > $ cd ../git-ci-config > ... hack hack hack ... > $ git push publish ci-config:refs/ci/config I dunno. The duality of refs/ci/config and refs/heads/ci-config is weird. Your two refspecs overlap on their RHS, and once you push up the refs/heads/ci-config you created with "git worktree add", they'll start to conflict (which maybe is OK as long as they're the same). In the patch I sent out a few hours ago I just caved and made refs/heads/ci-config the magic ref, in order to avoid complexity. But it also wouldn't be too bad to say "look, you can store this however you like in refs/heads/ or not at all, but install it into place with git push <remote> HEAD:refs/ci/config". The audience is git.git developers (and even advanced ones whose workflows involve tweaking their CI config), so I'd like to think they could figure it out. I have a feeling that fewer than 5 people in the world will end up using this feature either way. ;) -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-07 18:54 ` Jeff King @ 2020-05-07 19:33 ` Junio C Hamano 0 siblings, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-07 19:33 UTC (permalink / raw) To: Jeff King Cc: Đoàn Trần Công Danh, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > In the patch I sent out a few hours ago I just caved and made > refs/heads/ci-config the magic ref, in order to avoid complexity. > > But it also wouldn't be too bad to say "look, you can store this however > you like in refs/heads/ or not at all, but install it into place with > git push <remote> HEAD:refs/ci/config". The audience is git.git > developers (and even advanced ones whose workflows involve tweaking > their CI config), so I'd like to think they could figure it out. > > I have a feeling that fewer than 5 people in the world will end up using > this feature either way. ;) Yeah, let's not spend too much time and too many cycles to overengineer it. Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v2] ci: allow per-branch config for GitHub Actions 2020-05-05 21:04 ` Jeff King 2020-05-05 21:29 ` Junio C Hamano 2020-05-06 0:46 ` Đoàn Trần Công Danh @ 2020-05-07 16:20 ` Jeff King 2020-05-07 17:00 ` Taylor Blau 2020-05-07 19:53 ` Junio C Hamano 2 siblings, 2 replies; 62+ messages in thread From: Jeff King @ 2020-05-07 16:20 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin 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 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@peff.net> --- .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' 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 +# 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 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 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 1 sibling, 1 reply; 62+ messages in thread From: Taylor Blau @ 2020-05-07 17:00 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin 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@peff.net> > --- > .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 ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 2020-05-07 17:00 ` Taylor Blau @ 2020-05-07 17:18 ` Jeff King 0 siblings, 0 replies; 62+ messages in thread From: Jeff King @ 2020-05-07 17:18 UTC (permalink / raw) To: Taylor Blau Cc: Junio C Hamano, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Thu, May 07, 2020 at 11:00:42AM -0600, Taylor Blau wrote: > > 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. Yes, I intentionally left them out for that reason. It seemed simpler to just let the skip percolate down the dependency tree. > > +# 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. No, the whole point of doing the partial clone (rather than using actions/checkout, which doesn't support that) was so people didn't have to deal with the orphan-branch thing. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 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 19:53 ` Junio C Hamano 2020-05-07 20:46 ` Jeff King 1 sibling, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-07 19:53 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > + - 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 }}' Is it deliberate that the output from the script is not redirected to >/dev/null, which would mean they are allowed to do something that looks like: echo "::set-output name=enabled::frotz" or emit other random ::string-that-affects-github-actions to its standard output stream? > + then > + enabled=no > + fi > + echo "::set-output name=enabled::$enabled" ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 2020-05-07 19:53 ` Junio C Hamano @ 2020-05-07 20:46 ` Jeff King 2020-05-07 21:58 ` Junio C Hamano 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-07 20:46 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Thu, May 07, 2020 at 12:53:09PM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > + - 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 }}' > > Is it deliberate that the output from the script is not redirected > to >/dev/null, which would mean they are allowed to do something > that looks like: > > echo "::set-output name=enabled::frotz" > > or emit other random ::string-that-affects-github-actions to its > standard output stream? It was deliberate in the sense that I would allow them to write useful messages to the Actions log. If they want to do nonsense like "::set-output", then it's their foot and their gun. I don't know if Actions distinguishes between stdout and stderr here (i.e., if we redirected the script's stdout to stderr, would that prevent this case or not?). -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 2020-05-07 20:46 ` Jeff King @ 2020-05-07 21:58 ` Junio C Hamano 2020-05-08 18:00 ` Jeff King 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-07 21:58 UTC (permalink / raw) To: Jeff King Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > It was deliberate in the sense that I would allow them to write useful > messages to the Actions log. If they want to do nonsense like > "::set-output", then it's their foot and their gun. It's not like fooling the framework you laid out here is a potentially useful attack vector. We can assume that it is unlikely for the custom allow-ref to be writing a string that happens to begin with double-colon by mistake and making it harder to debug. > I don't know if Actions distinguishes between stdout and stderr here > (i.e., if we redirected the script's stdout to stderr, would that > prevent this case or not?). Perhaps we can experiment with "echo >&2 we are getting called" in the allow-ref script itself ;-). In any case, I'll queue it on 'pu'. Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 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 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-08 18:00 UTC (permalink / raw) To: Junio C Hamano Cc: Taylor Blau, Đoàn Trần Công Danh, git, Jeff Hostetler, Johannes Schindelin On Thu, May 07, 2020 at 02:58:16PM -0700, Junio C Hamano wrote: > In any case, I'll queue it on 'pu'. Thanks. I just noticed this needs a small fix to the sample script, which I gave the wrong name: diff --git a/ci/config/allow-refs.sample b/ci/config/allow-ref.sample similarity index 93% rename from ci/config/allow-refs.sample rename to ci/config/allow-ref.sample index f157f1945a..c9c9aea9ff 100755 --- a/ci/config/allow-refs.sample +++ b/ci/config/allow-ref.sample @@ -7,8 +7,8 @@ # your repository: # # git checkout -b ci-config -# cp allow-refs.sample allow-refs -# $EDITOR allow-refs +# cp allow-refs.sample allow-ref +# $EDITOR allow-ref # git commit -am "implement my ci preferences" # git push # > Perhaps we can experiment with "echo >&2 we are getting called" in > the allow-ref script itself ;-). That definitely ends up in the log. But doing: echo "::error file=foo.sh,line=1,col=2::this is the stdout msg" echo >&2 "::error file=foo.sh,line=1,col=2::this is the stderr msg" shows both versions formatted in red, which implies to me that stdout and stderr are treated the same. -Peff ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2] ci: allow per-branch config for GitHub Actions 2020-05-08 18:00 ` Jeff King @ 2020-05-09 1:23 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-09 1:23 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Taylor Blau, git, Jeff Hostetler, Johannes Schindelin On 2020-05-08 14:00:47-0400, Jeff King <peff@peff.net> wrote: > I just noticed this needs a small fix to the sample script, which I gave > the wrong name: > > diff --git a/ci/config/allow-refs.sample b/ci/config/allow-ref.sample > similarity index 93% > rename from ci/config/allow-refs.sample > rename to ci/config/allow-ref.sample > index f157f1945a..c9c9aea9ff 100755 > --- a/ci/config/allow-refs.sample > +++ b/ci/config/allow-ref.sample > @@ -7,8 +7,8 @@ > # your repository: > # > # git checkout -b ci-config > -# cp allow-refs.sample allow-refs > -# $EDITOR allow-refs > +# cp allow-refs.sample allow-ref Hi Peff, The source needs to be changed, too: ---------------8<------------- diff --git a/ci/config/allow-ref.sample b/ci/config/allow-ref.sample index c9c9aea9ff..249872425f 100755 --- a/ci/config/allow-ref.sample +++ b/ci/config/allow-ref.sample @@ -7,7 +7,7 @@ # your repository: # # git checkout -b ci-config -# cp allow-refs.sample allow-ref +# cp allow-ref.sample allow-ref # $EDITOR allow-ref # git commit -am "implement my ci preferences" # git push --------------------8<------------------ -- Danh ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 1/2] CI: limit GitHub Actions to designated branches 2020-05-04 16:23 ` Jeff King 2020-05-04 21:58 ` Taylor Blau @ 2020-05-05 0:34 ` Đoàn Trần Công Danh 1 sibling, 0 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 0:34 UTC (permalink / raw) To: Jeff King; +Cc: git, Jeff Hostetler, Johannes Schindelin On 2020-05-04 12:23:11-0400, Jeff King <peff@peff.net> wrote: > On Mon, May 04, 2020 at 10:49:31PM +0700, Đoàn Trần Công Danh wrote: > Doing "**" here makes sense to catch everything (it would be even better > if we could just say "everything with a pull request" by omitting the > branch filter entirely, but maybe that's not possible). When I was doing this, I couldn't create a fork of my fork to check the syntax for GitHub PR. So, I pick a safe step. Turn out, I can create PR against my own fork. And, it's possible to omit the branch filter entirely. > > + tags: > > + - '*' > > Would we want that here, too? I guess nobody is likely to push > "foo/v1.2.3". > > Or on the flip side, would we want to tighten this? If I push a tag > "wip", I probably don't want it built. Probably the right rule is > "annotated tags only", but I suspect that's not possible. From my reading, GitHub Actions only accepts filter by refname. From GitHub manual, we can limit tags selection by: -------------8<------ diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index adf8824af1..9bba0ce068 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,7 +11,8 @@ on: - pu - 'for-ci**' tags: - - '*' + - '**' + - '!**wip**' env: DEVELOPER: 1 ---------------->8------------- But, I'm running into GitHub internal error with this snippet. I'll look into it later. > > > + push: > > + branches: > > + - maint > > + - master > > + - next > > + - jch > > + - pu > > What happened to "for-ci" (presumably "for-ci/**")? Hm, it was lost when I created a lot of branch for testing and I send the incorrect one out. It'll be correct next time. -- Danh ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI 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 15:49 ` Đoàn Trần Công Danh 2020-05-04 16:37 ` Junio C Hamano 2020-05-05 16:26 ` [PATCH v3 0/3] Provide option to opt in/out GitHub Actions Đoàn Trần Công Danh 2 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-04 15:49 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King From 889cacb689 (ci: configure GitHub Actions for CI/PR, 2020-04-11), GitHub Actions was introduced as an alternative CI system for Git project. Let's advertise it to Git's contributors to help them test Git on various platforms before submitting to Git. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- Documentation/SubmittingPatches | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 4515cab519..741867dfe3 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -78,6 +78,11 @@ on open source projects), you can use their Travis CI integration to test your changes on Linux, Mac (and hopefully soon Windows). See GitHub-Travis CI hints section for details. +Alternately, you can use GitHub Actions (which supports testing your changes +on Linux, macOS, and Windows) by pushing into a branch whose name starts +with "for-ci/" or opening a GitHub's Pull Request against +https://github.com/git/git.git + Do not forget to update the documentation to describe the updated behavior and make sure that the resulting documentation set formats well (try the Documentation/doc-diff script). -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI 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 0 siblings, 1 reply; 62+ messages in thread From: Junio C Hamano @ 2020-05-04 16:37 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Jeff King Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > +Alternately, you can use GitHub Actions (which supports testing your changes > +on Linux, macOS, and Windows) by pushing into a branch whose name starts > +with "for-ci/" or opening a GitHub's Pull Request against > +https://github.com/git/git.git Can you tighten the description of "for-ci/" a bit? It's not like the convention is offered in _any_ repository, but it is active only if you push to a fork of git.git, right? If your fork is a fork of a fork, what happens (e.g. github.com/gitster/git is marked as a fork of git/git; when somebody forks from gitster/git, would they also get the for-ci/ convention)? Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI 2020-05-04 16:37 ` Junio C Hamano @ 2020-05-05 0:46 ` Đoàn Trần Công Danh 0 siblings, 0 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 0:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jeff Hostetler, Johannes Schindelin, Jeff King On 2020-05-04 09:37:11-0700, Junio C Hamano <gitster@pobox.com> wrote: > Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > > > +Alternately, you can use GitHub Actions (which supports testing your changes > > +on Linux, macOS, and Windows) by pushing into a branch whose name starts > > +with "for-ci/" or opening a GitHub's Pull Request against > > +https://github.com/git/git.git > > Can you tighten the description of "for-ci/" a bit? It's not like > the convention is offered in _any_ repository, but it is active only > if you push to a fork of git.git, right? The convention will work in any repository with "$TOPDIR/.github/workflows/*.yml" exists. Since GitHub Actions will look into the file ".github/workflows/*.yml" in current repository. > If your fork is a fork of > a fork, what happens (e.g. github.com/gitster/git is marked as a > fork of git/git; when somebody forks from gitster/git, would they > also get the for-ci/ convention)? Yes, they'll get that convention. -- Danh ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v3 0/3] Provide option to opt in/out GitHub Actions 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 15:49 ` [PATCH v2 2/2] SubmittingPatches: advertise GitHub Actions CI Đoàn Trần Công Danh @ 2020-05-05 16:26 ` Đ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 ` (3 more replies) 2 siblings, 4 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 16:26 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau, Junio C Hamano It seems like there're 2 schools for GitHub Actions, - Opt-in by using some specific patterns - Opt-out by using some specific patterns I tried to help everyone happy, but with my current knowledge, I couldn't. This series should be 2 patches only. I provide the [3/3] as a fix-up for [2/3]. Should we conclude that it's over-engineered, please drop [3/3]. Should we think it's OK to go that route, please fix-up [3/3] into [2/3]. Hopefully, we can get into some agreement on this matter. I'm not include range-diff/inter-diff against v2 since I reorder the patches, and [3/3] is meant for discussion. Đoàn Trần Công Danh (3): SubmittingPatches: advertise GitHub Actions CI CI: limit GitHub Actions to designated branches fixup! CI: limit GitHub Actions to designated branches .github/workflows/main.yml | 82 ++++++++++++++++++++++++++++++++- Documentation/SubmittingPatches | 6 +++ 2 files changed, 87 insertions(+), 1 deletion(-) -- 2.26.2.672.g232c24e857 ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v3 1/3] SubmittingPatches: advertise GitHub Actions CI 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 ` Đoàn Trần Công Danh 2020-05-05 16:47 ` 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 ` (2 subsequent siblings) 3 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 16:26 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau, Junio C Hamano From 889cacb689 (ci: configure GitHub Actions for CI/PR, 2020-04-11), GitHub Actions was introduced as an alternative CI system for Git project. Let's advertise it to Git's contributors to help them test Git on various platforms before submitting to Git. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- Documentation/SubmittingPatches | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 4515cab519..8686318550 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -78,6 +78,11 @@ on open source projects), you can use their Travis CI integration to test your changes on Linux, Mac (and hopefully soon Windows). See GitHub-Travis CI hints section for details. +Alternately, you can use GitHub Actions (which supports testing your changes +on Linux, macOS, and Windows) by pushing into a branch in your fork +or opening a GitHub's Pull Request against +https://github.com/git/git.git or a fork of that repository. + Do not forget to update the documentation to describe the updated behavior and make sure that the resulting documentation set formats well (try the Documentation/doc-diff script). -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v3 1/3] SubmittingPatches: advertise GitHub Actions CI 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 0 siblings, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-05 16:47 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On Tue, May 05, 2020 at 11:26:39PM +0700, Đoàn Trần Công Danh wrote: > From 889cacb689 (ci: configure GitHub Actions for CI/PR, 2020-04-11), > GitHub Actions was introduced as an alternative CI system for Git > project. > > Let's advertise it to Git's contributors to help them test Git on > various platforms before submitting to Git. I think this makes sense. Two things: > +Alternately, you can use GitHub Actions (which supports testing your changes > +on Linux, macOS, and Windows) by pushing into a branch in your fork > +or opening a GitHub's Pull Request against > +https://github.com/git/git.git or a fork of that repository. Probably "GitHub Pull Request" would be more idiomatic English. Do people need to enable Actions on their forks for the branch push to work? I didn't need to for my fork of git/git, but I'm not sure if that's because I was playing with Actions months ago and forgot, or if having actions enabled on the parent repo makes it work. When I made a new repository that was not connected, I had to explicitly enable Actions on the site before it would run the workflow file. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 1/3] SubmittingPatches: advertise GitHub Actions CI 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 0 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 16:59 UTC (permalink / raw) To: Jeff King Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On 2020-05-05 12:47:40-0400, Jeff King <peff@peff.net> wrote: > On Tue, May 05, 2020 at 11:26:39PM +0700, Đoàn Trần Công Danh wrote: > > > From 889cacb689 (ci: configure GitHub Actions for CI/PR, 2020-04-11), > > GitHub Actions was introduced as an alternative CI system for Git > > project. > > > > Let's advertise it to Git's contributors to help them test Git on > > various platforms before submitting to Git. > > I think this makes sense. Two things: > > > +Alternately, you can use GitHub Actions (which supports testing your changes > > +on Linux, macOS, and Windows) by pushing into a branch in your fork > > +or opening a GitHub's Pull Request against > > +https://github.com/git/git.git or a fork of that repository. > > Probably "GitHub Pull Request" would be more idiomatic English. I guess you're right. Well, I'm not native English speaker. I was thinking, this kind of Pull Request is specific to GitHub, and it's different from git-request-pull(1), So, I use "'s" :) > Do people need to enable Actions on their forks for the branch push to > work? I didn't need to for my fork of git/git, but I'm not sure if that's > because I was playing with Actions months ago and forgot, or if having > actions enabled on the parent repo makes it work. It's a long time from the first time I tinker with GitHub Actions, I couldn't recall if I need to enable it explicitly. I look into GitHub support pages[1] and GitHub tells me that GitHub Actions is enabled by default for everyone, and every repo. We need to disable GitHub Actions explicitly, if we don't want it. > When I made a new repository that was not connected, I had to explicitly > enable Actions on the site before it would run the workflow file. It seems like GitHub Actions will be triggered automatically if GitHub finds any files in "$TOPDIR/.github/workflows/*.yml" [1]: https://help.github.com/en/actions/getting-started-with-github-actions/about-github-actions#disabling-or-limiting-github-actions-for-your-repository-or-organization -- Danh ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 1/3] SubmittingPatches: advertise GitHub Actions CI 2020-05-05 16:59 ` Đoàn Trần Công Danh @ 2020-05-05 17:07 ` Jeff King 0 siblings, 0 replies; 62+ messages in thread From: Jeff King @ 2020-05-05 17:07 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On Tue, May 05, 2020 at 11:59:33PM +0700, Đoàn Trần Công Danh wrote: > > > +Alternately, you can use GitHub Actions (which supports testing your changes > > > +on Linux, macOS, and Windows) by pushing into a branch in your fork > > > +or opening a GitHub's Pull Request against > > > +https://github.com/git/git.git or a fork of that repository. > > > > Probably "GitHub Pull Request" would be more idiomatic English. > > I guess you're right. > > Well, I'm not native English speaker. > I was thinking, this kind of Pull Request is specific to GitHub, > and it's different from git-request-pull(1), So, I use "'s" :) Using the possessive "'s" would imply you're talking about GitHub's feature itself. But you're talking about "a" pull request, so I think you want to just GitHub as an adjective, which would not generally be possessive. English is quirky. :) > > When I made a new repository that was not connected, I had to explicitly > > enable Actions on the site before it would run the workflow file. > > It seems like GitHub Actions will be triggered automatically if GitHub > finds any files in "$TOPDIR/.github/workflows/*.yml" It definitely wasn't for me. Try creating a new repo and going to the "Actions" tab. I get a "Get Started with GitHub Actions" page. If I push up a copy of git.git's master, then I get "Workflows aren't being run in this repository" with a big green button to enable. I'm just not sure if that happens for people who fork git/git instead of making a new repo. I'm reluctant to delete and remake my fork, but I guess it wouldn't be too hard to re-create. I think it's OK to leave it out for now. Even if this is how it works for a fork, it's not hard to discover what to do if you click the Actions tab. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v3 2/3] CI: limit GitHub Actions to designated branches 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:26 ` Đoàn Trần Công Danh 2020-05-05 16:51 ` 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 17:01 ` [PATCH v3 0/3] Provide option to opt in/out GitHub Actions Jeff King 3 siblings, 2 replies; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 16:26 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau, Junio C Hamano Git's maintainer usually don't have enough time to debug the failure of invidual feature branch, they usually want to look at intergration branches only. Contributors now can have GitHub Actions as an opt-in option, should they want to check their code, they will push into designated branch. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 14 +++++++++++++- Documentation/SubmittingPatches | 3 ++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fd4df939b5..9bba0ce068 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,18 @@ name: CI/PR -on: [push, pull_request] +on: + pull_request: + push: + branches: + - maint + - master + - next + - jch + - pu + - 'for-ci**' + tags: + - '**' + - '!**wip**' env: DEVELOPER: 1 diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 8686318550..e516b080df 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -79,7 +79,8 @@ test your changes on Linux, Mac (and hopefully soon Windows). See GitHub-Travis CI hints section for details. Alternately, you can use GitHub Actions (which supports testing your changes -on Linux, macOS, and Windows) by pushing into a branch in your fork +on Linux, macOS, and Windows) by pushing into a branch whose name starts +with "for-ci", or a tag whose name doesn't have `wip`, or opening a GitHub's Pull Request against https://github.com/git/git.git or a fork of that repository. -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v3 2/3] CI: limit GitHub Actions to designated branches 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 18:49 ` Junio C Hamano 1 sibling, 1 reply; 62+ messages in thread From: Jeff King @ 2020-05-05 16:51 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On Tue, May 05, 2020 at 11:26:40PM +0700, Đoàn Trần Công Danh wrote: > -on: [push, pull_request] > +on: > + pull_request: > + push: > + branches: > + - maint > + - master > + - next > + - jch > + - pu > + - 'for-ci**' Should this be "for-ci/**", or are we intending for "for-ci-foo" to work? I'd suspect anybody who uses this would use a full directory namespace in a refspec (like "refs/heads/*:refs/heads/for-ci/*"). It might be simpler conceptually to only support that. > + tags: > + - '**' > + - '!**wip**' IMHO this "wip" match is going too far. That was the name in the example I used, but really it could have been anything. I think we should either: - just build all tags; it usually takes special effort to push them up anyway, so a one-off "just mark this spot" tag likely wouldn't get pushed anyway - just build v[0-9]*, which would catch actual releases -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 2/3] CI: limit GitHub Actions to designated branches 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 0 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 17:05 UTC (permalink / raw) To: Jeff King Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On 2020-05-05 12:51:25-0400, Jeff King <peff@peff.net> wrote: > On Tue, May 05, 2020 at 11:26:40PM +0700, Đoàn Trần Công Danh wrote: > > > -on: [push, pull_request] > > +on: > > + pull_request: > > + push: > > + branches: > > + - maint > > + - master > > + - next > > + - jch > > + - pu > > + - 'for-ci**' > > Should this be "for-ci/**", or are we intending for "for-ci-foo" to > work? I'd suspect anybody who uses this would use a full directory > namespace in a refspec (like "refs/heads/*:refs/heads/for-ci/*"). It > might be simpler conceptually to only support that. I made this because I saw someone mentioned that they would like to push to 'for-ci' and expect it works for them. I guess it may be better to have: - for-ci - for-ci/** > > > + tags: > > + - '**' > > + - '!**wip**' > > IMHO this "wip" match is going too far. That was the name in the example > I used, but really it could have been anything. I think we should > either: > > - just build all tags; it usually takes special effort to push them up > anyway, so a one-off "just mark this spot" tag likely wouldn't get > pushed anyway > > - just build v[0-9]*, which would catch actual releases This sounds better, just build "v[0-9]*" and ignore everything else, with this pattern, I think we don't need to advertise tag to our users. And our maintainer shouldn't worry about it, since our maintainer will (likely) only push v[0-9]* tagged code, anyway. -- Danh ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 2/3] CI: limit GitHub Actions to designated branches 2020-05-05 17:05 ` Đoàn Trần Công Danh @ 2020-05-05 17:11 ` Jeff King 0 siblings, 0 replies; 62+ messages in thread From: Jeff King @ 2020-05-05 17:11 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On Wed, May 06, 2020 at 12:05:46AM +0700, Đoàn Trần Công Danh wrote: > > Should this be "for-ci/**", or are we intending for "for-ci-foo" to > > work? I'd suspect anybody who uses this would use a full directory > > namespace in a refspec (like "refs/heads/*:refs/heads/for-ci/*"). It > > might be simpler conceptually to only support that. > > I made this because I saw someone mentioned that they would like to > push to 'for-ci' and expect it works for them. > > I guess it may be better to have: > > - for-ci > - for-ci/** Ah, that makes sense. Yeah, the double-rule looks fine to me, but understanding that use, the original "for-ci**" is OK to me, too. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 2/3] CI: limit GitHub Actions to designated branches 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 18:49 ` Junio C Hamano 1 sibling, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 18:49 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > Git's maintainer usually don't have enough time to debug the failure of > invidual feature branch, they usually want to look at intergration > branches only. Please please please do not make this thing about me. I do not have Actions enabled at https://github.com/gitster/git/ repository anyway. Instead, please focus on making the end result useful for general contributors. > Contributors now can have GitHub Actions as an opt-in option, > should they want to check their code, they will push into designated > branch. OK, so this is an opt-in approach as you outlined in the cover letter. > + pull_request: > + push: > + branches: > + - maint > + - master > + - next > + - jch > + - pu > + - 'for-ci**' > + tags: > + - '**' > + - '!**wip**' I agree with the agreement you and Peff reached about for-ci plus for-ci/** patterns and building only v[0-9] tags in your discussion downthread. Thaks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v3 3/3] fixup! CI: limit GitHub Actions to designated branches 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:26 ` [PATCH v3 2/3] CI: limit GitHub Actions to designated branches Đoàn Trần Công Danh @ 2020-05-05 16:26 ` Đ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 3 siblings, 1 reply; 62+ messages in thread From: Đoàn Trần Công Danh @ 2020-05-05 16:26 UTC (permalink / raw) To: git Cc: Đoàn Trần Công Danh, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau, Junio C Hamano Here is the patch to for discussion. Should we want to enable GitHub Action for all repo-s, except git/git and gitster/git. We'll want to have this patch fix-up to the previous one. With this patch merged, contributors will need to opt-out by push to a branch/tag with "wip" anywhere in the refname. Note that, integration branches (maint, master, next, jch, pu) will always be built, regardless of repo. Since the current condition is ugly enough, and I find it's very hard to wrap my head around those condition. Should this patch get fixing up into previous one, please remove the second paragraph in the body of previous patch. Thanks. Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> --- .github/workflows/main.yml | 80 ++++++++++++++++++++++++++++++--- Documentation/SubmittingPatches | 4 +- 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 9bba0ce068..81dfa3d228 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -4,12 +4,8 @@ on: pull_request: push: branches: - - maint - - master - - next - - jch - - pu - - 'for-ci**' + - '**' + - '!**wip**' tags: - '**' - '!**wip**' @@ -19,6 +15,15 @@ env: jobs: windows-build: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} runs-on: windows-latest steps: - uses: actions/checkout@v1 @@ -43,6 +48,15 @@ jobs: name: windows-artifacts path: artifacts windows-test: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} runs-on: windows-latest needs: [windows-build] strategy: @@ -82,6 +96,15 @@ jobs: name: failed-tests-windows path: ${{env.FAILED_TEST_ARTIFACTS}} vs-build: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} env: MSYSTEM: MINGW64 NO_PERL: 1 @@ -130,6 +153,15 @@ jobs: name: vs-artifacts path: artifacts vs-test: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} runs-on: windows-latest needs: [vs-build] strategy: @@ -166,6 +198,15 @@ jobs: ${{matrix.nr}} 10 t[0-9]*.sh) "@ regular: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} strategy: matrix: vector: @@ -201,6 +242,15 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} dockerized: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} strategy: matrix: vector: @@ -225,6 +275,15 @@ jobs: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} static-analysis: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} env: jobname: StaticAnalysis runs-on: ubuntu-latest @@ -233,6 +292,15 @@ jobs: - run: ci/install-dependencies.sh - run: ci/run-static-analysis.sh documentation: + if: >- + ${{ + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || + github.ref == 'refs/heads/maint' || + github.ref == 'refs/heads/master' || + github.ref == 'refs/heads/next' || + github.ref == 'refs/heads/jch' || + github.ref == 'refs/heads/pu' + }} env: jobname: Documentation runs-on: ubuntu-latest diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index e516b080df..a2e6a3d2ca 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -79,8 +79,8 @@ test your changes on Linux, Mac (and hopefully soon Windows). See GitHub-Travis CI hints section for details. Alternately, you can use GitHub Actions (which supports testing your changes -on Linux, macOS, and Windows) by pushing into a branch whose name starts -with "for-ci", or a tag whose name doesn't have `wip`, +on Linux, macOS, and Windows) by pushing into a branch or tag +whose name doesn't have `wip`, or opening a GitHub's Pull Request against https://github.com/git/git.git or a fork of that repository. -- 2.26.2.672.g232c24e857 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v3 3/3] fixup! CI: limit GitHub Actions to designated branches 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 0 siblings, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-05 18:59 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Jeff King, Taylor Blau Đoàn Trần Công Danh <congdanhqx@gmail.com> writes: > Should we want to enable GitHub Action for all repo-s, except git/git > and gitster/git. We'll want to have this patch fix-up to the previous > one. Just to get everybody on the same page, git/git has only the primary integration branches and tags gitster/git has broken out individual topics We do want to build everything in git/git repository. I do not have Actions enabled for gitster/git repository. I expect general contributors to fork from the former, push their work on their own branches (not the copies of the public integration branches like 'master', 'next', etc.) so that they can throw a pull request at git/git. > jobs: > windows-build: > + if: >- > + ${{ > + (github.repository_owner != 'git' && github.repository_owner != 'gitster') || > + github.ref == 'refs/heads/maint' || > + github.ref == 'refs/heads/master' || > + github.ref == 'refs/heads/next' || > + github.ref == 'refs/heads/jch' || > + github.ref == 'refs/heads/pu' > + }} Ialready asked not to make this about me. You do not want to search and replace 'gitster' with 'peff' or 'dscho' or whatever string, only because/when I chose to retire. Such an occasion shouldn't be a significant event to the project's codebase. Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v3 0/3] Provide option to opt in/out GitHub Actions 2020-05-05 16:26 ` [PATCH v3 0/3] Provide option to opt in/out GitHub Actions Đoàn Trần Công Danh ` (2 preceding siblings ...) 2020-05-05 16:26 ` [PATCH v3 3/3] fixup! " Đoàn Trần Công Danh @ 2020-05-05 17:01 ` Jeff King 3 siblings, 0 replies; 62+ messages in thread From: Jeff King @ 2020-05-05 17:01 UTC (permalink / raw) To: Đoàn Trần Công Danh Cc: git, Jeff Hostetler, Johannes Schindelin, Taylor Blau, Junio C Hamano On Tue, May 05, 2020 at 11:26:38PM +0700, Đoàn Trần Công Danh wrote: > It seems like there're 2 schools for GitHub Actions, > - Opt-in by using some specific patterns > - Opt-out by using some specific patterns > > I tried to help everyone happy, but with my current knowledge, I couldn't. Yeah, I agree it's not possible to make anyone happy. I think the special handling of git/git and gitster/git in patch 3 is not worth doing. That case is already easy to solve because it's two repos: just turn off Actions in gitster/git if Junio isn't looking at the results, and leave them on in git/git. The real problem we're trying to solve is people with forks who might want some branches with CI, but not others. Right now the only solution is for them to make a separate repository, and push to both. To do that we need some way of getting per-branch information into the Actions workflow. The ideal solution would just pull extra data from some other ref or config within the repository, but there doesn't seem to be a way to do that. So we're left with: - changing the tree of the branch; this is painful to manage, and the tree changes risk leaking out when we send out patches, etc. I think we all agree this is too horrible. - annotating the commit message with [skip ci] or similar. This is less painful, but still risks leaking out during patch mailing, etc. - sticking data in the branch name. This is the "for-ci" thing, which requires you to opt-in to CI. But it could be -wip or -no-ci or whatever, which would flip the default. The third one seems like the only workable thing, so we have to pick a direction. I prefer "for-ci" to "no-ci" because it's easier to add a refspec to push to for-ci/* than it is to remember to put "no-ci" in every branch name that you _don't_ want built. -Peff ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] ci: respect the [skip ci] convention in our GitHub workflow "CI/PR" 2020-05-03 9:36 ` Jeff King 2020-05-03 12:05 ` Danh Doan @ 2020-05-03 16:46 ` Junio C Hamano 1 sibling, 0 replies; 62+ messages in thread From: Junio C Hamano @ 2020-05-03 16:46 UTC (permalink / raw) To: Jeff King Cc: Johannes Schindelin via GitGitGadget, git, Jeff Hostetler, Johannes Schindelin Jeff King <peff@peff.net> writes: > I'm not sure whether we want to be building all of the individual topics > in gitster/git or not. In theory that provides more information, but I'm > not sure if anybody is looking at them (and all of the notifications > would go to Junio anyway). I do not think I am letting GitHub notifications for gitster/* come to my mailbox, so it is quite possible that many have piled up there without anybody (not even me) seeing them. I won't be locally able to notice breakages introduced by an individual topic to an environment I do not have access to, and building these topics with GitHub Actions may reveal which ones break, say, vsbuild, but I won't be debugging and fixing such issues myself anyway, so at least to me it is useless to run build there. I think I agree with your outline, i.e. build pull requests to help contributors, and build public integration branches to help the project as a whole. Thanks. ^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2020-05-09 1:23 UTC | newest] Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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.