Skip to content

Conversation

@nightlark
Copy link
Contributor

@nightlark nightlark commented Nov 12, 2025

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:

  • Enable building in forks without erroring on the permissions/upload steps by only running those steps when the workflow is running in the repo within the main LLVM org
  • Add an option to save the built package as an artifact to download for local testing/inspection
  • Add a dry-run option for testing the release workflow within the main LLVM repository without actually doing the upload (also as an input for workflow_call, so in the future the release workflow that releases everything can also be set up to do a test of the release workflows without uploading anything.)

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.

@github-actions
Copy link

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 @ followed by their GitHub username.

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.

@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2025

@llvm/pr-subscribers-github-workflow

Author: Ryan Mast (nightlark)

Changes

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:

  • Enable building in forks without erroring on the permissions/upload steps by only running those steps when the workflow is running in the repo within the main LLVM org
  • Add an option to save the built package as an artifact to download for local testing/inspection
  • Add a dry-run option for testing the release workflow within the main LLVM repository without actually doing the upload (also as an input for workflow_call, so in the future the release workflow that releases everything can also be set up to do a test of the release workflows without uploading anything.)

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:

  • (modified) .github/workflows/release-lit.yml (+23-1)
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 }}

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple notes:

  1. 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.yml for an example.
  2. We should save the artifacts unconditionally.
  3. 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 level github.repository_owner = 'llvm' tag. I think enabling testing on PRs makes this less of a burden on testing.

@zwuis
Copy link
Contributor

zwuis commented Nov 12, 2025

Please do not @ someone in PR description.

https://llvm.org/docs/DeveloperPolicy.html#commit-messages

@nightlark
Copy link
Contributor Author

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?

@zwuis
Copy link
Contributor

zwuis commented Nov 13, 2025

Please do not @ someone in PR description.

https://llvm.org/docs/DeveloperPolicy.html#commit-messages

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/

@nightlark nightlark force-pushed the lit-ci-test-release-workflow branch 7 times, most recently from 45236b7 to 4c30d5b Compare November 14, 2025 19:34
@nightlark
Copy link
Contributor Author

nightlark commented Nov 14, 2025

A couple notes:

  1. 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.yml for an example.

I removed the explicit dry-run input, and used a check against contains(github.workflow_ref, '/.github/workflows/release-tasks.yml') to limit the permission check and upload steps to the release-tasks workflow triggering it (otherwise the permission check fails on PRs, and the package doesn't get built or tested). It seems currently for actual releases, this workflow will only get triggered by release-tasks.yml when a llvmorg- tag is pushed, and I'm guessing we don't want to add any new conditions that can trigger an actual release.

I also added the lit package folder to the set of paths that can trigger running the workflow, to validate that the check-list tests still pass and any packaging related changes are okay.

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 push, which conflicts with doing a "dry-run" test for commits to main. github.workflow_ref is the only real indication I found to tell if this workflow is being triggered by a workflow_call and not a push to main, hence using contains for the check. An alternative is modifying release-tasks.yml to pass in an input to use as a flag.

  1. We should save the artifacts unconditionally.

Done.

  1. 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 level github.repository_owner = 'llvm' tag. I think enabling testing on PRs makes this less of a burden on testing.

I moved the repository owner check to the top of the workflow, along with allowing workflow_dispatch as a means to still manually trigger a dry run in forks (which is needed since I don't have the permissions necessary to let the workflows run on my PR in the LLVM repository, unless people are okay being constantly nagged to approve the workflow run).

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.

@nightlark nightlark force-pushed the lit-ci-test-release-workflow branch from 4c30d5b to 398753f Compare November 14, 2025 19:43
@nightlark nightlark force-pushed the lit-ci-test-release-workflow branch from 398753f to a486907 Compare November 14, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants