azurelinux icon indicating copy to clipboard operation
azurelinux copied to clipboard

Add merge conflict github PR check

Open mbykhovtsev-ms opened this issue 1 year ago • 3 comments

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • [x] The toolchain has been rebuilt successfully (or no changes were made to it)
  • [x] The toolchain/worker package manifests are up-to-date
  • [x] Any updated packages successfully build (or no packages were changed)
  • [x] Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • [x] Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • [x] All package sources are available
  • [x] cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • [x] LICENSE-MAP files are up-to-date (./LICENSES-AND-NOTICES/SPECS/data/licenses.json, ./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md, ./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)
  • [x] All source files have up-to-date hashes in the *.signatures.json files
  • [x] sudo make go-tidy-all and sudo make go-test-coverage pass
  • [x] Documentation has been updated to match any changes to the build system
  • [x] Ready to merge

Summary

Added merge conflict PR check to catch case of cherry-pick tools committing merge conflicts to fix bug https://microsoft.visualstudio.com/OS/_workitems/edit/53883399

Change Log
  • Added merge conflict PR check to catch case of cherry-pick tools committing merge conflicts
Does this affect the toolchain?

NO

Example of simulated committed merge conflict https://github.com/microsoft/azurelinux/actions/runs/11584391919/job/32251428087

mbykhovtsev-ms avatar Oct 22 '24 21:10 mbykhovtsev-ms

Naive question: have we explored the native facilities that GitHub provides to ensure that changes aren't in conflict / that the PR branch is up to date with incoming changes in the target branch? Or is there some use case that you're targeting that's not covered by those?

reubeno avatar Oct 23 '24 17:10 reubeno

Naive question: have we explored the native facilities that GitHub provides to ensure that changes aren't in conflict / that the PR branch is up to date with incoming changes in the target branch? Or is there some use case that you're targeting that's not covered by those?

Yeah, I am targeting in particular edge case of the fast track cherry-pick tool we have, it is the one that would commit merge conflicts and create PRs with them. A different approach might be to make that tool not create PRs and push merge conflicts in the first place, but then it would block the cherry-pick process via tool when any merge conflicts are present that needs to be resolved and will require it be done manually. I believe such edge case is not entirely rare A PR check was a suggestion in this bug by Henry https://microsoft.visualstudio.com/OS/_workitems/edit/53883399.

What other native facilities that Github provides do you suggest?

mbykhovtsev-ms avatar Oct 23 '24 18:10 mbykhovtsev-ms

Ah, I didn't realize this was primarily for cherry picks; thanks for the explanation. With that context, I'd agree it's a good idea to prevent unresolved (but already merged) conflicts from getting checked in (i.e., what you're doing here). I also think it's a good idea to pursue how to improve the cherry-picking process to avoid them getting committed in the first place, but agree that's more complicated and separable.

reubeno avatar Oct 23 '24 19:10 reubeno