ci: fix GitHub workflow when on a tagged revision

However, our GitHub workflow does not trigger on tags, therefore, this logic results in a missing build for that revision.

 ci/lib.sh | 2 ++
 1 file changed, 2 insertions(+)

ci/lib.sh +1 -0

index dac36886e37..f151e2f0ecb 100755
@@ -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.

ci/lib.sh +0 -0

 	export GIT_PROVE_OPTS="--timer --jobs 10"
 	export GIT_TEST_OPTS="--verbose-log -x"
 

base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · 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.

Quoted 17 lines

 ci/lib.sh | 2 ++
 1 file changed, 2 insertions(+)

ci/lib.sh +1 -0

index dac36886e37..f151e2f0ecb 100755
@@ -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.

ci/lib.sh then · line 155

Quoted 4 lines

ci/lib.sh +1 -0

@@ -151,6 +152,7 @@ then
 	CC="${CC:-gcc}"
 
 	cache_dir="$HOME/none"
+	DONT_SKIP_TAGS=t

Quite straight-forward. Thanks, will queue.

Quoted 4 lines

ci/lib.sh +0 -0

 	export GIT_PROVE_OPTS="--timer --jobs 10"
 	export GIT_TEST_OPTS="--verbose-log -x"
 

base-commit: f72f328bc57e1b0db380ef76e0c1e94a9ed0ac7c

Hi Junio,

Quoted 6 lines · Junio C Hamano

"Johannes Schindelin via GitGitGadget" <[email protected]>

Quoted 2 lines · writes:

However, our GitHub workflow does not trigger on tags, therefore, this logic results in a missing build for that revision.

Quoted 7 lines

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)?

From: Johannes Schindelin <[email protected]>

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 <[email protected]>

---
 .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

.github/workflows/main.yml +38 -1

index 5bd321e5e1..7391b46d61 100644
@@ -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

Signature · 1 line

gitgitgadget

Quoted 8 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

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.

Quoted 3 lines

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.

Quoted 66 lines

Signed-off-by Johannes Schindelin <[email protected]>

---
 .github/workflows/main.yml | 39 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

.github/workflows/main.yml +38 -2

index 5bd321e5e1..7391b46d61 100644
@@ -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

Hi Gábor,

Quoted 13 lines · SZEDER Gábor

Quoted 11 lines · Johannes Schindelin via GitGitGadget

From: Johannes Schindelin <[email protected]>

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).

Quoted 3 lines

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...

Quoted 3 lines · Johannes Schindelin

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.

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).

Quoted 3 lines · Johannes Schindelin

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).

Quoted 2 lines · Johannes Schindelin

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.

* 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.