Skip to content

Conversation

@DimitriPapadopoulos
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented May 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.20%. Comparing base (f2e83fc) to head (6cec618).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #666   +/-   ##
=======================================
  Coverage   53.20%   53.20%           
=======================================
  Files          13       13           
  Lines        1109     1109           
=======================================
  Hits          590      590           
  Misses        519      519           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@agronholm
Copy link
Contributor

I remember being burned by removing this before, but maybe the time is ripe this time around.

@agronholm agronholm merged commit a352b89 into pypa:main May 4, 2025
16 checks passed
@DimitriPapadopoulos DimitriPapadopoulos deleted the 3.x branch May 4, 2025 12:41
@DimitriPapadopoulos
Copy link
Contributor Author

I remember being burned by removing this before, but maybe the time is ripe this time around.

@agronholm Other maintainers claim that some version of Python needs to be defined for stability:

However, I fail to see how 3.x defines some version of Python from reading the documentation:
https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#using-the-python-version-input

Although I cannot find anything in the documentation clearly supporting that, I now suspect that python-version: 3.x might mean the latest release of Python 3, while the lack of python-version could pull any version of Python, typically the default version of Python bundled with the runner.

@DimitriPapadopoulos
Copy link
Contributor Author

@agronholm From actions/setup-python/README.md:

The python-version input is optional. If not supplied, the action will try to resolve the version from the default .python-version file. If the .python-version file doesn't exist Python or PyPy version from the PATH will be used.

The curial part of information that I missed from https://github.com/actions/setup-python/blob/main/docs/advanced-usage.md#using-the-python-version-input is:

x-ranges to specify the latest stable version of Python (for the specified major version):

Should I revert?

I still wonder why we use 3.x instead of 3 but 3.13 instead of 3.13.x.

@woodruffw
Copy link
Member

FWIW I'd suggest a revert here per the twine discussion: it's unfortunately documented (IMO) but 3.x is indeed a load-bearing setting most of the time.

@agronholm
Copy link
Contributor

May I ask what even prompted this PR @DimitriPapadopoulos ? Was something broken or were you using setup-python without the python-version parameter in your own projects?

@henryiii
Copy link
Contributor

henryiii commented May 5, 2025

There was a period when they made it an error to not define this input. (v4, I think). But it seems it might not be needed anymore? I'm pretty sure I've been leaving it off recently and it's been fine. Edit: I'm not sure about this, I've been using hynek/build-and-inspect-python-package for the dist builds, so setting up a Python version isn't needed there. But I could be wrong; it's worrisome that this CI job isn't tested. I'll see if I can find a job without this set.

v4 clearly states either python-version or python-version-file is required. Not seeing a mention otherwise.

@henryiii
Copy link
Contributor

henryiii commented May 5, 2025

Didn't see it missing, so just stuck it in a test PR (henryiii/check-sdist#99).

This is what it prints out:

Warning: Neither 'python-version' nor 'python-version-file' inputs were supplied. Attempting to find '.python-version' file.
Warning: .python-version doesn't exist.
Warning: The `python-version` input is not set.  The version of Python currently in `PATH` will be used.

So it pretty much does nothing when nothing is set here (though the default Python on the path for the runners may not be terrible). I believe it used to error, now it does not.

@woodruffw
Copy link
Member

Yep, when unset it falls back to the runner's system Python. IMO this is liable to cause confusion in some cases (maybe not here!) since the runner Python is sometimes EOL (meaning failed resolutions for packages that expect non-EOL Pythons).

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 5, 2025

Nothing was broken, but the documentation of actions/setup-python could be clearer about python-version. And indeed, python-version was not used in other projects.

In this case, could the version of Python bundled with ubuntu-latest be EOL someday? Should you always publish using the latest release of Python? Perhaps you simply don't want the warnings shown in #666 (comment)...

@agronholm
Copy link
Contributor

FWIW I'd suggest a revert here per the twine discussion: it's unfortunately documented (IMO) but 3.x is indeed a load-bearing setting most of the time.

Are you saying something bad will happen if it stays removed? What's the practical difference between setting it to 3.x and leaving it out at this point?

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 5, 2025

  • Setting python-version: 3.x will pull the latest release of Python 3 (currently 3.13.3).
  • Leaving out python-version will fall back on the version of Python bundled with the runner ubuntu-latest (currently 3.12.3 bundled with ubuntu-24.04).

So without python-version, the version of Python used in this job build will remain 3.12.3 until ubuntu-latest switches to ubuntu-26.04. Python 3.12 will not reach EOL before 2028-10 so I don't expect something bad happening, and this should be true for future iterations of ubuntu-latest.

Something bad might happen when specifically using older distributions than ubuntu-latest. For example, ubuntu-22.04 bundles Python 3.10.12 which will reach EOL on 2026-10; if ubuntu-22.04 is not phased out before 2026-10, this runner will in effect run an EOL'ed version of Python. Worse, dependencies of the build job might require newer versions of Python before 2026-10; but then I am not aware of such dependencies.

In short, there might be a risk for ubuntu-24.04 runners, but I suspect ubuntu-latest runners are designed not to be at risk: I expect them to be updated before the bundled Python version reaches EOL or before major dependencies stop supporting it.

@woodruffw
Copy link
Member

woodruffw commented May 5, 2025

Are you saying something bad will happen if it stays removed? What's the practical difference between setting it to 3.x and leaving it out at this point?

The practical difference is twofold:

  1. Setting python-version explicitly always (to my understanding) installs a GH-managed Python version rather than using a runner version. This matters if PEP 668 is a consideration, i.e. if with workflow does a bare pip install or similar without an intermediating venv -- "runner" Pythons will vary in their behavior over time, while GH-managed Python versions intentionally don't set the "externally-managed" marker (to my understanding, since they're explicitly ephemeral installations).
  2. For some runners (not in this repo, since you're using ubuntu-latest), omitting python-version means you end up running the workflow with an EOL'd Python. This can cause hard-to-debug workflow fails, particularly in release flows.

TL;DR: omitting python-version entirely means you get the runner's Python, including the runner's distribution's PEP 668 choices. These vary by runner type and version (and Python version) and aren't super well documented (IMO, even compared to setup-python's lackluster docs), so it's my general option that workflows should be explicit and declare python-version to the version they need (or, if all they need is a recent version, declare it as 3.x). In the worst case this has no practical effect (the runner would have been good enough), and in the best case it prevents a bunch of externally-managed-environment and version mismatch confusion.

(Or another way of putting it: without python-version or python-version-file you might as well remove actions/setup-python entirely, since it's effectively a no-op.)

agronholm added a commit that referenced this pull request May 5, 2025
@agronholm
Copy link
Contributor

Alright, reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants