From a188007b9220a0e5f2614619bf1aa0df57ced649 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Tue, 17 Oct 2023 14:42:31 -0400 Subject: [PATCH] [review bot] Add comment to the PR instead of printing something to console that nobody reads --- .github/workflows/trustedPR.yml | 118 ++++++++++++++++++++++++++++++ .github/workflows/untrustedPR.yml | 83 ++++++++++++--------- 2 files changed, 167 insertions(+), 34 deletions(-) create mode 100644 .github/workflows/trustedPR.yml diff --git a/.github/workflows/trustedPR.yml b/.github/workflows/trustedPR.yml new file mode 100644 index 00000000000000..18600b97f70f74 --- /dev/null +++ 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 index 723a499589820c..1942148bdb9056 100644 --- 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 += "
It should make the following changes:"; + format_output += "```diff\n" + format + "\n```\n
\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/