workflows/get-merge-commit: support merge conflicts, run as step in main jobs#410430
Merged
Mic92 merged 4 commits intoNixOS:masterfrom May 24, 2025
Merged
workflows/get-merge-commit: support merge conflicts, run as step in main jobs#410430Mic92 merged 4 commits intoNixOS:masterfrom
Mic92 merged 4 commits intoNixOS:masterfrom
Conversation
…ub-script The reason this was a separate shell script was, that this would be included in multiple workflows separately. But a while ago this had been changed to a re-usable workflow, so we can just as well inline the script. This also allows us to use actions/github-script, which makes for a much more readable script than the bash script before.
When a PR is having conflicts with the base branch, we used to skip most jobs depending on the target branch. With this change, we still run those jobs, but without actually merging the PR temporarily. That means we compare the head of the PR with the merge-base of the PR's branch and the target branch - i.e. the point where the PR branched off. This is not 100% accurate, but that's not important, because after resolving the merge conflicts, those workflows will run again anyway. It allows to give early feedback, though, instead of just skipping all the jobs.
We don't need a separate workflow anymore, because we don't need to skip dependent jobs on failures anymore. The biggest failure mode was "conflict" previously, but we resolved that on the last commit. The remaining failure modes are so rare, that it's OK to just fail the jobs in this case instead of marking them as "skipped". Especially, because the resolve-merge-commit job would have previously failed anyway. By moving this to an action we avoid running separate jobs each time we need the merge commit. This also makes the check list in PRs much cleaner.
…mit action This makes a difference for the case of a merge conflict: In that case, the magic `.../merge` branch actually points to the *last test merge commit* that was successful, which might not contain the latest head commit in any way. Running the tests on that commit is heavily misleading. By using the get-merge-commit action, we run on the PR's head commit in this case, which is much better.
github-actions
bot
added
6.topic: continuous integration
nix-owners
bot
requested review from
LeSuisse,
Mic92,
azuwis,
infinisil,
mweinelt,
philiptaron,
risicle and
zowoq
github-actions
bot
added
10.rebuild-darwin: 0
2 tasks
Contributor
|
Successfully created backport PR for |
1 task
github-actions
bot
added
the
8.has: port to stable
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR addresses two main issues with the
get-merge-commitre-usable workflow:Merge conflicts are now handled by running on the head commit instead of the merge commit (and on the merge-base commit instead of the target commit). This allows running workflows with useful feedback, even while a PR is conflicted.
I had already mentioned the ideas for this PR in #406825 (comment). There's one notable difference: We can't go back to
refs/pull/xxx/merge, instead we must move remaining users off of it as well. That's because I made the following observation: This magic branch uses the same merge commit that is exposed ingithub.event.pull_request.merge_commit_sha(or the API response, respectively). When a new merge commit is trying to be created, themergeablefield changes tonull(that's what we know about already, we use it in the get-merge-commit.sh script). But while that's the case, themerge_commit_shafield keeps the old value. This means it also keeps the old value, when the merge request becomes conflicted. TLDR: Workflows running withrefs/pull/xxx/mergemight run on outdated commits. For example, when you push changes to an already conflicted PR, those workflows will not take your changes into account and just repeat running against the same old temporary merge commit again and again.Thus, we now use the new
get-merge-commitcomposite action in those jobs, too. Since we can't get off of it, I not only moved it to the composite action, but also rewrote the script inactions/github-script, which is considerably shorter and easier to understand than the bash script we had before. ~ 80 lines -> ~ 40 lines.Things done
Add a 👍 reaction to pull requests you find important.