{"thread":{"id":"54392","subject":"[PATCH 0/2] ci: make the new GitHub workflow logic more defensive","startedAt":"2020-10-11T21:27:15Z","lastAt":"2020-10-12T19:29:58Z","messageCount":5,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"407308","messageId":"pull.748.git.1602451631.gitgitgadget@gmail.com","threadId":"54392","inReplyTo":null,"subject":"[PATCH 0/2] ci: make the new GitHub workflow logic more defensive","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-11T21:27:09Z","receivedAt":"2020-10-11T21:27:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Phil just reported that the check I added failed miserably\n[https://github.com/phil-blain/git/runs/1238660958] in their fork, and the \nentire run would be abandoned. That's totally unintended, and was caused by\na different metadata format for older recorded runs.\n\nLet's work around this particular issue, and then make the entire check a\nlot more robust.\n\nThis patch series is based on js/ci-ghwf-dedup-tests.\n\nJohannes Schindelin (2):\n ci: work around old records of GitHub runs\n ci: make the \"skip-if-redundant\" check more defensive\n\n .github/workflows/main.yml | 58 ++++++++++++++++++++------------------\n 1 file changed, 31 insertions(+), 27 deletions(-)\n\n\nbase-commit: 4463ce75b7eea47f9b484b05957def655d3f46d5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-748%2Fdscho%2Fci-ghwf-dedup-tests-defensive-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-748/dscho/ci-ghwf-dedup-tests-defensive-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/748\n-- \ngitgitgadget\n"},{"id":"407309","messageId":"6c0ba7ec05a41c38176571d675cc9d1ff298ecc5.1602451631.git.gitgitgadget@gmail.com","threadId":"54392","inReplyTo":"pull.748.git.1602451631.gitgitgadget@gmail.com","subject":"[PATCH 1/2] ci: work around old records of GitHub runs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-11T21:27:10Z","receivedAt":"2020-10-11T21:27:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nApparently older GitHub runs at least _sometimes_ lack information about\nthe `head_commit` (and therefore the `ci-config` check will fail with\n\"TypeError: Cannot read property 'tree_id' of null\") in the check added\nin 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested\ncommits/trees, 2020-10-08).\n\nLet's work around this by adding a defensive condition.\n\nReported-by: Philippe Blain \nSigned-off-by: Johannes Schindelin \n---\n .github/workflows/main.yml | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 0a9acb6a19..d4298878f5 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -65,7 +65,7 @@ jobs:\n core.setOutput('enabled', ' but skip');\n break;\n }\n- if (tree_id === run.head_commit.tree_id) {\n+ if (run.head_commit && 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-- \ngitgitgadget\n\n"},{"id":"407310","messageId":"5c889e487d344efd1a0eb0cb5b717375de450c89.1602451631.git.gitgitgadget@gmail.com","threadId":"54392","inReplyTo":"pull.748.git.1602451631.gitgitgadget@gmail.com","subject":"[PATCH 2/2] ci: make the \"skip-if-redundant\" check more defensive","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-11T21:27:11Z","receivedAt":"2020-10-11T21:27:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nIn 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested\ncommits/trees, 2020-10-08), we added a check that determines whether\nthere is already a workflow run for the given commit (or at least tree),\nand if found, skips the current run.\n\nWe just worked around an issue with this check where older runs might\nunexpectedly miss the `head_commit` attribute.\n\nLet's be even more defensive by catching all kinds of exceptions,\nlogging them as warnings, and continue the run without skipping it\n(after all, if the check fails, we _want_ to continue with the run).\n\nThis commit is best viewed with the diff option `-w` because it\nincreases the indentation level of the GitHub Action script by two\nspaces, surrounding it by a `try ... catch` construct.\n\nSigned-off-by: Johannes Schindelin \n---\n .github/workflows/main.yml | 58 ++++++++++++++++++++------------------\n 1 file changed, 31 insertions(+), 27 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex d4298878f5..9dc359bc23 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -41,35 +41,39 @@ jobs:\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+ try {\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 (run.head_commit && 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+ // 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 (run.head_commit && 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+ } catch (e) {\n+ core.warning(e);\n }\n \n windows-build:\n-- \ngitgitgadget\n"},{"id":"407371","messageId":"xmqqblh7p7n9.fsf@gitster.c.googlers.com","threadId":"54392","inReplyTo":"pull.748.git.1602451631.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] ci: make the new GitHub workflow logic more defensive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-12T19:27:06Z","receivedAt":"2020-10-12T19:27:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Phil just reported that the check I added failed miserably\n> [https://github.com/phil-blain/git/runs/1238660958] in their fork, and the \n> entire run would be abandoned. That's totally unintended, and was caused by\n> a different metadata format for older recorded runs.\n\nThanks for a quick response to breakage.\n\n> Let's work around this particular issue, and then make the entire check a\n> lot more robust.\n>\n> This patch series is based on js/ci-ghwf-dedup-tests.\n\nWill queue.\n"},{"id":"407372","messageId":"xmqq7drvp7in.fsf@gitster.c.googlers.com","threadId":"54392","inReplyTo":"5c889e487d344efd1a0eb0cb5b717375de450c89.1602451631.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] ci: make the \"skip-if-redundant\" check more defensive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-12T19:29:52Z","receivedAt":"2020-10-12T19:29:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> Let's be even more defensive by catching all kinds of exceptions,\n> logging them as warnings, and continue the run without skipping it\n> (after all, if the check fails, we _want_ to continue with the run).\n\nYeah, looks sensible.\n"}]}