Browse Source

ci: run pr checks on a merge commit instead of the PR head, add rebase-and-check label support (#1244)

* ci: use merge commit for checks, also warn about merge conflict

* ci: add rebase-and-check label support

* get merge_commit_sha only once

* ci: use only one checkout step
nikita kozlovsky 1 year ago
parent
commit
eb06b6627d

+ 6 - 7
.github/workflows/build_and_test_ya.yml

@@ -49,6 +49,9 @@ on:
       put_build_results_to_cache:
         type: boolean
         default: true
+      commit_sha:
+        type: string
+        default: ""
 defaults:
   run:
     shell: bash
@@ -57,15 +60,11 @@ jobs:
     name: Build and test ${{ inputs.build_preset }}
     runs-on: [ self-hosted, "${{ inputs.runner_label }}", "${{ inputs.runner_additional_label || inputs.runner_label }}"]
     steps:
-    - name: Checkout PR
-      uses: actions/checkout@v3
-      if: github.event.pull_request.head.sha != ''
-      with:
-        ref: ${{ github.event.pull_request.head.sha }}
     - name: Checkout
       uses: actions/checkout@v3
-      if: github.event.pull_request.head.sha == ''
-    
+      with:
+        ref: ${{ inputs.commit_sha }}
+
     - name: comment-build-start
       if: github.event_name == 'pull_request' || github.event_name == 'pull_request_target'
       shell: bash

+ 4 - 0
.github/workflows/build_and_test_ya_provisioned.yml

@@ -90,6 +90,9 @@ on:
       put_build_results_to_cache:
         type: boolean
         default: true
+      commit_sha:
+        type: string
+        default: ""
 jobs:
   main:
     uses: ./.github/workflows/build_and_test_ya.yml
@@ -107,4 +110,5 @@ jobs:
       link_threads: ${{ inputs.link_threads }}
       test_threads: ${{ inputs.test_threads }}
       put_build_results_to_cache: ${{ inputs.put_build_results_to_cache }}
+      commit_sha: ${{ inputs.commit_sha }}
     secrets: inherit

+ 99 - 23
.github/workflows/pr_check.yml

@@ -1,5 +1,5 @@
 name: PR-check
-on: 
+on:
   pull_request_target:
     branches:
       - 'main'
@@ -20,7 +20,8 @@ jobs:
     if: ${{vars.CHECKS_SWITCH != '' && fromJSON(vars.CHECKS_SWITCH).pr_check == true}}
     runs-on: ubuntu-latest
     outputs:
-      result: ${{ steps.check-ownership-membership.outputs.result }}
+      result: ${{ steps.check-ownership-membership.outputs.result == 'true' && steps.check-is-mergeable.outputs.result == 'true' }}
+      commit_sha: ${{ steps.check-is-mergeable.outputs.commit_sha }}
     steps:
       - name: Check if running tests is allowed
         id: check-ownership-membership
@@ -32,17 +33,17 @@ jobs:
             const okToTestLabel = labels.find(
               label => label.name == 'ok-to-test'
             );
-
+            
             console.log("okToTestLabel=%o", okToTestLabel !== undefined);
-
+            
             if (okToTestLabel !== undefined) {
               return true;
             }
-
+            
             // This is used primarily in forks. Repository owner
             // should be allowed to run anything.
             const userLogin = context.payload.pull_request.user.login;
-
+            
             // How to interpret membership status code:
             // https://docs.github.com/rest/collaborators/collaborators#check-if-a-user-is-a-repository-collaborator
             const isRepoCollaborator = async function () {
@@ -60,12 +61,12 @@ jobs:
                 throw error;
               }
             }
-
+            
             if (context.payload.repository.owner.login == userLogin) {
               console.log("You are the repository owner!");
               return true;
             }
-
+            
             if (await isRepoCollaborator()) {
               console.log("You are a collaborator!");
               return true;
@@ -88,26 +89,100 @@ jobs:
         uses: actions/github-script@v6
         with:
           script: |
-            const { owner, repo } = context.repo;
+            let labelsToRemove = ['ok-to-test', 'rebase-and-check'];
             const prNumber = context.payload.pull_request.number;
-            const labelToRemove = 'ok-to-test';
-            try {
-              const result = await github.rest.issues.removeLabel({
-                owner,
-                repo,
-                issue_number: prNumber,
-                name: labelToRemove
-              });
-            } catch(e) {
-              // ignore the 404 error that arises
-              // when the label did not exist for the
-              // organization member
-              console.log(e);
+            const prLabels = new Set(context.payload.pull_request.labels.map(l => l.name));
+            for await (const label of labelsToRemove.filter(l => prLabels.has(l))) {
+              core.info(`remove label=${label} for pr=${prNumber}`);
+              try {
+                const result = await github.rest.issues.removeLabel({
+                  ...context.repo,
+                  issue_number: prNumber,
+                  name: label
+                });
+              } catch(error) {
+                // ignore the 404 error that arises
+                // when the label did not exist for the
+                // organization member
+                if (error.status && error.status != 404) {
+                  throw error;
+                }
+              }
+            }
+      - name: check is mergeable
+        id: check-is-mergeable
+        if: steps.check-ownership-membership.outputs.result == 'true'
+        uses: actions/github-script@v6
+        with:
+          result-encoding: string
+          script: |
+            let pr = context.payload.pull_request;
+            const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
+            const header = `<!-- merge pr=${pr.number} -->\n`;
+            
+            const fail_msg = header + ':red_circle: Unable to merge your PR into the `main` branch. '
+                + 'Please rebase or merge it with the `main` branch.'
+            
+            let i = 0;
+            
+            while (pr.mergeable == null || i >= 60) {
+                console.log("get pull-request status");
+    
+                let result = await github.rest.pulls.get({
+                    ...context.repo,
+                    pull_number: pr.number
+                })
+            
+                pr = result.data;
+              
+                if (pr.mergeable == null) {
+                  await delay(5000);
+                }
+            
+                i += 1;
+            }
+            
+            console.log("pr.mergeable=%o", pr.mergeable);
+            
+            const { data: comments } = await github.rest.issues.listComments({
+                issue_number: context.issue.number,
+                owner: context.repo.owner,
+                repo: context.repo.repo
+            });
+            
+            const commentToUpdate = comments.find(comment => comment.body.startsWith(header));
+            
+            if (pr.mergeable === false) {
+                let commentParams = {
+                    ...context.repo,
+                    issue_number: context.issue.number,
+                    body: fail_msg
+                };
+            
+                if (commentToUpdate) {
+                    await github.rest.issues.updateComment({
+                        ...commentParams,
+                        comment_id: commentToUpdate.id,
+                    });
+                } else {
+                    await github.rest.issues.createComment({...commentParams});
+                }
+                core.setFailed("Merge conflict detected");
+                return false;
+            } else if (commentToUpdate) {
+                await github.rest.issues.deleteComment({
+                    ...context.repo,
+                    issue_number: context.issue.number,
+                    comment_id: commentToUpdate.id,
+                });
             }
+            core.info(`commit_sha=${pr.commit_sha}`);
+            core.setOutput('commit_sha', pr.merge_commit_sha);
+            return true;
   build_and_test:
     needs:
       - check-running-allowed
-    if: needs.check-running-allowed.outputs.result == 'true'
+    if: needs.check-running-allowed.outputs.result == 'true' && needs.check-running-allowed.outputs.commit_sha != ''
     strategy:
       fail-fast: false
       matrix:
@@ -122,4 +197,5 @@ jobs:
       test_threads: 52
       runner_label: auto-provisioned
       put_build_results_to_cache: true
+      commit_sha: ${{ needs.check-running-allowed.outputs.commit_sha }}
     secrets: inherit