* [PATCH] ci: fix GitHub workflow when on a tagged revision @ 2020-04-24 17:17 Johannes Schindelin via GitGitGadget 2020-04-24 20:50 ` Junio C Hamano 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget 0 siblings, 2 replies; 16+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2020-04-24 17:17 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> When `master` is tagged, and then both `master` and the tag are pushed, Travis CI will happily build both. That is a waste of energy, which is why we skip the build for `master` in that case. However, our GitHub workflow does not trigger on tags, therefore, this logic results in a missing build for that revision. Even worse: the run would _fail_ because we would skip the Windows build, there are no artifacts to publish, and therefore no artifacts to download in the Windows test jobs. Let's just change the GitHub workflow to skip the logic to skip revisions that are tagged. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Do not skip tagged revisions in the GitHub workflow runs I noticed a failure in the run for Git for Windows v2.26.2 which was caused by this. This patch is based on dd/ci-swap-azure-pipelines-with-github-actions. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-619%2Fdscho%2Fgithub-workflows-and-tags-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-619/dscho/github-workflows-and-tags-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/619 ci/lib.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/lib.sh b/ci/lib.sh index dac36886e37..f151e2f0ecb 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -1,6 +1,7 @@ # Library of functions shared by all CI scripts skip_branch_tip_with_tag () { + test -z "$DONT_SKIP_TAGS" || return 0 # Sometimes, a branch is pushed at the same time the tag that points # at the same commit as the tip of the branch is pushed, and building # both at the same time is a waste. @@ -151,6 +152,7 @@ then CC="${CC:-gcc}" cache_dir="$HOME/none" + DONT_SKIP_TAGS=t export GIT_PROVE_OPTS="--timer --jobs 10" export GIT_TEST_OPTS="--verbose-log -x" base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c -- gitgitgadget ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] ci: fix GitHub workflow when on a tagged revision 2020-04-24 17:17 [PATCH] ci: fix GitHub workflow when on a tagged revision Johannes Schindelin via GitGitGadget @ 2020-04-24 20:50 ` Junio C Hamano 2020-04-24 21:12 ` Johannes Schindelin 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget 1 sibling, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-04-24 20:50 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > However, our GitHub workflow does not trigger on tags, therefore, this > logic results in a missing build for that revision. Thanks for noticing. The arrangement we had, which essentially said "we know we will always build tagged version, so a push of a branch that happens to have a tagged version at the tip can be ignored", served us wonderfully. Now the maintainers of projects (not just this one) are forbidden from tagging the tip of master, queue something else on top and push the result out, as it won't build or test the tagged version, which is a bit sad. > ci/lib.sh | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/ci/lib.sh b/ci/lib.sh > index dac36886e37..f151e2f0ecb 100755 > --- a/ci/lib.sh > +++ b/ci/lib.sh > @@ -1,6 +1,7 @@ > # Library of functions shared by all CI scripts > > skip_branch_tip_with_tag () { > + test -z "$DONT_SKIP_TAGS" || return 0 > # Sometimes, a branch is pushed at the same time the tag that points > # at the same commit as the tip of the branch is pushed, and building > # both at the same time is a waste. > @@ -151,6 +152,7 @@ then > CC="${CC:-gcc}" > > cache_dir="$HOME/none" > + DONT_SKIP_TAGS=t Quite straight-forward. Thanks, will queue. > export GIT_PROVE_OPTS="--timer --jobs 10" > export GIT_TEST_OPTS="--verbose-log -x" > > base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] ci: fix GitHub workflow when on a tagged revision 2020-04-24 20:50 ` Junio C Hamano @ 2020-04-24 21:12 ` Johannes Schindelin 2020-04-24 21:24 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-04-24 21:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Fri, 24 Apr 2020, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > However, our GitHub workflow does not trigger on tags, therefore, this > > logic results in a missing build for that revision. > > Thanks for noticing. The arrangement we had, which essentially said > "we know we will always build tagged version, so a push of a branch > that happens to have a tagged version at the tip can be ignored", > served us wonderfully. Now the maintainers of projects (not just > this one) are forbidden from tagging the tip of master, queue > something else on top and push the result out, as it won't build or > test the tagged version, which is a bit sad. Nobody forbids this... ;-) And I was wrong, our current GitHub workflow _is_ triggered by tags. See e.g. https://github.com/git-for-windows/git/actions/runs/83103626 which was triggered by v2.26.2.windows.1. However, you also see that it failed due to the reason I described in the commit message. I guess that we should either go with this patch (which would trigger full runs also on tags), or we could alternatively trigger only on branch pushes (and not tag pushes)? Ciao, Dscho > > > ci/lib.sh | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/ci/lib.sh b/ci/lib.sh > > index dac36886e37..f151e2f0ecb 100755 > > --- a/ci/lib.sh > > +++ b/ci/lib.sh > > @@ -1,6 +1,7 @@ > > # Library of functions shared by all CI scripts > > > > skip_branch_tip_with_tag () { > > + test -z "$DONT_SKIP_TAGS" || return 0 > > # Sometimes, a branch is pushed at the same time the tag that points > > # at the same commit as the tip of the branch is pushed, and building > > # both at the same time is a waste. > > @@ -151,6 +152,7 @@ then > > CC="${CC:-gcc}" > > > > cache_dir="$HOME/none" > > + DONT_SKIP_TAGS=t > > Quite straight-forward. Thanks, will queue. > > > export GIT_PROVE_OPTS="--timer --jobs 10" > > export GIT_TEST_OPTS="--verbose-log -x" > > > > base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] ci: fix GitHub workflow when on a tagged revision 2020-04-24 21:12 ` Johannes Schindelin @ 2020-04-24 21:24 ` Junio C Hamano 0 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2020-04-24 21:24 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Hi Junio, > > On Fri, 24 Apr 2020, Junio C Hamano wrote: > >> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> >> writes: >> >> > However, our GitHub workflow does not trigger on tags, therefore, this >> > logic results in a missing build for that revision. >> >> Thanks for noticing. The arrangement we had, which essentially said >> "we know we will always build tagged version, so a push of a branch >> that happens to have a tagged version at the tip can be ignored", >> served us wonderfully. Now the maintainers of projects (not just >> this one) are forbidden from tagging the tip of master, queue >> something else on top and push the result out, as it won't build or >> test the tagged version, which is a bit sad. > > Nobody forbids this... ;-) > > And I was wrong, our current GitHub workflow _is_ triggered by tags. See > e.g. https://github.com/git-for-windows/git/actions/runs/83103626 which > was triggered by v2.26.2.windows.1. > > However, you also see that it failed due to the reason I described in the > commit message. > > I guess that we should either go with this patch (which would trigger full > runs also on tags), or we could alternatively trigger only on branch > pushes (and not tag pushes)? Hmph, even if we were to go with this patch (which should be safer), we'd definitely need an updated log message, then. Thanks for double-checking. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs 2020-04-24 17:17 [PATCH] ci: fix GitHub workflow when on a tagged revision Johannes Schindelin via GitGitGadget 2020-04-24 20:50 ` Junio C Hamano @ 2020-10-08 15:29 ` Johannes Schindelin via GitGitGadget 2020-10-08 15:29 ` [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees Johannes Schindelin via GitGitGadget ` (2 more replies) 1 sibling, 3 replies; 16+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2020-10-08 15:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin Whenever a GitGitGadget Pull Request is sent to the Git mailing list, a tag is pushed to gitgitgadget/git to commemorate that iteration. The push event caused for that triggers the CI/PR workflow, and reveals a pretty old bug where the windows-build steps are skipped for tagged revisions, but the windows-test steps are not (and will therefore fail). That means, of course, that every GitGitGadget PR is marked with a failed test once it is submitted. This patch series is designed to address this issue, and is based on am/ci-wsfix (the initial round was based on dd/ci-swap-azure-pipelines-with-github-actions but would now cause merge conflicts). Changes since v1: * Rather than returning early from skip_branch_tip_with_tag(), we now skip the function call altogether when run in a GitHub workflow. * The intention of the tag skipping was replicated by introducing another check in ci-config: is there a successful workflow run for the same commit (or at least for the same tree)? If yes, skip, referring to that successful run. Johannes Schindelin (2): ci: skip GitHub workflow runs for already-tested commits/trees ci: do not skip tagged revisions in GitHub workflows .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++- ci/lib.sh | 2 ++ 2 files changed, 40 insertions(+), 1 deletion(-) base-commit: 055747cd75c0904cc8122e5c12bd45e9f4743c30 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-619%2Fdscho%2Fgithub-workflows-and-tags-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-619/dscho/github-workflows-and-tags-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/619 Range-diff vs v1: -: ---------- > 1: 914868d558 ci: skip GitHub workflow runs for already-tested commits/trees 1: d9823f82ee ! 2: 931a2b8482 ci: fix GitHub workflow when on a tagged revision @@ Metadata Author: Johannes Schindelin <Johannes.Schindelin@gmx.de> ## Commit message ## - ci: fix GitHub workflow when on a tagged revision + ci: do not skip tagged revisions in GitHub workflows When `master` is tagged, and then both `master` and the tag are pushed, Travis CI will happily build both. That is a waste of energy, which is why we skip the build for `master` in that case. - However, our GitHub workflow does not trigger on tags, therefore, this - logic results in a missing build for that revision. + Our GitHub workflow is also triggered by tags. However, the run would + fail because the `windows-test` jobs are _not_ skipped on tags, but the + `windows-build` job _is skipped (and therefore fails to upload the + build artifacts needed by the test jobs). - Even worse: the run would _fail_ because we would skip the Windows - build, there are no artifacts to publish, and therefore no artifacts to - download in the Windows test jobs. + In addition, we just added logic to our GitHub workflow that will skip + runs altogether if there is already a successful run for the same commit + or at least for the same tree. - Let's just change the GitHub workflow to skip the logic to skip - revisions that are tagged. + Let's just change the GitHub workflow to no longer specifically skip + tagged revisions. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## ci/lib.sh ## -@@ - # Library of functions shared by all CI scripts - - skip_branch_tip_with_tag () { -+ test -z "$DONT_SKIP_TAGS" || return 0 - # Sometimes, a branch is pushed at the same time the tag that points - # at the same commit as the tip of the branch is pushed, and building - # both at the same time is a waste. @@ ci/lib.sh: then + CI_REPO_SLUG="$GITHUB_REPOSITORY" + CI_JOB_ID="$GITHUB_RUN_ID" CC="${CC:-gcc}" ++ DONT_SKIP_TAGS=t cache_dir="$HOME/none" -+ DONT_SKIP_TAGS=t - export GIT_PROVE_OPTS="--timer --jobs 10" - export GIT_TEST_OPTS="--verbose-log -x" +@@ ci/lib.sh: good_trees_file="$cache_dir/good-trees" + + mkdir -p "$cache_dir" + ++test -n "${DONT_SKIP_TAGS-}" || + skip_branch_tip_with_tag + skip_good_tree + -- gitgitgadget ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget @ 2020-10-08 15:29 ` Johannes Schindelin via GitGitGadget 2020-10-09 7:29 ` SZEDER Gábor 2020-10-08 15:29 ` [PATCH v2 2/2] ci: do not skip tagged revisions in GitHub workflows Johannes Schindelin via GitGitGadget 2020-10-08 21:11 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Junio C Hamano 2 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2020-10-08 15:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> When pushing a commit that has already passed a CI or PR build successfully, it makes sense to save some energy and time and skip the new build. Let's teach our GitHub workflow to do that. For good measure, we also compare the tree ID, which is what we actually test (the commit ID might have changed due to a reworded commit message, which should not affect the outcome of the run). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5bd321e5e1..7391b46d61 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,7 @@ jobs: ci-config: runs-on: ubuntu-latest outputs: - enabled: ${{ steps.check-ref.outputs.enabled }} + enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }} steps: - name: try to clone ci-config branch continue-on-error: true @@ -35,6 +35,43 @@ jobs: enabled=no fi echo "::set-output name=enabled::$enabled" + - name: skip if the commit or tree was already tested + id: skip-if-redundant + uses: actions/github-script@v3 + if: steps.check-ref.outputs.enabled == 'yes' + with: + github-token: ${{secrets.GITHUB_TOKEN}} + script: | + // Figure out workflow ID, commit and tree + const { data: run } = await github.actions.getWorkflowRun({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.runId, + }); + const workflow_id = run.workflow_id; + const head_sha = run.head_sha; + const tree_id = run.head_commit.tree_id; + + // See whether there is a successful run for that commit or tree + const { data: runs } = await github.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + per_page: 500, + status: 'success', + workflow_id, + }); + for (const run of runs.workflow_runs) { + if (head_sha === run.head_sha) { + core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`); + core.setOutput('enabled', ' but skip'); + break; + } + if (tree_id === run.head_commit.tree_id) { + core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); + core.setOutput('enabled', ' but skip'); + break; + } + } windows-build: needs: ci-config -- gitgitgadget ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-08 15:29 ` [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees Johannes Schindelin via GitGitGadget @ 2020-10-09 7:29 ` SZEDER Gábor 2020-10-09 11:13 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: SZEDER Gábor @ 2020-10-09 7:29 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > When pushing a commit that has already passed a CI or PR build > successfully, it makes sense to save some energy and time and skip the > new build. > > Let's teach our GitHub workflow to do that. > > For good measure, we also compare the tree ID, which is what we actually > test (the commit ID might have changed due to a reworded commit message, > which should not affect the outcome of the run). We have been doing this on Travis CI for a few years now. Does that approach not work on GitHub actions? Please explain in the commit message why a different approach is taken here. > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 38 insertions(+), 1 deletion(-) > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index 5bd321e5e1..7391b46d61 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -9,7 +9,7 @@ jobs: > ci-config: > runs-on: ubuntu-latest > outputs: > - enabled: ${{ steps.check-ref.outputs.enabled }} > + enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }} > steps: > - name: try to clone ci-config branch > continue-on-error: true > @@ -35,6 +35,43 @@ jobs: > enabled=no > fi > echo "::set-output name=enabled::$enabled" > + - name: skip if the commit or tree was already tested > + id: skip-if-redundant > + uses: actions/github-script@v3 > + if: steps.check-ref.outputs.enabled == 'yes' > + with: > + github-token: ${{secrets.GITHUB_TOKEN}} > + script: | > + // Figure out workflow ID, commit and tree > + const { data: run } = await github.actions.getWorkflowRun({ > + owner: context.repo.owner, > + repo: context.repo.repo, > + run_id: context.runId, > + }); > + const workflow_id = run.workflow_id; > + const head_sha = run.head_sha; > + const tree_id = run.head_commit.tree_id; > + > + // See whether there is a successful run for that commit or tree > + const { data: runs } = await github.actions.listWorkflowRuns({ > + owner: context.repo.owner, > + repo: context.repo.repo, > + per_page: 500, > + status: 'success', > + workflow_id, > + }); > + for (const run of runs.workflow_runs) { > + if (head_sha === run.head_sha) { > + core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`); > + core.setOutput('enabled', ' but skip'); > + break; > + } > + if (tree_id === run.head_commit.tree_id) { > + core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); > + core.setOutput('enabled', ' but skip'); > + break; > + } > + } > > windows-build: > needs: ci-config > -- > gitgitgadget > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-09 7:29 ` SZEDER Gábor @ 2020-10-09 11:13 ` Johannes Schindelin 2020-10-10 7:25 ` SZEDER Gábor 0 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-10-09 11:13 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Johannes Schindelin via GitGitGadget, git [-- Attachment #1: Type: text/plain, Size: 4415 bytes --] Hi Gábor, On Fri, 9 Oct 2020, SZEDER Gábor wrote: > On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > When pushing a commit that has already passed a CI or PR build > > successfully, it makes sense to save some energy and time and skip the > > new build. > > > > Let's teach our GitHub workflow to do that. > > > > For good measure, we also compare the tree ID, which is what we actually > > test (the commit ID might have changed due to a reworded commit message, > > which should not affect the outcome of the run). > > We have been doing this on Travis CI for a few years now. Does that > approach not work on GitHub actions? Please explain in the commit > message why a different approach is taken here. You're not being terribly clear about what exactly "We have been doing". Are you referring to the `skip_good_tree()` function that stores information in a file in the `good_trees_file`? If so, no, we cannot do that anywhere else than on Travis because that relies on a directory that is somehow shared between runs. And that is a feature that only Travis offers as far as I know (and it does not come without issues, e.g. when two concurrent runs try to write to the same file at the same time). Since this strategy relies on a Travis-only feature that does not work on the three other CI services we use (Cirrus CI, Azure DevOps, GitHub Actions), I see little point mentioning it in this commit message... However, I might be very well wrong on that assessment. Ciao, Dscho > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 38 insertions(+), 1 deletion(-) > > > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > > index 5bd321e5e1..7391b46d61 100644 > > --- a/.github/workflows/main.yml > > +++ b/.github/workflows/main.yml > > @@ -9,7 +9,7 @@ jobs: > > ci-config: > > runs-on: ubuntu-latest > > outputs: > > - enabled: ${{ steps.check-ref.outputs.enabled }} > > + enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }} > > steps: > > - name: try to clone ci-config branch > > continue-on-error: true > > @@ -35,6 +35,43 @@ jobs: > > enabled=no > > fi > > echo "::set-output name=enabled::$enabled" > > + - name: skip if the commit or tree was already tested > > + id: skip-if-redundant > > + uses: actions/github-script@v3 > > + if: steps.check-ref.outputs.enabled == 'yes' > > + with: > > + github-token: ${{secrets.GITHUB_TOKEN}} > > + script: | > > + // Figure out workflow ID, commit and tree > > + const { data: run } = await github.actions.getWorkflowRun({ > > + owner: context.repo.owner, > > + repo: context.repo.repo, > > + run_id: context.runId, > > + }); > > + const workflow_id = run.workflow_id; > > + const head_sha = run.head_sha; > > + const tree_id = run.head_commit.tree_id; > > + > > + // See whether there is a successful run for that commit or tree > > + const { data: runs } = await github.actions.listWorkflowRuns({ > > + owner: context.repo.owner, > > + repo: context.repo.repo, > > + per_page: 500, > > + status: 'success', > > + workflow_id, > > + }); > > + for (const run of runs.workflow_runs) { > > + if (head_sha === run.head_sha) { > > + core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`); > > + core.setOutput('enabled', ' but skip'); > > + break; > > + } > > + if (tree_id === run.head_commit.tree_id) { > > + core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); > > + core.setOutput('enabled', ' but skip'); > > + break; > > + } > > + } > > > > windows-build: > > needs: ci-config > > -- > > gitgitgadget > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-09 11:13 ` Johannes Schindelin @ 2020-10-10 7:25 ` SZEDER Gábor 2020-10-11 10:28 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: SZEDER Gábor @ 2020-10-10 7:25 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Johannes Schindelin via GitGitGadget, git On Fri, Oct 09, 2020 at 01:13:03PM +0200, Johannes Schindelin wrote: > Hi Gábor, > > On Fri, 9 Oct 2020, SZEDER Gábor wrote: > > > On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > > > When pushing a commit that has already passed a CI or PR build > > > successfully, it makes sense to save some energy and time and skip the > > > new build. > > > > > > Let's teach our GitHub workflow to do that. > > > > > > For good measure, we also compare the tree ID, which is what we actually > > > test (the commit ID might have changed due to a reworded commit message, > > > which should not affect the outcome of the run). > > > > We have been doing this on Travis CI for a few years now. Does that > > approach not work on GitHub actions? Please explain in the commit > > message why a different approach is taken here. > > You're not being terribly clear about what exactly "We have been doing". > > Are you referring to the `skip_good_tree()` function that stores > information in a file in the `good_trees_file`? Yes, in the commit message you pretty accurately described the exact purpose of that function. > If so, no, we cannot do that anywhere else than on Travis because that > relies on a directory that is somehow shared between runs. And that is a > feature that only Travis offers as far as I know (and it does not come > without issues, e.g. when two concurrent runs try to write to the same > file at the same time). Every branchname-job combination has its own cache, and while the job is running it writes to a local copy of its own cache, so concurrent runs don't seem to be an issue. > Since this strategy relies on a Travis-only feature that does not work on > the three other CI services we use (Cirrus CI, Azure DevOps, GitHub > Actions), I see little point mentioning it in this commit message... This commit duplicates already existing functionality, so, yes, the commit message should definitely have explained why that already existing approach was not suitable for GitHub Actions. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-10 7:25 ` SZEDER Gábor @ 2020-10-11 10:28 ` Johannes Schindelin 2020-10-12 16:12 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-10-11 10:28 UTC (permalink / raw) To: SZEDER Gábor; +Cc: Johannes Schindelin via GitGitGadget, git [-- Attachment #1: Type: text/plain, Size: 1532 bytes --] Hi Gábor, On Sat, 10 Oct 2020, SZEDER Gábor wrote: > On Fri, Oct 09, 2020 at 01:13:03PM +0200, Johannes Schindelin wrote: > > > Since this strategy relies on a Travis-only feature that does not work > > on the three other CI services we use (Cirrus CI, Azure DevOps, GitHub > > Actions), I see little point mentioning it in this commit message... > > This commit duplicates already existing functionality, so, yes, the > commit message should definitely have explained why that already > existing approach was not suitable for GitHub Actions. No, this is not duplicating functionality. The `skip_good_tree()` function requires a persistent directory into which it writes a record of the trees it considers good, based on past runs. It later recalls which trees are considers good and skips the current run if there is a record for this tree. The fact that it requires a persistent directory binds it to Travis CI. As far as I can tell, no other CI service offers that feature (and from where I sit, for good reason, because it is asking for all kinds of fun in concurrent scenarios). What my patch does might duplicate the intention, but absolutely not the functionality. For one, there is no extra record required. It uses the API to query the existing logs. Also, the patch specifically adjusts the GitHub workflow itself. Therefore, unlike the `skip_good_tree()` function, it does not pretend to be generic (which `skip_good_tree()` really is not, as pointed out above). Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-11 10:28 ` Johannes Schindelin @ 2020-10-12 16:12 ` Junio C Hamano 2020-10-12 18:57 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-10-12 16:12 UTC (permalink / raw) To: Johannes Schindelin Cc: SZEDER Gábor, Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Also, the patch specifically adjusts the GitHub workflow itself. > Therefore, unlike the `skip_good_tree()` function, it does not pretend to > be generic (which `skip_good_tree()` really is not, as pointed out above). I think skip_good_tree aspired to be a generic one, by having the "if we are not in travis nor GitHub actions, return early" at its very beginning. The person who adds support to GitHub workflow could have done one of two things. One is to recognise the aspiration, and restructure existing skip_good_tree from skip_good_tree () { return if not travis and if not github actions bunch of code that happens to work only on travis } to skip_good_tree () { if travis then bunch of code that happens to work only on travis else if github actions bunch of code that happens to work only with github fi } without touching the caller. That lets skip_good_tree to be generic. Another is to completely ignore that aspiration, maybe doing all that inside the workflow script (which by definition works only with github). I think the latter (i.e. what the patch choose to do, which is not to bother with the ci/*.sh scripts at all) would be cleaner in this particular case, but then it would have made the intention more clear if the conditional at the beginning of skip_good_tree() were adjusted, perhaps? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-12 16:12 ` Junio C Hamano @ 2020-10-12 18:57 ` Johannes Schindelin 2020-10-15 17:17 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2020-10-12 18:57 UTC (permalink / raw) To: Junio C Hamano Cc: SZEDER Gábor, Johannes Schindelin via GitGitGadget, git Hi Junio, On Mon, 12 Oct 2020, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > Also, the patch specifically adjusts the GitHub workflow itself. > > Therefore, unlike the `skip_good_tree()` function, it does not pretend to > > be generic (which `skip_good_tree()` really is not, as pointed out above). > > I think skip_good_tree aspired to be a generic one, by having the > "if we are not in travis nor GitHub actions, return early" at its > very beginning. [...] it would have made the intention more clear if the > conditional at the beginning of skip_good_tree() were adjusted, perhaps? Full disclosure: I am not even sure what to do with Travis support at this stage (but then, I am not the one to decide over that). In any case, it appears that no matter how generic the `skip_good_tree()` function meant to be, by virtue of using that `cache: directories:` feature (https://docs.travis-ci.com/user/caching) it is limited to Travis CI nevertheless. So if we were to adjust the conditional at the beginning of `skip_good_tree()`, it would probably make sense to make it clear that this not only does not work with GitHub Actions, but in fact it does not work with anything but Travis. Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-12 18:57 ` Johannes Schindelin @ 2020-10-15 17:17 ` Junio C Hamano 2020-10-15 19:39 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2020-10-15 17:17 UTC (permalink / raw) To: Johannes Schindelin Cc: SZEDER Gábor, Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Full disclosure: I am not even sure what to do with Travis support at this > stage (but then, I am not the one to decide over that). Are you talking about the migration to travis-ci.com and having to give them potentially more access to github.com/git/git/ repository? It does worry me, too. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees 2020-10-15 17:17 ` Junio C Hamano @ 2020-10-15 19:39 ` Johannes Schindelin 0 siblings, 0 replies; 16+ messages in thread From: Johannes Schindelin @ 2020-10-15 19:39 UTC (permalink / raw) To: Junio C Hamano Cc: SZEDER Gábor, Johannes Schindelin via GitGitGadget, git Hi Junio, On Thu, 15 Oct 2020, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > Full disclosure: I am not even sure what to do with Travis support at this > > stage (but then, I am not the one to decide over that). > > Are you talking about the migration to travis-ci.com and having to > give them potentially more access to github.com/git/git/ repository? > > It does worry me, too. I meant that, together with worries about the future of Travis CI given that they've been acquired by Idera. Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] ci: do not skip tagged revisions in GitHub workflows 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget 2020-10-08 15:29 ` [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees Johannes Schindelin via GitGitGadget @ 2020-10-08 15:29 ` Johannes Schindelin via GitGitGadget 2020-10-08 21:11 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Junio C Hamano 2 siblings, 0 replies; 16+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2020-10-08 15:29 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> When `master` is tagged, and then both `master` and the tag are pushed, Travis CI will happily build both. That is a waste of energy, which is why we skip the build for `master` in that case. Our GitHub workflow is also triggered by tags. However, the run would fail because the `windows-test` jobs are _not_ skipped on tags, but the `windows-build` job _is skipped (and therefore fails to upload the build artifacts needed by the test jobs). In addition, we just added logic to our GitHub workflow that will skip runs altogether if there is already a successful run for the same commit or at least for the same tree. Let's just change the GitHub workflow to no longer specifically skip tagged revisions. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- ci/lib.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/lib.sh b/ci/lib.sh index 3eefec500d..79f81563cb 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -149,6 +149,7 @@ then CI_REPO_SLUG="$GITHUB_REPOSITORY" CI_JOB_ID="$GITHUB_RUN_ID" CC="${CC:-gcc}" + DONT_SKIP_TAGS=t cache_dir="$HOME/none" @@ -167,6 +168,7 @@ good_trees_file="$cache_dir/good-trees" mkdir -p "$cache_dir" +test -n "${DONT_SKIP_TAGS-}" || skip_branch_tip_with_tag skip_good_tree -- gitgitgadget ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget 2020-10-08 15:29 ` [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees Johannes Schindelin via GitGitGadget 2020-10-08 15:29 ` [PATCH v2 2/2] ci: do not skip tagged revisions in GitHub workflows Johannes Schindelin via GitGitGadget @ 2020-10-08 21:11 ` Junio C Hamano 2 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2020-10-08 21:11 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > * The intention of the tag skipping was replicated by introducing another > check in ci-config: is there a successful workflow run for the same > commit (or at least for the same tree)? If yes, skip, referring to that > successful run. Nice. Our tests and builds do not do anything differently depending on the refname we are on or the exact commit object name for that matter (other than using it in "git version" output), so "has this tree tested?" is good optimization strategy for this particular project. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-10-15 19:39 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-24 17:17 [PATCH] ci: fix GitHub workflow when on a tagged revision Johannes Schindelin via GitGitGadget 2020-04-24 20:50 ` Junio C Hamano 2020-04-24 21:12 ` Johannes Schindelin 2020-04-24 21:24 ` Junio C Hamano 2020-10-08 15:29 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Johannes Schindelin via GitGitGadget 2020-10-08 15:29 ` [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees Johannes Schindelin via GitGitGadget 2020-10-09 7:29 ` SZEDER Gábor 2020-10-09 11:13 ` Johannes Schindelin 2020-10-10 7:25 ` SZEDER Gábor 2020-10-11 10:28 ` Johannes Schindelin 2020-10-12 16:12 ` Junio C Hamano 2020-10-12 18:57 ` Johannes Schindelin 2020-10-15 17:17 ` Junio C Hamano 2020-10-15 19:39 ` Johannes Schindelin 2020-10-08 15:29 ` [PATCH v2 2/2] ci: do not skip tagged revisions in GitHub workflows Johannes Schindelin via GitGitGadget 2020-10-08 21:11 ` [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).