-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lit][ci] Enable testing Release Lit workflow in forks #167643
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
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-github-workflow Author: Ryan Mast (nightlark) ChangesI'm looking at Python packaging of some parts of LLVM and updating them to use current best practices (#125220 (comment)), and couldn't test the CI release workflow in my fork. This updates the CI workflow to:
These things could be split out into separate PRs if desired. The first two are probably the key ones needed to facilitate testing in forks. cc: @ddunbar @mstorsjo @jeremyd2019 @boomanaiden154 @seldridge @tru Full diff: https://github.com/llvm/llvm-project/pull/167643.diff 1 Files Affected:
diff --git a/.github/workflows/release-lit.yml b/.github/workflows/release-lit.yml
index 8b1ce04e12c4f..9e729335fd7c0 100644
--- a/.github/workflows/release-lit.yml
+++ b/.github/workflows/release-lit.yml
@@ -10,6 +10,14 @@ on:
description: 'Release Version'
required: true
type: string
+ dry-run:
+ description: 'Dry run of lit package release, without uploading to PyPI'
+ required: false
+ type: boolean
+ save-artifact:
+ description: 'Save lit package as an artifact'
+ required: false
+ type: boolean
workflow_call:
inputs:
@@ -17,6 +25,10 @@ on:
description: 'Release Version'
required: true
type: string
+ dry-run:
+ description: 'Dry run of lit package release, without uploading to PyPI'
+ required: false
+ type: boolean
secrets:
RELEASE_TASKS_USER_TOKEN:
description: "Secret used to check user permissions."
@@ -38,6 +50,7 @@ jobs:
sudo apt-get install -y python3-setuptools python3-psutil python3-github
- name: Check Permissions
+ if: github.repository_owner == 'llvm'
env:
GITHUB_TOKEN: ${{ github.token }}
USER_TOKEN: ${{ secrets.RELEASE_TASKS_USER_TOKEN }}
@@ -47,7 +60,7 @@ jobs:
- name: Setup Cpp
uses: aminya/setup-cpp@a276e6e3d1db9160db5edc458e99a30d3b109949 # v1.7.1
with:
- compiler: llvm-16.0.6
+ compiler: llvm-19.1.7
cmake: true
ninja: true
@@ -65,7 +78,15 @@ jobs:
sed -i 's/ + "dev"//g' lit/__init__.py
python3 setup.py sdist bdist_wheel
+ - name: Save lit package as an artifact
+ if: inputs['save-artifact']
+ uses: actions/upload-artifact@v4
+ with:
+ name: lit-artifact
+ path: llvm/utils/lit/dist/
+
- name: Upload lit to test.pypi.org
+ if: github.repository_owner == 'llvm' && inputs['dry-run'] != 'true'
uses: pypa/gh-action-pypi-publish@ed0c53931b1dc9bd32cbe73a98c7f6766f8a527e # v1.13.0
with:
password: ${{ secrets.LLVM_LIT_TEST_PYPI_API_TOKEN }}
@@ -73,6 +94,7 @@ jobs:
packages-dir: llvm/utils/lit/dist/
- name: Upload lit to pypi.org
+ if: github.repository_owner == 'llvm' && inputs['dry-run'] != 'true'
uses: pypa/gh-action-pypi-publish@ed0c53931b1dc9bd32cbe73a98c7f6766f8a527e # v1.13.0
with:
password: ${{ secrets.LLVM_LIT_PYPI_API_TOKEN }}
|
boomanaiden154
left a comment
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.
A couple notes:
- We should ideally make this workflow run in dry run mode when a pull request is made that touches this workflow file, and maybe on push. Then we can drop the explicit dry-run input. See
build-ci-container.ymlfor an example. - We should save the artifacts unconditionally.
- I'm not a fan of adding
github.repository_owner == 'llvm'lines per-step. According to https://llvm.org/docs/CIBestPractices.html, we should have a job levelgithub.repository_owner = 'llvm'tag. I think enabling testing on PRs makes this less of a burden on testing.
|
Please do not @ someone in PR description. |
|
Since a squashing workflow is used, should I also edit the PR description to have less commentary and just be what the final commit message body should say in it? |
Please ignore it. It seems that the requirement will be removed. https://discourse.llvm.org/t/forbidding-username-in-commits/86997/18 https://github.blog/changelog/2025-11-07-removing-notifications-for-mentions-in-commit-messages/ |
45236b7 to
4c30d5b
Compare
I removed the explicit dry-run input, and used a check against I also added the Somewhat annoying, for workflow_call triggers almost all of the information in the github context is inherited from the parent workflow -- in this case that means the event_name is
Done.
I moved the repository owner check to the top of the workflow, along with allowing The permissions check and upload steps have a check to specifically avoid any surprises if anyone were to make so release-tasks.yml could also be triggered by a workflow_dispatch. |
4c30d5b to
398753f
Compare
Signed-off-by: Ryan Mast <[email protected]>
398753f to
a486907
Compare
I'm looking at Python packaging of some parts of LLVM and updating them to use current best practices (#125220 (comment)), and couldn't test the CI release workflow in my fork.
This updates the CI workflow to:
These things could be split out into separate PRs if desired. The first two are probably the key ones needed to facilitate testing in forks.