{"thread":{"id":"53310","subject":"[PATCH] ci: fix GitHub workflow when on a tagged revision","startedAt":"2020-04-24T17:17:45Z","lastAt":"2020-10-15T19:39:41Z","messageCount":16,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Johannes Schindelin","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"396157","messageId":"pull.619.git.1587748660308.gitgitgadget@gmail.com","threadId":"53310","inReplyTo":null,"subject":"[PATCH] ci: fix GitHub workflow when on a tagged revision","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-24T17:17:40Z","receivedAt":"2020-04-24T17:17:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen `master` is tagged, and then both `master` and the tag are pushed,\nTravis CI will happily build both. That is a waste of energy, which is\nwhy we skip the build for `master` in that case.\n\nHowever, our GitHub workflow does not trigger on tags, therefore, this\nlogic results in a missing build for that revision.\n\nEven worse: the run would _fail_ because we would skip the Windows\nbuild, there are no artifacts to publish, and therefore no artifacts to\ndownload in the Windows test jobs.\n\nLet's just change the GitHub workflow to skip the logic to skip\nrevisions that are tagged.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    Do not skip tagged revisions in the GitHub workflow runs\n    \n    I noticed a failure in the run for Git for Windows v2.26.2 which was\n    caused by this.\n    \n    This patch is based on dd/ci-swap-azure-pipelines-with-github-actions.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-619%2Fdscho%2Fgithub-workflows-and-tags-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-619/dscho/github-workflows-and-tags-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/619\n\n ci/lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex dac36886e37..f151e2f0ecb 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -1,6 +1,7 @@\n # Library of functions shared by all CI scripts\n \n skip_branch_tip_with_tag () {\n+\ttest -z \"$DONT_SKIP_TAGS\" || return 0\n \t# Sometimes, a branch is pushed at the same time the tag that points\n \t# at the same commit as the tip of the branch is pushed, and building\n \t# both at the same time is a waste.\n@@ -151,6 +152,7 @@ then\n \tCC=\"${CC:-gcc}\"\n \n \tcache_dir=\"$HOME/none\"\n+\tDONT_SKIP_TAGS=t\n \n \texport GIT_PROVE_OPTS=\"--timer --jobs 10\"\n \texport GIT_TEST_OPTS=\"--verbose-log -x\"\n\nbase-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c\n-- \ngitgitgadget\n"},{"id":"396181","messageId":"xmqqlfmkbob4.fsf@gitster.c.googlers.com","threadId":"53310","inReplyTo":"pull.619.git.1587748660308.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci: fix GitHub workflow when on a tagged revision","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-24T20:50:55Z","receivedAt":"2020-04-24T20:51:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> However, our GitHub workflow does not trigger on tags, therefore, this\n> logic results in a missing build for that revision.\n\nThanks for noticing.  The arrangement we had, which essentially said\n\"we know we will always build tagged version, so a push of a branch\nthat happens to have a tagged version at the tip can be ignored\",\nserved us wonderfully.  Now the maintainers of projects (not just\nthis one) are forbidden from tagging the tip of master, queue\nsomething else on top and push the result out, as it won't build or\ntest the tagged version, which is a bit sad.\n\n>  ci/lib.sh | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index dac36886e37..f151e2f0ecb 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -1,6 +1,7 @@\n>  # Library of functions shared by all CI scripts\n>  \n>  skip_branch_tip_with_tag () {\n> +\ttest -z \"$DONT_SKIP_TAGS\" || return 0\n>  \t# Sometimes, a branch is pushed at the same time the tag that points\n>  \t# at the same commit as the tip of the branch is pushed, and building\n>  \t# both at the same time is a waste.\n> @@ -151,6 +152,7 @@ then\n>  \tCC=\"${CC:-gcc}\"\n>  \n>  \tcache_dir=\"$HOME/none\"\n> +\tDONT_SKIP_TAGS=t\n\nQuite straight-forward.  Thanks, will queue.\n\n>  \texport GIT_PROVE_OPTS=\"--timer --jobs 10\"\n>  \texport GIT_TEST_OPTS=\"--verbose-log -x\"\n>\n> base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c\n"},{"id":"396184","messageId":"nycvar.QRO.7.76.6.2004242306450.18039@tvgsbejvaqbjf.bet","threadId":"53310","inReplyTo":"xmqqlfmkbob4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] ci: fix GitHub workflow when on a tagged revision","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-24T21:12:11Z","receivedAt":"2020-04-24T21:12:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Fri, 24 Apr 2020, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > However, our GitHub workflow does not trigger on tags, therefore, this\n> > logic results in a missing build for that revision.\n>\n> Thanks for noticing.  The arrangement we had, which essentially said\n> \"we know we will always build tagged version, so a push of a branch\n> that happens to have a tagged version at the tip can be ignored\",\n> served us wonderfully.  Now the maintainers of projects (not just\n> this one) are forbidden from tagging the tip of master, queue\n> something else on top and push the result out, as it won't build or\n> test the tagged version, which is a bit sad.\n\nNobody forbids this... ;-)\n\nAnd I was wrong, our current GitHub workflow _is_ triggered by tags. See\ne.g. https://github.com/git-for-windows/git/actions/runs/83103626 which\nwas triggered by v2.26.2.windows.1.\n\nHowever, you also see that it failed due to the reason I described in the\ncommit message.\n\nI guess that we should either go with this patch (which would trigger full\nruns also on tags), or we could alternatively trigger only on branch\npushes (and not tag pushes)?\n\nCiao,\nDscho\n\n>\n> >  ci/lib.sh | 2 ++\n> >  1 file changed, 2 insertions(+)\n> >\n> > diff --git a/ci/lib.sh b/ci/lib.sh\n> > index dac36886e37..f151e2f0ecb 100755\n> > --- a/ci/lib.sh\n> > +++ b/ci/lib.sh\n> > @@ -1,6 +1,7 @@\n> >  # Library of functions shared by all CI scripts\n> >\n> >  skip_branch_tip_with_tag () {\n> > +\ttest -z \"$DONT_SKIP_TAGS\" || return 0\n> >  \t# Sometimes, a branch is pushed at the same time the tag that points\n> >  \t# at the same commit as the tip of the branch is pushed, and building\n> >  \t# both at the same time is a waste.\n> > @@ -151,6 +152,7 @@ then\n> >  \tCC=\"${CC:-gcc}\"\n> >\n> >  \tcache_dir=\"$HOME/none\"\n> > +\tDONT_SKIP_TAGS=t\n>\n> Quite straight-forward.  Thanks, will queue.\n>\n> >  \texport GIT_PROVE_OPTS=\"--timer --jobs 10\"\n> >  \texport GIT_TEST_OPTS=\"--verbose-log -x\"\n> >\n> > base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c\n>\n>\n"},{"id":"396187","messageId":"xmqqd07wbmqq.fsf@gitster.c.googlers.com","threadId":"53310","inReplyTo":"nycvar.QRO.7.76.6.2004242306450.18039@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] ci: fix GitHub workflow when on a tagged revision","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-24T21:24:45Z","receivedAt":"2020-04-24T21:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi Junio,\n>\n> On Fri, 24 Apr 2020, Junio C Hamano wrote:\n>\n>> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n>> writes:\n>>\n>> > However, our GitHub workflow does not trigger on tags, therefore, this\n>> > logic results in a missing build for that revision.\n>>\n>> Thanks for noticing.  The arrangement we had, which essentially said\n>> \"we know we will always build tagged version, so a push of a branch\n>> that happens to have a tagged version at the tip can be ignored\",\n>> served us wonderfully.  Now the maintainers of projects (not just\n>> this one) are forbidden from tagging the tip of master, queue\n>> something else on top and push the result out, as it won't build or\n>> test the tagged version, which is a bit sad.\n>\n> Nobody forbids this... ;-)\n>\n> And I was wrong, our current GitHub workflow _is_ triggered by tags. See\n> e.g. https://github.com/git-for-windows/git/actions/runs/83103626 which\n> was triggered by v2.26.2.windows.1.\n>\n> However, you also see that it failed due to the reason I described in the\n> commit message.\n>\n> I guess that we should either go with this patch (which would trigger full\n> runs also on tags), or we could alternatively trigger only on branch\n> pushes (and not tag pushes)?\n\nHmph, even if we were to go with this patch (which should be safer),\nwe'd definitely need an updated log message, then.\n\nThanks for double-checking.\n\n"},{"id":"407157","messageId":"pull.619.v2.git.1602170976.gitgitgadget@gmail.com","threadId":"53310","inReplyTo":"pull.619.git.1587748660308.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-08T15:29:33Z","receivedAt":"2020-10-08T15:29:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Whenever a GitGitGadget Pull Request is sent to the Git mailing list, a tag\nis pushed to gitgitgadget/git to commemorate that iteration.\n\nThe push event caused for that triggers the CI/PR workflow, and reveals a\npretty old bug where the windows-build steps are skipped for tagged\nrevisions, but the windows-test steps are not (and will therefore fail).\n\nThat means, of course, that every GitGitGadget PR is marked with a failed\ntest once it is submitted.\n\nThis patch series is designed to address this issue, and is based on \nam/ci-wsfix (the initial round was based on \ndd/ci-swap-azure-pipelines-with-github-actions but would now cause merge\nconflicts).\n\nChanges since v1:\n\n * Rather than returning early from skip_branch_tip_with_tag(), we now skip\n   the function call altogether when run in a GitHub workflow.\n * The intention of the tag skipping was replicated by introducing another\n   check in ci-config: is there a successful workflow run for the same\n   commit (or at least for the same tree)? If yes, skip, referring to that\n   successful run.\n\nJohannes Schindelin (2):\n  ci: skip GitHub workflow runs for already-tested commits/trees\n  ci: do not skip tagged revisions in GitHub workflows\n\n .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-\n ci/lib.sh                  |  2 ++\n 2 files changed, 40 insertions(+), 1 deletion(-)\n\n\nbase-commit: 055747cd75c0904cc8122e5c12bd45e9f4743c30\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-619%2Fdscho%2Fgithub-workflows-and-tags-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-619/dscho/github-workflows-and-tags-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/619\n\nRange-diff vs v1:\n\n -:  ---------- > 1:  914868d558 ci: skip GitHub workflow runs for already-tested commits/trees\n 1:  d9823f82ee ! 2:  931a2b8482 ci: fix GitHub workflow when on a tagged revision\n     @@ Metadata\n      Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n      \n       ## Commit message ##\n     -    ci: fix GitHub workflow when on a tagged revision\n     +    ci: do not skip tagged revisions in GitHub workflows\n      \n          When `master` is tagged, and then both `master` and the tag are pushed,\n          Travis CI will happily build both. That is a waste of energy, which is\n          why we skip the build for `master` in that case.\n      \n     -    However, our GitHub workflow does not trigger on tags, therefore, this\n     -    logic results in a missing build for that revision.\n     +    Our GitHub workflow is also triggered by tags. However, the run would\n     +    fail because the `windows-test` jobs are _not_ skipped on tags, but the\n     +    `windows-build` job _is skipped (and therefore fails to upload the\n     +    build artifacts needed by the test jobs).\n      \n     -    Even worse: the run would _fail_ because we would skip the Windows\n     -    build, there are no artifacts to publish, and therefore no artifacts to\n     -    download in the Windows test jobs.\n     +    In addition, we just added logic to our GitHub workflow that will skip\n     +    runs altogether if there is already a successful run for the same commit\n     +    or at least for the same tree.\n      \n     -    Let's just change the GitHub workflow to skip the logic to skip\n     -    revisions that are tagged.\n     +    Let's just change the GitHub workflow to no longer specifically skip\n     +    tagged revisions.\n      \n          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n      \n       ## ci/lib.sh ##\n     -@@\n     - # Library of functions shared by all CI scripts\n     - \n     - skip_branch_tip_with_tag () {\n     -+\ttest -z \"$DONT_SKIP_TAGS\" || return 0\n     - \t# Sometimes, a branch is pushed at the same time the tag that points\n     - \t# at the same commit as the tip of the branch is pushed, and building\n     - \t# both at the same time is a waste.\n      @@ ci/lib.sh: then\n     + \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n     + \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n       \tCC=\"${CC:-gcc}\"\n     ++\tDONT_SKIP_TAGS=t\n       \n       \tcache_dir=\"$HOME/none\"\n     -+\tDONT_SKIP_TAGS=t\n       \n     - \texport GIT_PROVE_OPTS=\"--timer --jobs 10\"\n     - \texport GIT_TEST_OPTS=\"--verbose-log -x\"\n     +@@ ci/lib.sh: good_trees_file=\"$cache_dir/good-trees\"\n     + \n     + mkdir -p \"$cache_dir\"\n     + \n     ++test -n \"${DONT_SKIP_TAGS-}\" ||\n     + skip_branch_tip_with_tag\n     + skip_good_tree\n     + \n\n-- \ngitgitgadget\n"},{"id":"407156","messageId":"914868d558b1aa8ebec6e9196c5ae83a2bd566bf.1602170976.git.gitgitgadget@gmail.com","threadId":"53310","inReplyTo":"pull.619.v2.git.1602170976.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-08T15:29:34Z","receivedAt":"2020-10-08T15:29:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen pushing a commit that has already passed a CI or PR build\nsuccessfully, it makes sense to save some energy and time and skip the\nnew build.\n\nLet's teach our GitHub workflow to do that.\n\nFor good measure, we also compare the tree ID, which is what we actually\ntest (the commit ID might have changed due to a reworded commit message,\nwhich should not affect the outcome of the run).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-\n 1 file changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 5bd321e5e1..7391b46d61 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -9,7 +9,7 @@ jobs:\n   ci-config:\n     runs-on: ubuntu-latest\n     outputs:\n-      enabled: ${{ steps.check-ref.outputs.enabled }}\n+      enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}\n     steps:\n       - name: try to clone ci-config branch\n         continue-on-error: true\n@@ -35,6 +35,43 @@ jobs:\n             enabled=no\n           fi\n           echo \"::set-output name=enabled::$enabled\"\n+      - name: skip if the commit or tree was already tested\n+        id: skip-if-redundant\n+        uses: actions/github-script@v3\n+        if: steps.check-ref.outputs.enabled == 'yes'\n+        with:\n+          github-token: ${{secrets.GITHUB_TOKEN}}\n+          script: |\n+            // Figure out workflow ID, commit and tree\n+            const { data: run } = await github.actions.getWorkflowRun({\n+              owner: context.repo.owner,\n+              repo: context.repo.repo,\n+              run_id: context.runId,\n+            });\n+            const workflow_id = run.workflow_id;\n+            const head_sha = run.head_sha;\n+            const tree_id = run.head_commit.tree_id;\n+\n+            // See whether there is a successful run for that commit or tree\n+            const { data: runs } = await github.actions.listWorkflowRuns({\n+              owner: context.repo.owner,\n+              repo: context.repo.repo,\n+              per_page: 500,\n+              status: 'success',\n+              workflow_id,\n+            });\n+            for (const run of runs.workflow_runs) {\n+              if (head_sha === run.head_sha) {\n+                core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`);\n+                core.setOutput('enabled', ' but skip');\n+                break;\n+              }\n+              if (tree_id === run.head_commit.tree_id) {\n+                core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);\n+                core.setOutput('enabled', ' but skip');\n+                break;\n+              }\n+            }\n \n   windows-build:\n     needs: ci-config\n-- \ngitgitgadget\n\n"},{"id":"407158","messageId":"931a2b8482dd10b64b506e70539ee1bf9f8cf2e7.1602170976.git.gitgitgadget@gmail.com","threadId":"53310","inReplyTo":"pull.619.v2.git.1602170976.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] ci: do not skip tagged revisions in GitHub workflows","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-08T15:29:35Z","receivedAt":"2020-10-08T15:29:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nWhen `master` is tagged, and then both `master` and the tag are pushed,\nTravis CI will happily build both. That is a waste of energy, which is\nwhy we skip the build for `master` in that case.\n\nOur GitHub workflow is also triggered by tags. However, the run would\nfail because the `windows-test` jobs are _not_ skipped on tags, but the\n`windows-build` job _is skipped (and therefore fails to upload the\nbuild artifacts needed by the test jobs).\n\nIn addition, we just added logic to our GitHub workflow that will skip\nruns altogether if there is already a successful run for the same commit\nor at least for the same tree.\n\nLet's just change the GitHub workflow to no longer specifically skip\ntagged revisions.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n ci/lib.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 3eefec500d..79f81563cb 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -149,6 +149,7 @@ then\n \tCI_REPO_SLUG=\"$GITHUB_REPOSITORY\"\n \tCI_JOB_ID=\"$GITHUB_RUN_ID\"\n \tCC=\"${CC:-gcc}\"\n+\tDONT_SKIP_TAGS=t\n \n \tcache_dir=\"$HOME/none\"\n \n@@ -167,6 +168,7 @@ good_trees_file=\"$cache_dir/good-trees\"\n \n mkdir -p \"$cache_dir\"\n \n+test -n \"${DONT_SKIP_TAGS-}\" ||\n skip_branch_tip_with_tag\n skip_good_tree\n \n-- \ngitgitgadget\n"},{"id":"407186","messageId":"xmqqblhcwhh3.fsf@gitster.c.googlers.com","threadId":"53310","inReplyTo":"pull.619.v2.git.1602170976.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] Do not skip tagged revisions in the GitHub workflow runs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-08T21:11:36Z","receivedAt":"2020-10-08T21:11:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n>  * The intention of the tag skipping was replicated by introducing another\n>    check in ci-config: is there a successful workflow run for the same\n>    commit (or at least for the same tree)? If yes, skip, referring to that\n>    successful run.\n\nNice.\n\nOur tests and builds do not do anything differently depending on the\nrefname we are on or the exact commit object name for that matter\n(other than using it in \"git version\" output), so \"has this tree\ntested?\" is good optimization strategy for this particular project.\n"},{"id":"407198","messageId":"20201009072922.GC24813@szeder.dev","threadId":"53310","inReplyTo":"914868d558b1aa8ebec6e9196c5ae83a2bd566bf.1602170976.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-10-09T07:29:22Z","receivedAt":"2020-10-09T07:29:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> When pushing a commit that has already passed a CI or PR build\n> successfully, it makes sense to save some energy and time and skip the\n> new build.\n> \n> Let's teach our GitHub workflow to do that.\n> \n> For good measure, we also compare the tree ID, which is what we actually\n> test (the commit ID might have changed due to a reworded commit message,\n> which should not affect the outcome of the run).\n\nWe have been doing this on Travis CI for a few years now.  Does that\napproach not work on GitHub actions?  Please explain in the commit\nmessage why a different approach is taken here.\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 38 insertions(+), 1 deletion(-)\n> \n> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> index 5bd321e5e1..7391b46d61 100644\n> --- a/.github/workflows/main.yml\n> +++ b/.github/workflows/main.yml\n> @@ -9,7 +9,7 @@ jobs:\n>    ci-config:\n>      runs-on: ubuntu-latest\n>      outputs:\n> -      enabled: ${{ steps.check-ref.outputs.enabled }}\n> +      enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}\n>      steps:\n>        - name: try to clone ci-config branch\n>          continue-on-error: true\n> @@ -35,6 +35,43 @@ jobs:\n>              enabled=no\n>            fi\n>            echo \"::set-output name=enabled::$enabled\"\n> +      - name: skip if the commit or tree was already tested\n> +        id: skip-if-redundant\n> +        uses: actions/github-script@v3\n> +        if: steps.check-ref.outputs.enabled == 'yes'\n> +        with:\n> +          github-token: ${{secrets.GITHUB_TOKEN}}\n> +          script: |\n> +            // Figure out workflow ID, commit and tree\n> +            const { data: run } = await github.actions.getWorkflowRun({\n> +              owner: context.repo.owner,\n> +              repo: context.repo.repo,\n> +              run_id: context.runId,\n> +            });\n> +            const workflow_id = run.workflow_id;\n> +            const head_sha = run.head_sha;\n> +            const tree_id = run.head_commit.tree_id;\n> +\n> +            // See whether there is a successful run for that commit or tree\n> +            const { data: runs } = await github.actions.listWorkflowRuns({\n> +              owner: context.repo.owner,\n> +              repo: context.repo.repo,\n> +              per_page: 500,\n> +              status: 'success',\n> +              workflow_id,\n> +            });\n> +            for (const run of runs.workflow_runs) {\n> +              if (head_sha === run.head_sha) {\n> +                core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`);\n> +                core.setOutput('enabled', ' but skip');\n> +                break;\n> +              }\n> +              if (tree_id === run.head_commit.tree_id) {\n> +                core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);\n> +                core.setOutput('enabled', ' but skip');\n> +                break;\n> +              }\n> +            }\n>  \n>    windows-build:\n>      needs: ci-config\n> -- \n> gitgitgadget\n> \n"},{"id":"407210","messageId":"nycvar.QRO.7.76.6.2010091254180.50@tvgsbejvaqbjf.bet","threadId":"53310","inReplyTo":"20201009072922.GC24813@szeder.dev","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-09T11:13:03Z","receivedAt":"2020-10-09T13:30:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Fri, 9 Oct 2020, SZEDER Gábor wrote:\n\n> On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > When pushing a commit that has already passed a CI or PR build\n> > successfully, it makes sense to save some energy and time and skip the\n> > new build.\n> >\n> > Let's teach our GitHub workflow to do that.\n> >\n> > For good measure, we also compare the tree ID, which is what we actually\n> > test (the commit ID might have changed due to a reworded commit message,\n> > which should not affect the outcome of the run).\n>\n> We have been doing this on Travis CI for a few years now.  Does that\n> approach not work on GitHub actions?  Please explain in the commit\n> message why a different approach is taken here.\n\nYou're not being terribly clear about what exactly \"We have been doing\".\n\nAre you referring to the `skip_good_tree()` function that stores\ninformation in a file in the `good_trees_file`?\n\nIf so, no, we cannot do that anywhere else than on Travis because that\nrelies on a directory that is somehow shared between runs. And that is a\nfeature that only Travis offers as far as I know (and it does not come\nwithout issues, e.g. when two concurrent runs try to write to the same\nfile at the same time).\n\nSince this strategy relies on a Travis-only feature that does not work on\nthe three other CI services we use (Cirrus CI, Azure DevOps, GitHub\nActions), I see little point mentioning it in this commit message...\n\nHowever, I might be very well wrong on that assessment.\n\nCiao,\nDscho\n\n>\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-\n> >  1 file changed, 38 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\n> > index 5bd321e5e1..7391b46d61 100644\n> > --- a/.github/workflows/main.yml\n> > +++ b/.github/workflows/main.yml\n> > @@ -9,7 +9,7 @@ jobs:\n> >    ci-config:\n> >      runs-on: ubuntu-latest\n> >      outputs:\n> > -      enabled: ${{ steps.check-ref.outputs.enabled }}\n> > +      enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }}\n> >      steps:\n> >        - name: try to clone ci-config branch\n> >          continue-on-error: true\n> > @@ -35,6 +35,43 @@ jobs:\n> >              enabled=no\n> >            fi\n> >            echo \"::set-output name=enabled::$enabled\"\n> > +      - name: skip if the commit or tree was already tested\n> > +        id: skip-if-redundant\n> > +        uses: actions/github-script@v3\n> > +        if: steps.check-ref.outputs.enabled == 'yes'\n> > +        with:\n> > +          github-token: ${{secrets.GITHUB_TOKEN}}\n> > +          script: |\n> > +            // Figure out workflow ID, commit and tree\n> > +            const { data: run } = await github.actions.getWorkflowRun({\n> > +              owner: context.repo.owner,\n> > +              repo: context.repo.repo,\n> > +              run_id: context.runId,\n> > +            });\n> > +            const workflow_id = run.workflow_id;\n> > +            const head_sha = run.head_sha;\n> > +            const tree_id = run.head_commit.tree_id;\n> > +\n> > +            // See whether there is a successful run for that commit or tree\n> > +            const { data: runs } = await github.actions.listWorkflowRuns({\n> > +              owner: context.repo.owner,\n> > +              repo: context.repo.repo,\n> > +              per_page: 500,\n> > +              status: 'success',\n> > +              workflow_id,\n> > +            });\n> > +            for (const run of runs.workflow_runs) {\n> > +              if (head_sha === run.head_sha) {\n> > +                core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`);\n> > +                core.setOutput('enabled', ' but skip');\n> > +                break;\n> > +              }\n> > +              if (tree_id === run.head_commit.tree_id) {\n> > +                core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`);\n> > +                core.setOutput('enabled', ' but skip');\n> > +                break;\n> > +              }\n> > +            }\n> >\n> >    windows-build:\n> >      needs: ci-config\n> > --\n> > gitgitgadget\n> >\n>\n"},{"id":"407261","messageId":"20201010072508.GD24813@szeder.dev","threadId":"53310","inReplyTo":"nycvar.QRO.7.76.6.2010091254180.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-10-10T07:25:08Z","receivedAt":"2020-10-10T07:28:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Oct 09, 2020 at 01:13:03PM +0200, Johannes Schindelin wrote:\n> Hi Gábor,\n> \n> On Fri, 9 Oct 2020, SZEDER Gábor wrote:\n> \n> > On Thu, Oct 08, 2020 at 03:29:34PM +0000, Johannes Schindelin via GitGitGadget wrote:\n> > > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > >\n> > > When pushing a commit that has already passed a CI or PR build\n> > > successfully, it makes sense to save some energy and time and skip the\n> > > new build.\n> > >\n> > > Let's teach our GitHub workflow to do that.\n> > >\n> > > For good measure, we also compare the tree ID, which is what we actually\n> > > test (the commit ID might have changed due to a reworded commit message,\n> > > which should not affect the outcome of the run).\n> >\n> > We have been doing this on Travis CI for a few years now.  Does that\n> > approach not work on GitHub actions?  Please explain in the commit\n> > message why a different approach is taken here.\n> \n> You're not being terribly clear about what exactly \"We have been doing\".\n> \n> Are you referring to the `skip_good_tree()` function that stores\n> information in a file in the `good_trees_file`?\n\nYes, in the commit message you pretty accurately described the exact\npurpose of that function.\n\n> If so, no, we cannot do that anywhere else than on Travis because that\n> relies on a directory that is somehow shared between runs. And that is a\n> feature that only Travis offers as far as I know (and it does not come\n> without issues, e.g. when two concurrent runs try to write to the same\n> file at the same time).\n\nEvery branchname-job combination has its own cache, and while the job\nis running it writes to a local copy of its own cache, so concurrent\nruns don't seem to be an issue.\n\n> Since this strategy relies on a Travis-only feature that does not work on\n> the three other CI services we use (Cirrus CI, Azure DevOps, GitHub\n> Actions), I see little point mentioning it in this commit message...\n\nThis commit duplicates already existing functionality, so, yes, the\ncommit message should definitely have explained why that already\nexisting approach was not suitable for GitHub Actions.\n\n"},{"id":"407318","messageId":"nycvar.QRO.7.76.6.2010111221350.50@tvgsbejvaqbjf.bet","threadId":"53310","inReplyTo":"20201010072508.GD24813@szeder.dev","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-11T10:28:33Z","receivedAt":"2020-10-12T09:38:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Gábor,\n\nOn Sat, 10 Oct 2020, SZEDER Gábor wrote:\n\n> On Fri, Oct 09, 2020 at 01:13:03PM +0200, Johannes Schindelin wrote:\n>\n> > Since this strategy relies on a Travis-only feature that does not work\n> > on the three other CI services we use (Cirrus CI, Azure DevOps, GitHub\n> > Actions), I see little point mentioning it in this commit message...\n>\n> This commit duplicates already existing functionality, so, yes, the\n> commit message should definitely have explained why that already\n> existing approach was not suitable for GitHub Actions.\n\nNo, this is not duplicating functionality.\n\nThe `skip_good_tree()` function requires a persistent directory into which\nit writes a record of the trees it considers good, based on past runs. It\nlater recalls which trees are considers good and skips the current run if\nthere is a record for this tree.\n\nThe fact that it requires a persistent directory binds it to Travis CI. As\nfar as I can tell, no other CI service offers that feature (and from where\nI sit, for good reason, because it is asking for all kinds of fun in\nconcurrent scenarios).\n\nWhat my patch does might duplicate the intention, but absolutely not the\nfunctionality. For one, there is no extra record required. It uses the API\nto query the existing logs.\n\nAlso, the patch specifically adjusts the GitHub workflow itself.\nTherefore, unlike the `skip_good_tree()` function, it does not pretend to\nbe generic (which `skip_good_tree()` really is not, as pointed out above).\n\nCiao,\nDscho\n"},{"id":"407342","messageId":"xmqqy2kbqv7b.fsf@gitster.c.googlers.com","threadId":"53310","inReplyTo":"nycvar.QRO.7.76.6.2010111221350.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-12T16:12:56Z","receivedAt":"2020-10-12T16:13:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Also, the patch specifically adjusts the GitHub workflow itself.\n> Therefore, unlike the `skip_good_tree()` function, it does not pretend to\n> be generic (which `skip_good_tree()` really is not, as pointed out above).\n\nI think skip_good_tree aspired to be a generic one, by having the\n\"if we are not in travis nor GitHub actions, return early\" at its\nvery beginning.  The person who adds support to GitHub workflow\ncould have done one of two things.  \n\nOne is to recognise the aspiration, and restructure existing\nskip_good_tree from\n\n    skip_good_tree () {\n\treturn if not travis and if not github actions\n\n\tbunch of code that happens to work only on travis\n    }\n\nto\n\n    skip_good_tree () {\n\tif travis\n\tthen\n\t    bunch of code that happens to work only on travis\n\telse if github actions\n\t    bunch of code that happens to work only with github\n\tfi\n    }\n\nwithout touching the caller.  That lets skip_good_tree to be\ngeneric.\n\nAnother is to completely ignore that aspiration, maybe doing all\nthat inside the workflow script (which by definition works only with\ngithub).\n\nI think the latter (i.e. what the patch choose to do, which is not\nto bother with the ci/*.sh scripts at all) would be cleaner in this\nparticular case, but then it would have made the intention more\nclear if the conditional at the beginning of skip_good_tree() were\nadjusted, perhaps?\n"},{"id":"407365","messageId":"nycvar.QRO.7.76.6.2010122044460.50@tvgsbejvaqbjf.bet","threadId":"53310","inReplyTo":"xmqqy2kbqv7b.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-12T18:57:46Z","receivedAt":"2020-10-12T18:57:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 12 Oct 2020, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > Also, the patch specifically adjusts the GitHub workflow itself.\n> > Therefore, unlike the `skip_good_tree()` function, it does not pretend to\n> > be generic (which `skip_good_tree()` really is not, as pointed out above).\n>\n> I think skip_good_tree aspired to be a generic one, by having the\n> \"if we are not in travis nor GitHub actions, return early\" at its\n> very beginning. [...] it would have made the intention more clear if the\n> conditional at the beginning of skip_good_tree() were adjusted, perhaps?\n\nFull disclosure: I am not even sure what to do with Travis support at this\nstage (but then, I am not the one to decide over that).\n\nIn any case, it appears that no matter how generic the `skip_good_tree()`\nfunction meant to be, by virtue of using that `cache: directories:`\nfeature (https://docs.travis-ci.com/user/caching) it is limited to Travis\nCI nevertheless.\n\nSo if we were to adjust the conditional at the beginning of\n`skip_good_tree()`, it would probably make sense to make it clear that\nthis not only does not work with GitHub Actions, but in fact it does not\nwork with anything but Travis.\n\nCiao,\nDscho\n"},{"id":"407632","messageId":"xmqq4kmvh0i1.fsf@gitster.c.googlers.com","threadId":"53310","inReplyTo":"nycvar.QRO.7.76.6.2010122044460.50@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-15T17:17:42Z","receivedAt":"2020-10-15T17:17:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Full disclosure: I am not even sure what to do with Travis support at this\n> stage (but then, I am not the one to decide over that).\n\nAre you talking about the migration to travis-ci.com and having to\ngive them potentially more access to github.com/git/git/ repository?\n\nIt does worry me, too.\n"},{"id":"407667","messageId":"nycvar.QRO.7.76.6.2010152134590.56@tvgsbejvaqbjf.bet","threadId":"53310","inReplyTo":"xmqq4kmvh0i1.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 1/2] ci: skip GitHub workflow runs for already-tested commits/trees","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-10-15T19:39:32Z","receivedAt":"2020-10-15T19:39:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 15 Oct 2020, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> > Full disclosure: I am not even sure what to do with Travis support at this\n> > stage (but then, I am not the one to decide over that).\n>\n> Are you talking about the migration to travis-ci.com and having to\n> give them potentially more access to github.com/git/git/ repository?\n>\n> It does worry me, too.\n\nI meant that, together with worries about the future of Travis CI given\nthat they've been acquired by Idera.\n\nCiao,\nDscho\n"}]}