Merge pull request #8 from dsp-testing/henrymercer/handle-merge-conflicts-in-releases

Henrymercer/handle merge conflicts in releases

Author: henrymercer

.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,12 @@ 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 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.')
@@ -246,6 +255,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 +288,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 +336,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__':

.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

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [UNRELEASED]
 
 - Add `working-directory` input to the `autobuild` action. [#1024](https://github.com/github/codeql-action/pull/1024)
+- The `analyze` and `upload-sarif` actions will now wait up to 2 minutes for processing to complete after they have uploaded the results so they can report any processing errors that occurred. This behavior can be disabled by setting the `wait-for-processing` action input to `"false"`. [#1007](https://github.com/github/codeql-action/pull/1007)
 
 ## 2.1.8 - 08 Apr 2022
 

analyze/action.yml

@@ -61,7 +61,7 @@ inputs:
   wait-for-processing:
     description: If true, the Action will wait for the uploaded SARIF to be processed before completing.
     required: true
-    default: "false"
+    default: "true"
   token:
     default: ${{ github.token }}
   matrix:

lib/upload-lib.js

@@ -485,23 +485,17 @@ export async function waitForProcessing(
       logger.info(`Analysis upload status is ${status}.`);
       if (status === "complete") {
         break;
+      } else if (status === "pending") {
+        logger.debug("Analysis processing is still pending...");
       } else if (status === "failed") {
         throw new Error(
           `Code Scanning could not process the submitted SARIF file:\n${response.data.errors}`
         );
       }
     } catch (e) {
-      if (util.isHTTPError(e)) {
-        switch (e.status) {
-          case 404:
-            logger.debug("Analysis is not found yet...");
-            break; // Note this breaks from the case statement, not the outer loop.
-          default:
-            throw e;
-        }
-      } else {
-        throw e;
-      }
+      logger.warning(
+        `An error occurred checking the status of the delivery. ${e} It should still be processed in the background, but errors that occur during processing may not be reported.`
+      );
     }
     await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS);
   }

lib/upload-lib.js.map

@@ -29,7 +29,7 @@ inputs:
   wait-for-processing:
     description: If true, the Action will wait for the uploaded SARIF to be processed before completing.
     required: true
-    default: "false"
+    default: "true"
 outputs:
   sarif-id:
     description: The ID of the uploaded SARIF file.