[review bot] Add comment to the PR instead of printing something to c…

…onsole that nobody reads
microsoft · Oct 17, 2023 · a188007 · a188007
1 parent 5a62a63
commit a188007
Show file tree

Hide file tree

Showing 2 changed files with 167 additions and 34 deletions.
diff --git a/.github/workflows/trustedPR.yml b/.github/workflows/trustedPR.yml
@@ -0,0 +1,118 @@
+# Modelled after https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
+
+name: Post Check For Common Mistakes
+
+on:
+  workflow_run:
+    workflows: ["Check For Common Mistakes"]
+    types:
+      - completed
+
+permissions:
+  contents: read
+
+jobs:
+  comment:
+    permissions:
+      pull-requests: write
+    runs-on: ubuntu-22.04
+    if: >
+      ${{ github.event.workflow_run.event == 'pull_request' }}
+
+    steps:
+      - name: 'Download artifact'
+        uses: actions/github-script@v6
+        with:
+          script: |
+            var artifacts = await github.rest.actions.listWorkflowRunArtifacts({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               run_id: ${{github.event.workflow_run.id }},
+            });
+            console.log(artifacts.data)
+            var matchArtifact = artifacts.data.artifacts.filter((artifact) => {
+              return artifact.name == "pr"
+            })[0];
+            console.log(matchArtifact)
+            var download = await github.rest.actions.downloadArtifact({
+               owner: context.repo.owner,
+               repo: context.repo.repo,
+               artifact_id: matchArtifact.id,
+               archive_format: 'zip',
+            });
+            var fs = require('fs');
+            fs.writeFileSync('${{github.workspace}}/pr.zip', Buffer.from(download.data));
+      - run: unzip pr.zip
+
+      - uses: actions/github-script@v6
+        with:
+          script: |
+            const { promises: fs } = require('fs')
+            const body = (await fs.readFile('body', 'utf8')).trim()
+            const issue_number = Number(await fs.readFile('./NR'));
+
+            if (body.length > 0) {
+              try {
+                let page = 1;
+                let comment_updated = false;
+
+                while (!comment_updated) {
+                  const { data } = await github.rest.issues.listComments({
+                    ...context.repo,
+                    issue_number,
+                    page: page,
+                    per_page: 100, // Maximum comments per page
+                  });
+
+                  if (data.length === 0) {
+                    break;
+                  }
+                  for (comment of data) {
+                    if (comment.user.login === github.context.actor) {
+                      github.rest.issues.updateComment({
+                        ...context.repo,
+                        comment_id: comment.id,
+                        body: body,
+                      });
+                      comment_updated = true;
+                      break;
+                    }
+                  }
+                  page++;
+                }
+                if (!comment_updated) {
+                  github.rest.issues.createComment({
+                    ...context.repo,
+                    issue_number,
+                    body: body,
+                  });
+                }
+              } catch (error) {
+                core.setFailed(error.message);
+              }
+            } else {
+              // Delete old comments
+              let page = 1;
+              while (true) {
+                const { data } = await github.rest.issues.listComments({
+                  ...context.repo,
+                  issue_number,
+                  page: page,
+                  per_page: 100, // Maximum comments per page
+                });
+
+                if (data.length === 0) {
+                  break;
+                }
+                for (comment of data) {
+                  if (comment.user.login === github.context.actor) {
+                    github.rest.issues.deleteComment({
+                      ...context.repo,
+                      comment_id: comment.id,
+                    });
+                  }
+                }
+                page++;
+              }
+            }
+
diff --git a/.github/workflows/untrustedPR.yml b/.github/workflows/untrustedPR.yml
@@ -17,6 +17,11 @@ jobs:
       - name: Bootstrap
         run: ./bootstrap-vcpkg.sh
 
+      - name: Save PR number
+        run: |
+          mkdir -p ./pr
+          echo ${{ github.event.number }} > ./pr/NR
+
       - name: Formatting
         run: |
           git config user.email github-actions
@@ -30,8 +35,8 @@ jobs:
           if [ -s .github-pr.changed-portfiles ]; then (grep -n -H -E '(vcpkg_apply_patches|vcpkg_build_msbuild|vcpkg_extract_source_archive_ex)' $(cat .github-pr.changed-portfiles) || true) > .github-pr.deprecated-function; else touch .github-pr.deprecated-function; fi
           if [ -s .github-pr.changed-portfiles ]; then (grep -n -H -E '(vcpkg_install_cmake|vcpkg_build_cmake|vcpkg_configure_cmake|vcpkg_fixup_cmake_targets)' $(cat .github-pr.changed-portfiles) || true) > .github-pr.deprecated-cmake; else touch .github-pr.deprecated-cmake; fi
           git diff --name-status --merge-base HEAD^ HEAD --diff-filter=MAR -- '*vcpkg.json' | sed 's/[MAR]\t*//' > .github-pr.changed-manifest-files
-          cat .github-pr.changed-manifest-files | while read filename; do grep -q -E '"license": ' "$filename" || echo "$filename" || true; done > .github-pr.missing-license
-          cat .github-pr.changed-manifest-files | while read filename; do match=$(grep -oiP '"license": ".*\K(AGPL-1\.0|AGPL-3\.0|BSD-2-Clause-FreeBSD|BSD-2-Clause-NetBSD|bzip2-1\.0\.5|eCos-2\.0|GFDL-1\.1|GFDL-1\.2|GFDL-1\.3|GPL-1\.0|GPL-1\.0\+|GPL-2\.0|GPL-2\.0\+|GPL-2\.0-with-autoconf-exception|GPL-2\.0-with-bison-exception|GPL-2\.0-with-classpath-exception|GPL-2\.0-with-font-exception|GPL-2\.0-with-GCC-exception|GPL-3\.0|GPL-3\.0\+|GPL-3\.0-with-autoconf-exception|GPL-3\.0-with-GCC-exception|LGPL-2\.0|LGPL-2\.0\+|LGPL-2\.1|LGPL-2\.1\+|LGPL-3\.0|LGPL-3\.0\+|Nunit|StandardML-NJ|wxWindows)(?=[ "])' "$filename" || true); if [ ! -z "$match" ]; then echo "$filename (has deprecated license \"$match\")" ; fi ; done > .github-pr.deprecated-license
+          cat .github-pr.changed-manifest-files | while read filename; do grep -q -E '"license": ' "$filename" || echo "- \`$filename\`" || true; done > .github-pr.missing-license
+          cat .github-pr.changed-manifest-files | while read filename; do match=$(grep -oiP '"license": ".*\K(AGPL-1\.0|AGPL-3\.0|BSD-2-Clause-FreeBSD|BSD-2-Clause-NetBSD|bzip2-1\.0\.5|eCos-2\.0|GFDL-1\.1|GFDL-1\.2|GFDL-1\.3|GPL-1\.0|GPL-1\.0\+|GPL-2\.0|GPL-2\.0\+|GPL-2\.0-with-autoconf-exception|GPL-2\.0-with-bison-exception|GPL-2\.0-with-classpath-exception|GPL-2\.0-with-font-exception|GPL-2\.0-with-GCC-exception|GPL-3\.0|GPL-3\.0\+|GPL-3\.0-with-autoconf-exception|GPL-3\.0-with-GCC-exception|LGPL-2\.0|LGPL-2\.0\+|LGPL-2\.1|LGPL-2\.1\+|LGPL-3\.0|LGPL-3\.0\+|Nunit|StandardML-NJ|wxWindows)(?=[ "])' "$filename" || true); if [ ! -z "$match" ]; then echo "- \`$filename\` (has deprecated license \`$match\`)" ; fi ; done > .github-pr.deprecated-license
           ./vcpkg format-manifest --all --convert-control
           git diff > .github-pr.format-manifest
           git add -u
@@ -60,46 +65,49 @@ jobs:
             const deprecated_license = (await fs.readFile('.github-pr.deprecated-license', 'utf8')).trim()
 
             let approve = true;
+            let full_output = ""
             if (format !== "") {
               var format_output = '';
               format_output += "All vcpkg.json files must be formatted. To fix this problem, run:\n";
-              format_output += "./vcpkg format-manifest ports/*/vcpkg.json\n";
+              format_output += "`./vcpkg format-manifest --all`\n";
               format_output += "\n";
-              format_output += "It should make the following changes:";
-              format_output += "```diff\n" + format + "\n```";
-              core.error(format_output);
+              format_output += "<details><summary>It should make the following changes:</summary>";
+              format_output += "```diff\n" + format + "\n```\n</details>\n\n";
+              full_output += format_output;
               approve = false;
             }
             if (add_version_out !== "") {
               var add_version_output = '';
               add_version_output += "PRs must add only one version, and must not modify any published versions.\n";
-              add_version_output += "When making any changes to a library, the version or port-version in vcpkg.json must be modified, and the version database updated.\n";
+              add_version_output += "When making any changes to a library, the `version` or `port-version` in `vcpkg.json` must be modified, and the version database updated.\n";
               add_version_output += "Making the following changes will fix this problem:";
-              add_version_output += "```diff\n" + add_version_out + "\n```";
-              core.error(add_version_output);
+              add_version_output += "```diff\n" + add_version_out + "\n```\n\n";
+              full_output += add_version_output
               approve = false;
             }
             if (version_string_out !== "") {
-              core.warning(version_string_out);
+              full_output += version_string_out + "\n\n";
             }
             if (add_version !== "") {
               var update_version_db_output = '';
               update_version_db_output += "After committing all other changes, the version database must be updated.\n";
               update_version_db_output += "This can be done by running the following commands after committing your changes:\n"
-              update_version_db_output += "\n"
-              update_version_db_output += "git add -u && git commit\n"
-              update_version_db_output += "git checkout ${{ github.event.pull_request.base.sha }} -- versions\n"
-              update_version_db_output += "./vcpkg x-add-version --all"
-              core.error(update_version_db_output);
+              update_version_db_output += "1. `git add -u && git commit`\n"
+              update_version_db_output += "2. `git checkout ${{ github.event.pull_request.base.sha }} -- versions`\n"
+              update_version_db_output += "3. `./vcpkg x-add-version --all`\n"
+              update_version_db_output += "\n\n"
+              full_output = update_version_db_output
               approve = false;
             }
-            
-            if (deprecated_function.length > 0) {
-              var deprecated_output = '';
-              deprecated_output += "You have modified or added at least one portfile where deprecated functions are used.\n"
-              deprecated_output += "If you feel able to do so, please consider migrating them to the new functions.\n";
-              core.warning(deprecated_output);
 
+            const has_deprecated_functions = deprecated_function.length > 0 || deprecated_cmake.length > 0;
+
+            if (has_deprecated_functions) {
+              full_output += "You have modified or added at least one portfile where deprecated functions are used.\n"
+              full_output += "If you feel able to do so, please consider migrating them to the new functions.\n";
+            }
+
+            if (deprecated_function.length > 0) {
               let deprecated_functions = {
                   vcpkg_extract_source_archive_ex: 'vcpkg_extract_source_archive https://learn.microsoft.com/en-us/vcpkg/maintainers/functions/vcpkg_extract_source_archive',
                   vcpkg_build_msbuild: 'vcpkg_install_msbuild https://learn.microsoft.com/en-us/vcpkg/maintainers/functions/vcpkg_install_msbuild',
@@ -119,15 +127,11 @@ jobs:
             }
 
             if (deprecated_cmake.length > 0) {
-              var deprecated_output = '';
-              deprecated_output += "You have modified or added at least one portfile where deprecated functions are used.\n"
-              deprecated_output += "These functions have been forbidden in vcpkg, please migrating them to the new functions.\n";
-              deprecated_output += "In the ports that use the new function vcpkg_cmake_configure, vcpkg_cmake_install, vcpkg_cmake_build or vcpkg_cmake_config_fixup, you have to add the corresponding dependencies:\n";
-              deprecated_output += "```json\n";
-              deprecated_output += '{\n  "name": "vcpkg-cmake",\n  "host": true\n},\n'
-              deprecated_output += '{\n  "name": "vcpkg-cmake-config",\n  "host": true\n}\n';
-              deprecated_output += "```\n";
-              core.error(deprecated_output);
+              full_output += "In the ports that use the new function `vcpkg_cmake_configure`, `vcpkg_cmake_install`, `vcpkg_cmake_build` or `vcpkg_cmake_config_fixup`, you have to add the corresponding dependencies:\n";
+              full_output += "```json\n";
+              full_output += '{\n  "name": "vcpkg-cmake",\n  "host": true\n},\n'
+              full_output += '{\n  "name": "vcpkg-cmake-config",\n  "host": true\n}\n';
+              full_output += "```\n";
 
               let deprecated_functions = {
                   vcpkg_install_cmake: 'vcpkg_cmake_install (from port vcpkg-cmake)',
@@ -149,11 +153,16 @@ jobs:
               }
             }
 
+            if (has_deprecated_functions) {
+              full_output += "See _Files Changed_ tab for more information.\n";
+              full_output += "\n\n";
+            }
+
             if (missing_license !== "" || deprecated_license !== "") {
               var license_output = '';
-              license_output += 'You have modified or added at least one vcpkg.json where you should check the \"license\" field.\n'
+              license_output += 'You have modified or added at least one `vcpkg.json` where you should check the `license` field.\n'
               if (missing_license !== "") {
-                license_output += 'If you feel able to do so, please consider adding a "license" field to the following files:\n'
+                license_output += 'If you feel able to do so, please consider adding a `license` field to the following files:\n'
                 license_output += missing_license
                 license_output += "\n\nValid values for the license field can be found at https://learn.microsoft.com/en-us/vcpkg/reference/vcpkg-json#license\n\n"
               }
@@ -162,10 +171,16 @@ jobs:
                 license_output += deprecated_license
                 license_output += "\n\nDeprecated and non deprecated license identifiers can be found at https://spdx.org/licenses/#deprecated\n"
               }
-
-              core.warning(license_output);
+              full_output += license_output
             }
+            console.log(full_output)
+            await fs.writeFile("pr/body", full_output)
 
             if (!approve) {
               process.exitCode = 1;
             }
+
+      - uses: actions/upload-artifact@v3
+        with:
+          name: pr
+          path: pr/