-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Github] Remove separate tools checkout from pr-code workflows #159967
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Github] Remove separate tools checkout from pr-code workflows #159967
Conversation
These separate checkouts I believe were originally carried over from when we were using pull_request_target. We needed two checkouts to ensure we were not executing user supplied code. We kept them to ensure we were using the latest version of the tools, but this was born mostly out of a misunderstanding of how Github Actions works. All PRs directly against main are executed as if merged into main, so already are using the latest version of the tools no matter the branch point. Stacked PRs still need to be rebased for changes to propagate but these files have been pretty stable for the past two years or so and I can't imagine any changes needed to keep things running on release/stacked PR branches.
@llvm/pr-subscribers-github-workflow Author: Aiden Grossman (boomanaiden154) ChangesThese separate checkouts I believe were originally carried over from when we were using pull_request_target. We needed two checkouts to ensure we were not executing user supplied code. We kept them to ensure we were using the latest version of the tools, but this was born mostly out of a misunderstanding of how Github Actions works. All PRs directly against main are executed as if merged into main, so already are using the latest version of the tools no matter the branch point. Stacked PRs still need to be rebased for changes to propagate but these files have been pretty stable for the past two years or so and I can't imagine any changes needed to keep things running on release/stacked PR branches. Full diff: https://github.com/llvm/llvm-project/pull/159967.diff 2 Files Affected:
diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 9396bf019e1ac..61c8680cd72a1 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -32,19 +32,6 @@ jobs:
base_sha: 'HEAD~1'
sha: 'HEAD'
- # We need to pull the script from the main branch, so that we ensure
- # we get the latest version of this script.
- - name: Fetch code formatting utils
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
- with:
- repository: ${{ github.repository }}
- ref: ${{ github.base_ref }}
- sparse-checkout: |
- llvm/utils/git/requirements_formatting.txt
- llvm/utils/git/code-format-helper.py
- sparse-checkout-cone-mode: false
- path: code-format-tools
-
- name: "Listed files"
env:
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
@@ -65,10 +52,10 @@ jobs:
with:
python-version: '3.11'
cache: 'pip'
- cache-dependency-path: 'code-format-tools/llvm/utils/git/requirements_formatting.txt'
+ cache-dependency-path: 'llvm/utils/git/requirements_formatting.txt'
- name: Install python dependencies
- run: pip install -r code-format-tools/llvm/utils/git/requirements_formatting.txt
+ run: pip install -r llvm/utils/git/requirements_formatting.txt
- name: Run code formatter
env:
@@ -77,7 +64,7 @@ jobs:
# Create an empty comments file so the pr-write job doesn't fail.
run: |
echo "[]" > comments &&
- python ./code-format-tools/llvm/utils/git/code-format-helper.py \
+ python ./llvm/utils/git/code-format-helper.py \
--write-comment-to-file \
--token ${{ secrets.GITHUB_TOKEN }} \
--issue-number $GITHUB_PR_NUMBER \
diff --git a/.github/workflows/pr-code-lint.yml b/.github/workflows/pr-code-lint.yml
index adb6c6e8f4c76..16f514ba27c17 100644
--- a/.github/workflows/pr-code-lint.yml
+++ b/.github/workflows/pr-code-lint.yml
@@ -45,18 +45,6 @@ jobs:
run: |
echo "Changed files:"
echo "$CHANGED_FILES"
-
- - name: Fetch code linting utils
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
- with:
- repository: ${{ github.repository }}
- ref: ${{ github.base_ref }}
- sparse-checkout: |
- llvm/utils/git/code-lint-helper.py
- llvm/utils/git/requirements_linting.txt
- clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py
- sparse-checkout-cone-mode: false
- path: code-lint-tools
- name: Install clang-tidy
uses: aminya/setup-cpp@17c11551771948abc5752bbf3183482567c7caf0 # v1.1.1
@@ -69,7 +57,7 @@ jobs:
python-version: '3.12'
- name: Install Python dependencies
- run: python3 -m pip install -r code-lint-tools/llvm/utils/git/requirements_linting.txt
+ run: python3 -m pip install -r llvm/utils/git/requirements_linting.txt
# TODO: create special mapping for 'codegen' targets, for now build predefined set
# TODO: add entrypoint in 'compute_projects.py' that only adds a project and its direct dependencies
@@ -106,7 +94,7 @@ jobs:
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
run: |
echo "[]" > comments &&
- python3 ./code-lint-tools/llvm/utils/git/code-lint-helper.py \
+ python3 llvm/utils/git/code-lint-helper.py \
--token ${{ secrets.GITHUB_TOKEN }} \
--issue-number $GITHUB_PR_NUMBER \
--start-rev HEAD~1 \
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Do you have test run available to see if it works properly?
https://github.com/llvm/llvm-project/actions/runs/17896422866/job/50883647621?pr=159967 |
This path was left untouched after refactoring made in #159967 which broke clang-tidy runner.
This path was left untouched after refactoring made in llvm/llvm-project#159967 which broke clang-tidy runner.
* main: (1562 commits) Document Policy on supporting newer C++ standard in LLVM codebase (llvm#156823) [MLIR][Transform][SMT] Introduce transform.smt.constrain_params (llvm#159450) Reapply "[compiler-rt] Remove %T from shared object substitutions (llvm#155302)" [NFC] [IndVarSimplify] Add non-overflowing usub test (llvm#159683) [Github] Remove separate tools checkout from pr-code workflows (llvm#159967) [clang] fix using enum redecl in template regression (llvm#159996) [DAG] Skip `mstore` combine for `<1 x ty>` vectors (llvm#159915) [mlir] Expose optional `PatternBenefit` to `func` populate functions (NFC) (llvm#159986) [LV] Set correct costs for interleave group members. [clang] ast-dump: use template pattern for `instantiated_from` (llvm#159952) [ARM] ha-alignstack-call.ll - regenerate test checks (llvm#159988) [LLD][MachO] Silence warning when building with MSVC [llvm][Analysis] Silence warning when building with MSVC [LV] Skip select cost for invariant divisors in legacy cost model. [Clang] Fix an error-recovery crash after d1a80de (llvm#159976) [VPlanPatternMatch] Introduce m_ConstantInt (llvm#159558) [GlobalISel] Add G_ABS computeKnownBits (llvm#154413) [gn build] Port 4cabd1e Reland "[clangd] Add feature modules registry" (llvm#154836) [LV] Also handle non-uniform scalarized loads when processing AddrDefs. ...
These separate checkouts I believe were originally carried over from when we were using pull_request_target. We needed two checkouts to ensure we were not executing user supplied code. We kept them to ensure we were using the latest version of the tools, but this was born mostly out of a misunderstanding of how Github Actions works. All PRs directly against main are executed as if merged into main, so already are using the latest version of the tools no matter the branch point. Stacked PRs still need to be rebased for changes to propagate but these files have been pretty stable for the past two years or so and I can't imagine any changes needed to keep things running on release/stacked PR branches.