GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-138497: Support LLVM_VERSION configuration via env by danigm · Pull Request #138498 · python/cpython

Conversation

@danigm

@danigm danigm commented Sep 4, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This patch makes possible to override the llvm version used during build using "LLVM_VERSION" and "EXTERNALS_LLVM_TAG" environment variables.

This is useful to test the build with other LLVM versions without changing the sources, for example to build with llvm-20.

Related issue: #136895

  • Issue: Make LLVM_VERSION configurable with env variable #138497

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@danigm
danigm requested review from brandtbucher, diegorusso and savannahostrowski as code owners September 4, 2025 10:23
@bedevere-app bedevere-app Bot added the awaiting review label Sep 4, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Sep 4, 2025
Make LLVM_VERSION configurable with env variable #138497
Closed
@brandtbucher

brandtbucher commented Sep 8, 2025

Copy link
Copy Markdown
Member

If we do this:

  • We should do it in a manner similar to CFLAGS_JIT, where the env var is captured by the configure script and passed via the command-line to the build script.
  • EXTERNALS_LLVM_TAG should be left alone, I don't think it makes sense for anyone to override that (it's only used for Windows builds and it's literally a tag in a repo we control).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@danigm
danigm requested review from AA-Turner, corona10, emmatyping and erlend-aasland as code owners September 9, 2025 07:13
@danigm
danigm force-pushed the llvm-version branch from e3a6760 to 8560996 Compare September 9, 2025 07:21
@danigm

danigm commented Sep 9, 2025

Copy link
Copy Markdown
Contributor Author

If we do this:

* We should do it in a manner similar to `CFLAGS_JIT`, where the env var is captured by the `configure` script and passed via the command-line to the build script.

* `EXTERNALS_LLVM_TAG` should be left alone, I don't think it makes sense for anyone to override that (it's only used for Windows builds and it's literally a tag in a repo we control).

I've done that way. Get the env variable in configure and pass to Tools/jit/build.py script as a new argument --llvm-version.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@danigm
danigm force-pushed the llvm-version branch 2 times, most recently from b5b9649 to 4189a3e Compare September 9, 2025 08:03
@brandtbucher

brandtbucher commented Sep 10, 2025

Copy link
Copy Markdown
Member

@savannahostrowski, thoughts on this change?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@emmatyping
emmatyping removed their request for review September 11, 2025 23:57
savannahostrowski
savannahostrowski requested changes Sep 15, 2025
View reviewed changes

@savannahostrowski savannahostrowski left a comment

Copy link
Copy Markdown
Member

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.

Thanks for this! I think this change is fair, so long as we only advertise that a single LLVM version is officially supported.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Build/2025-09-04-12-16-31.gh-issue-138497.Y_5YXh.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting changes and removed awaiting review labels Sep 15, 2025
@bedevere-app

bedevere-app Bot commented Sep 15, 2025

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
emmatyping
emmatyping reviewed Sep 16, 2025
View reviewed changes
Comment thread Tools/jit/_llvm.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@danigm

danigm commented Sep 16, 2025

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Sep 16, 2025
@bedevere-app

bedevere-app Bot commented Sep 16, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@savannahostrowski: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app
bedevere-app Bot requested a review from savannahostrowski September 16, 2025 05:45
@danigm

danigm commented Sep 17, 2025

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Sep 17, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@savannahostrowski: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
savannahostrowski
savannahostrowski reviewed Sep 17, 2025
View reviewed changes

@savannahostrowski savannahostrowski left a comment

Copy link
Copy Markdown
Member

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.

Thanks for making the updates - I have a couple of other small wording suggestions but I think otherwise this looks good.

Would you also mind updating the Tools/jit/README.md to reflect this change? Right now, the README says that LLVM 19 is required. With this change, we should call out that LLVM 19 is the officially supported version, but that you can modify it if need be, using the env var.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/Build/2025-09-04-12-16-31.gh-issue-138497.Y_5YXh.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/jit/build.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/jit/_targets.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@danigm
danigm force-pushed the llvm-version branch from eb2a37a to 18f96e9 Compare September 17, 2025 11:08
danigm and others added 8 commits October 7, 2025 08:16
@danigm
Reword news line with more information about LLVM
42d1d12
@danigm
Set _LLVM_VERSION as default value for Target.llvm_version
4ce99f3
@danigm
Do not set target.llvm_version on empty string
2a82537
@danigm
Add warning when building with custom LLVM_VERSION
6528dae
@danigm @savannahostrowski
Update Misc/NEWS.d/next/Build/2025-09-04-12-16-31.gh-issue-138497.Y_5…
c45361d 
…YXh.rst

Rewording news entry

Co-authored-by: Savannah Bailey <[email protected]>
@danigm
Move LLVM version warning to experimental banner
10175c7
@danigm
Add LLVM_VERSION env info to Tools/jit/README.md
a60b02d
@danigm @savannahostrowski
Fix warning synxtax in Tools/jit/_targets.py
8cea949 
Co-authored-by: Savannah Ostrowski <[email protected]>
@danigm
danigm force-pushed the llvm-version branch from de0d94d to 8cea949 Compare October 7, 2025 06:16
@danigm

danigm commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

@savannahostrowski conflict solved, I've rebased on top of main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Oct 7, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@danigm Please avoid rebasing and force pushing, it can result in lots of unrelated codeowners being summoned for a review.

It also makes it harder to review, and everything is squash merged anyway.
savannahostrowski reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk
hugovk removed request for AA-Turner, AlexWaygood, FFY00, JelleZijlstra, abalkin, corona10, ericvsmith, erlend-aasland, gpshead, jaraco, lysnikolaou, pganssle, picnixz, rhettinger, vsajip and warsaw October 7, 2025 06:38
@savannahostrowski savannahostrowski mentioned this pull request Oct 26, 2025
gh-138451: Support custom LLVM installation path #138452
Merged
@zanieb zanieb mentioned this pull request Jan 19, 2026
Upstream patch patch-jit-llvm-version-3.15.patch astral-sh/python-build-standalone#881
Closed
@savannahostrowski savannahostrowski mentioned this pull request Mar 11, 2026
Allow Clang-19+ for JIT (currently hard requires clang-19) #145800
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@savannahostrowski savannahostrowski savannahostrowski approved these changes
@emmatyping emmatyping emmatyping left review comments
@diegorusso diegorusso diegorusso approved these changes
@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@danigm @brandtbucher @savannahostrowski @hugovk @diegorusso @emmatyping
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.