Fix upload-snapshot on push
#128
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using `tag-git` would overwrite the existing one! Signed-off-by: Johannes Schindelin <[email protected]>
The `getToken()` method does not need any parameters when being called in `slash-commands.js` because there, the `owner`/`repo` values are obvious. But when triggering an `upload-snapshot` workflow, we are juggling three different repositories, therefore we absolutely need to specify for which repository the token needs to target. This was the _actual_ reason why the `upload-snapshot` run was not triggered after v2.48.1 was released, the conclusions I drew in #124 were incorrect. Signed-off-by: Johannes Schindelin <[email protected]>
When handling a `push` event on `git-for-windows/git`, it is necessary to validate that the `tag-git` Check Run has the information needed to determine the `git-artifacts` runs. If that information is not there (e.g. because `tag-git` was accidentally updated and now contains invalid/unparseable data), we should error out earlier. Signed-off-by: Johannes Schindelin <[email protected]>
When a `push` wants to trigger a new snapshot to be uploaded using already-built Git artifacts (which is the case when Git for Windows' "Rebase to <git-version>" PRs are "merged" by updating the `main` ref), after verifying that there is a valid `tag-git` Check Run, GitForWindowsHelper wants to find the associated `git-artifacts` workflow runs. They can be identified using the `tag-git` _workflow_ run, though, i.e. the ID of the workflow as run in `git-for-windows-automation`, not the mirrored _Check Run_ in `git-for-windows/git`. Signed-off-by: Johannes Schindelin <[email protected]>
It is the `git-for-windows` org, not the `git` one in which the `git-for-windows-automation` workflows live and run... Signed-off-by: Johannes Schindelin <[email protected]>
…sions" A couple of weeks ago, when I analyzed why no `upload-snapshots` workflow run was triggered after "merging" the v2.48.1 PR (for details, see #124), I concluded mistakenly that updating the `main` ref would not issue a `push` webhook event. However, the `push` webhook event _is_ triggered. It was simply not handled correctly. Now that I fixed these issues over the course of the preceding commits, it will work. And therefore the code added in above-mentioned PR needs to be dropped lest two `upload-snapshot` workflow runs are triggered to run concurrently. This reverts commit c2b466e (snapshot uploading: do upload the regular Git for Windows versions, 2025-02-13). Signed-off-by: Johannes Schindelin <[email protected]>
In 64b47fb (test-pr-comment-delivery: have an explicit way to avoid triggering runs, 2025-02-13), I modified the script so that no workflow runs would be triggered accidentally. The idea was to avoid changing state on github.com while debugging webhook event deliveries on my local machine, even though information may need to be looked up on github.com using the GitHub App credentials. What I missed was that there are more ways to mess up said state, e.g. by queuing Check Runs. This actually happened to me, where an accidentally-incorrect `tag-git` label was used when queuing the Check Run for an `upload-snapshot` workflow run, and it overwrote the existing one, but the existing one was very much needed to figure out which `git-artifacts` runs to use, and it took me over an hour to reconstruct everything into a healthy shape. Let's prevent future mishaps like that. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
to git-for-windows/git-snapshots
that referenced
this pull request
Mar 11, 2025
There has been a problem with the `upload-snapshot` workflow, fixed in git-for-windows/gfw-helper-github-app#128, that prevented the v2.49.0-rc* releases from being added. I manually ran the automation for -rc2, but for -rc0 and -rc1 I want to use a different approach: I want to cross-link to the existing releases in `git-for-windows/git`. The new `--backfill-release=<tag-name>` option allows just that. Signed-off-by: Johannes Schindelin <[email protected]>
mjcheetham
approved these changes
Mar 12, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are two situations when the
upload-snapshotworkflow needs to be triggered:git-for-windows/gitPR is merged, which means thatmainis updated.tag-gitworkflow run with thesnapshotflag set totrue.tag-gitrun completes, GitForWindowsHelper triggersgit-artifactsworkflow runs for x86_64, aarch64 and i686 each.git-artifactsruns completes with success, GitForWindowsHelper triggers theupload-snapshotworkflow run./git-artifactsslash command triggers thetag-gitworkflow, this time withsnapshotset tofalse.tag-gitrun completes successfully, thegit-artifactsworkflow runs are triggered.git-artifactsruns completes with success, the PR author is expected to run the manual tests./release.release-gitworkflow, which creates a new GitHub Release, among other things.release-gitworkflow run concludes successfully, GitForWindowsHelper "merges" the PR by updating themainref.pushwebhook event, which GitForWindowsHelper needs to handle by triggering theupload-snapshotworkflow run.The difference between these two situations is, of course, that in the latter case, there are already the release artifacts, while in the former case, snapshot artifacts first need to be built.
Those code paths are slightly different; The snapshot artifacts case requires a far longer cascade which GitForWindowsHelper needs orchestrate by reacting to 5 webhook events (
push,tag-gitcompletion, 3xgit-artifactscompletion).The "easier" code path is the one where the release artifacts are already there, and the
upload-snapshotworkflow run can be triggered immediately after thepushevent. However, that code path was broken, so that when the v2.48.1 PR was "merged", noupload-snapshotworkflow run was triggered. My analysis was incorrect and I missed that there probably had been apushevent. Based on my incorrect conclusion, I opened #124.Since essentially the same code path was still responsible to handle the release artifacts scenario, unsurprisingly no
upload-snapshotwas triggered since #124 was merged. Meaning: We miss snapshots for v2.49.0-rc0 and v2.49.0-rc1 (and we missed v2.49.0-rc2 until I worked on this PR and ran the steps manually to upload the v2.49.0-rc2 snapshot by way of proving that my fixes are correct this time).When I realized that the v2.49.0-rc2 snapshot had not been uploaded, I investigated, and found two webhook events that resulted in failure: the
pushone (which I had assumed would not trigger on updating a ref), and theworkflow_run.completeone (handling the completedrelease-gitworkflow run, logic which I introduced in #124).This here PR fixes those woes, and then reverts the change introduced in #124 to avoid double-triggering the
upload-snapshotworkflow run.This closes git-for-windows/git#5482