From 42ef6baf82435c9e60f92974097657417ca4839a Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Thu, 11 Jan 2024 11:43:12 +0100 Subject: [PATCH] ci: High risk file changes as PR comment (#3554) Remove the no changes in high-risk file script and hashes, and replace it by adding a GH PR comment. --- .github/file-filters.yml | 12 +++++ .../workflows/changes-in-high-risk-code.yml | 51 +++++++++++++++++++ .github/workflows/lint.yml | 9 ---- .pre-commit-config.yaml | 8 --- Makefile | 4 -- scripts/no-changes-in-high-risk-files.sh | 25 --------- 6 files changed, 63 insertions(+), 46 deletions(-) create mode 100644 .github/file-filters.yml create mode 100644 .github/workflows/changes-in-high-risk-code.yml delete mode 100755 scripts/no-changes-in-high-risk-files.sh diff --git a/.github/file-filters.yml b/.github/file-filters.yml new file mode 100644 index 00000000000..a5407a6ab26 --- /dev/null +++ b/.github/file-filters.yml @@ -0,0 +1,12 @@ +# This is used by the action https://github.com/dorny/paths-filter (which we have forked to https://github.com/getsentry/paths-filter) + +high_risk_code: &high_risk_code + - 'Sources/Sentry/SentryNSURLSessionTaskSearch.m' + - 'Sources/Sentry/SentryNetworkTracker.m' + - 'Sources/Sentry/SentryUIViewControllerSwizzling.m' + - 'Sources/Sentry/SentryNSDataSwizzling.m' + - 'Sources/Sentry/SentrySubClassFinder.m' + - 'Sources/Sentry/SentryCoreDataSwizzling.m' + - 'Sources/Sentry/SentrySwizzleWrapper.m' + - 'Sources/Sentry/include/SentrySwizzle.h' + - 'Sources/Sentry/SentrySwizzle.m' diff --git a/.github/workflows/changes-in-high-risk-code.yml b/.github/workflows/changes-in-high-risk-code.yml new file mode 100644 index 00000000000..e822fe9c8d2 --- /dev/null +++ b/.github/workflows/changes-in-high-risk-code.yml @@ -0,0 +1,51 @@ +name: Changes In High Risk Code +on: + pull_request: + +# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + files-changed: + name: Detect changed files + runs-on: ubuntu-latest + # Map a step output to a job output + outputs: + high_risk_code: ${{ steps.changes.outputs.high_risk_code }} + high_risk_code_files: ${{ steps.changes.outputs.high_risk_code_files }} + steps: + - uses: actions/checkout@v4 + - name: Get changed files + id: changes + uses: getsentry/paths-filter@4512585405083f25c027a35db413c2b3b9006d50 # v2.11.1 + with: + token: ${{ github.token }} + filters: .github/file-filters.yml + + # Enable listing of files matching each filter. + # Paths to files will be available in `${FILTER_NAME}_files` output variable. + list-files: csv + + validate-high-risk-code: + if: needs.files-changed.outputs.high_risk_code == 'true' + needs: files-changed + runs-on: ubuntu-latest + steps: + - name: Comment on PR to notify of changes in high risk files + uses: actions/github-script@v7 + env: + high_risk_code: ${{ needs.files-changed.outputs.high_risk_code_files }} + with: + script: | + const highRiskFiles = process.env.high_risk_code; + const fileList = highRiskFiles.split(',').map(file => `- [ ] ${file}`).join('\n'); + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: `### 🚨 Detected changes in high risk code 🚨 \n High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:\n ${fileList}` + }) + + diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 9ec0e534916..b08460b62f0 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -10,7 +10,6 @@ on: - 'Samples/**' - '.github/workflows/lint.yml' - 'scripts/ci-select-xcode.sh' - - 'scripts/no-changes-in-high-risk-files.sh' pull_request: paths: @@ -20,7 +19,6 @@ on: - 'Samples/**' - '.github/workflows/lint.yml' - 'scripts/ci-select-xcode.sh' - - 'scripts/no-changes-in-high-risk-files.sh' - 'Sentry.xcodeproj/**' - '*.podspec' @@ -74,10 +72,3 @@ jobs: - run: pod repo update - name: Validate HybridPod Podspec run: pod lib lint ./Tests/HybridSDKTest/HybridPod.podspec --allow-warnings --verbose --platforms=ios "--include-podspecs={SentryPrivate.podspec,Sentry.podspec}" - - validate-high-risk-files: - name: No changes in high risk files - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - run: ./scripts/no-changes-in-high-risk-files.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54ae71c236f..90fe2b02560 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,11 +45,3 @@ repos: types_or: ["swift", "objective-c", "objective-c++", "c", "c++" ] args: - "lint" - - - id: no-changes-in-high-risk-files - name: No Changes in High Risk Files - entry: make - language: system - types_or: ["swift","objective-c", "objective-c++", "c", "c++"] - args: - - "no-changes-in-high-risk-files" diff --git a/Makefile b/Makefile index d004282f681..fa81df596b3 100644 --- a/Makefile +++ b/Makefile @@ -18,10 +18,6 @@ lint: swiftlint --strict .PHONY: lint -no-changes-in-high-risk-files: - @echo "--> Checking if there are changes in high risk files" - ./scripts/no-changes-in-high-risk-files.sh - format: format-clang format-swift # Format ObjC, ObjC++, C, and C++ diff --git a/scripts/no-changes-in-high-risk-files.sh b/scripts/no-changes-in-high-risk-files.sh deleted file mode 100755 index 376122b987d..00000000000 --- a/scripts/no-changes-in-high-risk-files.sh +++ /dev/null @@ -1,25 +0,0 @@ -#!/bin/bash -set -euo pipefail - -# To update the sha run the command in ACTUAL and copy the result in EXPECTED. - -ACTUAL=$(shasum -a 256 ./Sources/Sentry/SentryNSURLSessionTaskSearch.m ./Sources/Sentry/SentryNetworkTracker.m ./Sources/Sentry/SentryUIViewControllerSwizzling.m ./Sources/Sentry/SentryNSDataSwizzling.m ./Sources/Sentry/SentrySubClassFinder.m ./Sources/Sentry/SentryCoreDataSwizzling.m ./Sources/Sentry/SentrySwizzleWrapper.m ./Sources/Sentry/include/SentrySwizzle.h ./Sources/Sentry/SentrySwizzle.m) -EXPECTED="819d5ca5e3db2ac23c859b14c149b7f0754d3ae88bea1dba92c18f49a81da0e1 ./Sources/Sentry/SentryNSURLSessionTaskSearch.m -bee886f6eaa3ec0ba999fe0dbe38537a47cb328a3432b908231326a65a9fb2d3 ./Sources/Sentry/SentryNetworkTracker.m -40f476800b32cf885dba9ac3de75d93b6f536e819fe8e51d071b4610c879b416 ./Sources/Sentry/SentryUIViewControllerSwizzling.m -e95e62ec7363984f20c78643bb7d992a41a740f97e1befb71525ac34caf88b37 ./Sources/Sentry/SentryNSDataSwizzling.m -cc3849725bd1733515c71742872bed94ca47d2c115ef9d8c98383eae2e171925 ./Sources/Sentry/SentrySubClassFinder.m -59db11da66e6ac0058526be0be08b57cdccd3727033e85164a631b205e972134 ./Sources/Sentry/SentryCoreDataSwizzling.m -b8da44f7b4d9d0e0ed9d426d7c44d73b6f5de553f48d5c4aa6567ac56c994a27 ./Sources/Sentry/SentrySwizzleWrapper.m -b1c642450170358cab39b4cc6cd546f27c41b12eacb90c3ad93f87733d46e56c ./Sources/Sentry/include/SentrySwizzle.h -f97128c823f92d1c2ec37e5e3b2914f7488a94043af6a8344e348f1a14425f47 ./Sources/Sentry/SentrySwizzle.m" - -if [ "$ACTUAL" = "$EXPECTED" ]; then - echo "No changes in high risk files." - exit 0 -else - echo "Changes in high risk files. You might want to test your changes by using 'make test-alamofire' and 'make test-homekit' to ensure the changes are safe to run on third party projects." - echo "If your changes are intended and everything is running properly, please update the sha in ./scripts/no-changes-in-high-risk-files.sh to: " - echo "$ACTUAL" - exit 1 -fi