From f6312f1322bd54138163c559cc89e298d4b5e543 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Wed, 13 Apr 2022 18:07:35 +0100 Subject: [PATCH 1/3] Commit any conflicts during v1 backport to simplify release process The process of creating the v1 release can run into merge conflicts. We commit the unresolved conflicts so a maintainer can easily resolve them (vs erroring and requiring maintainers to reconstruct the release manually). --- .github/update-release-branch.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/.github/update-release-branch.py b/.github/update-release-branch.py index 393eb7ec9a..f247695c4a 100644 --- a/.github/update-release-branch.py +++ b/.github/update-release-branch.py @@ -23,11 +23,12 @@ ORIGIN = 'origin' # Runs git with the given args and returns the stdout. -# Raises an error if git does not exit successfully. -def run_git(*args): +# Raises an error if git does not exit successfully (unless passed +# allow_non_zero_exit_code=True). +def run_git(*args, allow_non_zero_exit_code=False): cmd = ['git', *args] p = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - if (p.returncode != 0): + if not allow_non_zero_exit_code and p.returncode != 0: raise Exception('Call to ' + ' '.join(cmd) + ' exited with code ' + str(p.returncode) + ' stderr:' + p.stderr.decode('ascii')) return p.stdout.decode('ascii') @@ -36,7 +37,9 @@ def branch_exists_on_remote(branch_name): return run_git('ls-remote', '--heads', ORIGIN, branch_name).strip() != '' # Opens a PR from the given branch to the target branch -def open_pr(repo, all_commits, source_branch_short_sha, new_branch_name, source_branch, target_branch, conductor, is_v2_release, labels): +def open_pr( + repo, all_commits, source_branch_short_sha, new_branch_name, source_branch, target_branch, + conductor, is_v2_release, labels, conflicted_files): # Sort the commits into the pull requests that introduced them, # and any commits that don't have a pull request pull_requests = [] @@ -81,6 +84,10 @@ def open_pr(repo, all_commits, source_branch_short_sha, new_branch_name, source_ body.append('') body.append('Please review the following:') + if len(conflicted_files) > 0: + body.append(' - [ ] You have amended the merge commit appearing in this branch to resolve ' + + 'the merge conflicts in the following files:') + body.extend([f' - [ ] `{file}`' for file in conflicted_files]) body.append(' - [ ] The CHANGELOG displays the correct version and date.') body.append(' - [ ] The CHANGELOG includes all relevant, user-facing changes since the last release.') body.append(' - [ ] There are no unexpected commits being merged into the ' + target_branch + ' branch.') @@ -246,6 +253,11 @@ def main(): # Create the new branch and push it to the remote print('Creating branch ' + new_branch_name) + # The process of creating the v1 release can run into merge conflicts. We commit the unresolved + # conflicts so a maintainer can easily resolve them (vs erroring and requiring maintainers to + # reconstruct the release manually) + conflicted_files = [] + if args.mode == V1_MODE: # If we're performing a backport, start from the v1 branch print(f'Creating {new_branch_name} from the {ORIGIN}/v1 branch') @@ -274,7 +286,12 @@ def main(): print(' Nothing to revert.') print(f'Merging {ORIGIN}/{source_branch} into the release prep branch') - run_git('merge', f'{ORIGIN}/{source_branch}', '--no-edit') + # Commit any conflicts (see the comment for `conflicted_files`) + run_git('merge', f'{ORIGIN}/{source_branch}', allow_non_zero_exit_code=True) + conflicted_files = run_git('diff', '--name-only', '--diff-filter', 'U').splitlines() + if len(conflicted_files) > 0: + run_git('add', '.') + run_git('commit', '--no-edit') # Migrate the package version number from a v2 version number to a v1 version number print(f'Setting version number to {version}') @@ -317,6 +334,7 @@ def main(): conductor=args.conductor, is_v2_release=args.mode == V2_MODE, labels=['Update dependencies'] if args.mode == V1_MODE else [], + conflicted_files=conflicted_files ) if __name__ == '__main__': From 5b5ed44ab7e2d467ee4075abb5ed4653c9bb4ca8 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Thu, 14 Apr 2022 19:38:04 +0100 Subject: [PATCH 2/3] Add a PR check to check for conflict markers This check is primarily intended to validate that any merge conflicts in the v2 -> v1 backport PR are fixed before the PR is merged. --- .github/workflows/check-for-conflicts.yml | 31 +++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/check-for-conflicts.yml diff --git a/.github/workflows/check-for-conflicts.yml b/.github/workflows/check-for-conflicts.yml new file mode 100644 index 0000000000..fdd04120de --- /dev/null +++ b/.github/workflows/check-for-conflicts.yml @@ -0,0 +1,31 @@ +# Checks for any conflict markers created by git. This check is primarily intended to validate that +# any merge conflicts in the v2 -> v1 backport PR are fixed before the PR is merged. +name: Check for conflicts + +on: + pull_request: + branches: [main, v1, v2] + # Run checks on reopened draft PRs to support triggering PR checks on draft PRs that were opened + # by other workflows. + types: [opened, synchronize, reopened, ready_for_review] + +jobs: + check-for-conflicts: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Check for conflicts + run: | + # Use `|| true` since grep returns exit code 1 if there are no matches, and we don't want + # this to fail the workflow. + FILES_WITH_CONFLICTS=$(grep --extended-regexp --ignore-case --line-number --recursive \ + '^(<<<<<<<|>>>>>>>)' . || true) + if [[ "${FILES_WITH_CONFLICTS}" ]]; then + echo "Fail: Found merge conflict markers in the following files:" + echo "" + echo "${FILES_WITH_CONFLICTS}" + exit 1 + else + echo "Success: Found no merge conflict markers." + fi From 074853a9a2289268af42d9c0d463185f015e03f0 Mon Sep 17 00:00:00 2001 From: Henry Mercer Date: Mon, 25 Apr 2022 16:37:32 +0100 Subject: [PATCH 3/3] Suggest resolving conflicts by adding new commits vs amending the merge commit This gives us slightly messier git history, but more importantly makes reviewing substantially easier. --- .github/update-release-branch.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/update-release-branch.py b/.github/update-release-branch.py index f247695c4a..106053dc35 100644 --- a/.github/update-release-branch.py +++ b/.github/update-release-branch.py @@ -85,9 +85,11 @@ def open_pr( body.append('') body.append('Please review the following:') if len(conflicted_files) > 0: - body.append(' - [ ] You have amended the merge commit appearing in this branch to resolve ' + - 'the merge conflicts in the following files:') + body.append(' - [ ] You have added commits to this branch that resolve the merge conflicts ' + + 'in the following files:') body.extend([f' - [ ] `{file}`' for file in conflicted_files]) + body.append(' - [ ] Another maintainer has reviewed the additional commits you added to this ' + + 'branch to resolve the merge conflicts.') body.append(' - [ ] The CHANGELOG displays the correct version and date.') body.append(' - [ ] The CHANGELOG includes all relevant, user-facing changes since the last release.') body.append(' - [ ] There are no unexpected commits being merged into the ' + target_branch + ' branch.')