use explicit setup-python versions over implicit expose- (Cherry-pick of #21568, #21582) #21668
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The project is currently using a mix of the GitHub supplied `setup-python` action and `pantsbuild/actions/expose-pythons`. On GitHub managed runners they behave similarly: * `setup-python` will install a version of Python if missing, or add it to the PATH if already present at the expected location (managed runners historically have multiple Pythons baked into the image) * `expose-pythons` will add all Python's at the expected GitHub location to the PATH. (Which ones is up to GitHub.) So today the invocation of `setup-python` followed by `expose-pythons` is redundant. Consolidating on `setup-python` let's us be explicit about expected versions (more like the ARM image) and reduces the number of custom actions the project manages while still making multiple Python versions available. NOTE: The awkward double newlines in the final yaml are a pre-existing issue https://stackoverflow.com/questions/45004464/yaml-dump-adding-unwanted-newlines-in-multiline-strings See #21552 for some history regarding the `setup-` vs `expose-` actions.
This fixes the "Build wheels (macOS12-x86_64)" CI job, by installing Python. It was previously failing with errors like: ``` > Run ./pants run src/python/pants_release/release.py -- build-wheels pants: Failed to find a Python 3.9 interpreter ``` I think this was a small oversight in #21568 (in particular https://github.com/pantsbuild/pants/pull/21568/files#diff-b980287839311482e79f8adac1fdd7768274cf1e677a1103d9444185e4aabbd1L890), which wasn't caught by CI because the build-wheels jobs were skipped.
|
I haven't had time to debug the errors here. Recorded for posterity/convenience: |
benjyw
approved these changes
Nov 19, 2024
|
I think the compatible with recent Python pip version is only in Pants 2.24, correct? |
|
Ah yes, not installing Python 3.13 works better. |
|
(Marking this as draft, it doesn't need to be merged before the 2.23.0 release and so better to wait.) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The project is currently using a mix of the GitHub supplied
setup-pythonaction andpantsbuild/actions/expose-pythons. On GitHub managed runners they behave similarly:setup-pythonwill install a version of Python if missing, or add it to the PATH if already present at the expected location (managed runners historically have multiple Pythons baked into the image)expose-pythonswill add all Python's at the expected GitHub location to the PATH. (Which ones is up to GitHub.)So today the invocation of
setup-pythonfollowed byexpose-pythonsis redundant.Consolidating on
setup-pythonlet's us be explicit about expected versions (more like the ARM image) and reduces the number of custom actions the project manages while still making multiple Python versions available.NOTE: The awkward double newlines in the final yaml are a pre-existing issue
https://stackoverflow.com/questions/45004464/yaml-dump-adding-unwanted-newlines-in-multiline-strings
See #21552 for some history regarding the
setup-vsexpose-actions.