Skip to content
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

Unhelpful error message related to unresolvable python version #421

Closed
2 of 5 tasks
scop opened this issue Jun 8, 2022 · 22 comments
Closed
2 of 5 tasks

Unhelpful error message related to unresolvable python version #421

scop opened this issue Jun 8, 2022 · 22 comments
Labels
bug Something isn't working

Comments

@scop
Copy link

scop commented Jun 8, 2022

Description:
When neither python-version or python-version-file is set, and there is no version file matching python-version-file's default (.python-version), the error message could use an improvement. Currently it is not helpful, and even somewhat confusing.

Action version:
v4.0.0

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:

All.

Repro steps:
https://github.com/scop/bash-completion/runs/6798283007?check_suite_focus=true#step:3:1

Expected behavior:
Helpful error message. For example:

Error: Could not determine Python version to set up. Please either specify a version using python-version in the action configuration, or path to a valid version file using python-version-file. The version file currently in effect, /home/runner/work/bash-completion/bash-completion/.python-version, does not exist.

Actual behavior:
Unhelpful error message:

Error: The specified python version file at: /home/runner/work/bash-completion/bash-completion/.python-version does not exist

There are two problems with this:

  1. I did not specify a python version file
  2. It's kind of an unexpected change coming from setup-python v3, and does not point towards a good way to fix it. The only hint is towards pointing to .python-version which isn't arguably good advice, because those files in many workflows are not something that are supposed to be checked in to git in the first place, because they are expected to contain for example pyenv virtual env names which are specific to developer local setups, not something to share. In that sense, it is also a quite unfortunate default.
@dmitry-shibanov
Copy link
Contributor

Hello @scop. Thank you for your report. We will investigate the issue. I think we should update the error message to be more informative and add notes about the new behaviour to the documentation.

@hairmare
Copy link

hairmare commented Jun 9, 2022

Is there any reason why it can't just default to the previous behavior when no python-version is provided and the .python-version file does not exist?

I see myself touching a lot of repositories to get this merged otherwise since i tend to just use the latest version provided by the action when i'm using it in the context of having to run some generic python tooling rather than working on an actual python project.

Would the recommended solution be to just use the system python that is provided by the runner?

@scop
Copy link
Author

scop commented Jun 10, 2022

Is there any reason why it can't just default to the previous behavior when no python-version is provided and the .python-version file does not exist?

That's what I'd prefer, too.

@ljmf00
Copy link

ljmf00 commented Jun 10, 2022

Is there any reason why it can't just default to the previous behavior when no python-version is provided and the .python-version file does not exist?

I dont see a clear reason in the changelog. I agree with the proposed behaviour. This quite big breaking change is unnecessary/avoidable.

The latest stable python was always the default, the user has the ability to choose if they want a specific version. Plus a reason to not go forward with this, is the fact that dependabot doesn't update to the latest version of python defined in the workflow, only the action, meaning pinning vulnerable tags is going to be more common.

@henryiii
Copy link

I've got lots of repositories with things like:

  steps:
    - uses: actions/checkout@v3
    - uses: actions/setup-python@v3
    - uses: pre-commit/action@v3.0.0 # or some other one line run here

(and tutorials, etc). All of them will have to have a manual version added, that now needs to be updated yearly. It does seem like a default to fall back on "latest" if the version file isn't present like v1-v3 would be really nice if possible! These repo's don't have a single Python "version" that should be locked by a file, it's just needed to do a bit of Python for style, updates, etc. The regular CI jobs have the normal matrix of versions. Adding the .python-version is great, but losing the ability to run this in a single line and getting the latest Python seems like a downgrade. (Maybe there's some technical reason I couldn't find?) (Also, there doesn't seem to be a way to specify latest at all - #300).

@hairmare
Copy link

hairmare commented Jun 11, 2022

Is there any reason why it can't just default to the previous behavior when no python-version is provided and the .python-version file does not exist?

The argument for the change made in https://github.com/actions/setup-python/pull/336/files#r854878982 is that it brings it inline with what the other setup- actions (like setup-node and setup-ruby) are doing.

Whilst i'm not all to happy about this, i can see the merits of it, so i guess dependabot is going to provide me with ample work over the course of the next few days or weeks (no biggie tho).

The closest I've found to specifying latest so far is using 3.x, so i'll be updating all my non-python repos that run actions like the aforementioned pre-commit/action with the following:

    - uses: actions/setup-python@v4
      with:
        python-version: '3.x'

Looking back at 2to3, this does seem to make sense (although i hope there will never be a reason for 3to4 given tooling like pyupgrade exists and is being used more broadly nowadays).

@scop
Copy link
Author

scop commented Jun 11, 2022

The closest I've found to specifying latest so far is using 3.x

Perhaps somewhat cosmetic as far as the real world effects go, but note that per the usage docs, this supports "SemVer's version range syntax", which I guess means https://docs.npmjs.com/cli/v6/using-npm/semver#ranges. In that sense, one might want to use something like >=3.6 if there's a lower bound and the intent is that not all 3.x versions are fine. And it would, for better or for worse, probably also mean that major versions > 3 (if they ever materialize) would match.

@abitrolly
Copy link

abitrolly commented Jun 13, 2022

@hairmare

The argument for the change made in https://github.com/actions/setup-python/pull/336/files#r854878982 is that it brings it inline with what the other setup- actions (like setup-node and setup-ruby) are doing.

From https://github.com/actions/setup-node docs.

The node-version input is optional. If not supplied, the node version from PATH will be used. However, it is recommended to always specify Node.js version and don't rely on the system one.

So it doesn't behave like setup-python@v4, which fails if version is not set.

@brettcannon
Copy link

Regardless of whether the semantics get brought back to v3, having a clearer error message about needing to specify one of python-version or python-version-file would help.

@henryiii
Copy link

henryiii commented Jun 15, 2022

IMO, it would have been better to update setup-ruby to match setup-node and setup-python, rather than remove something useful (especially for beginners) from setup-python. Also setup-ruby was moved to ruby/setup-ruby, so it's not even in actions/ anymore.

Also, it should be pointed out that .ruby-version is for rbenv, which is a very standard and common way to configure multiple Ruby's - I use it on all my Ruby projects. While .python-version is from pyenv, which is a fork of rbenv for Python and much less of a standard; it is not properly supported by many tools like CMake, nox, tox, etc. I've personally only used it to compile beta versions once in a while, and I work on dozens of Python projects. I believe far more people use Conda in Python, which gets the Python version very differently. And there are other active experiments in multiple Python support. Supporting it is fine, but forcing it is not ideal.

I realized that setting python-version: "3.x" or ">=3.7" does at least remove the need to update the Python version once a year - if a default is not going to be re-added, then I can start updating my projects, examples, cookiecutter, and tutorials to use that.

@abitrolly
Copy link

The official way to specify Python version is using pyproject.toml.

@abitrolly
Copy link

abitrolly commented Jun 16, 2022

Regardless of whether the semantics get brought back to v3, having a clearer error message about needing to specify one of python-version or python-version-file would help.

It also also would help having updated the README.md with v1-v4 changelog and parameter reference listing defaults. Having a breaking change version bump without documenting the break steals time.

@henryiii
Copy link

The official way to specify Python version is using pyproject.toml.

Just a warning, this is all-or-nothing. If you specify any [project] entries, you need to use all of them and support PEP 621 builds if you are developing a package. Even setuptools supports that now (61+), so you usually can do it1, but you can't do it half way (such as also providing setup.cfg or setup.py with base configuration there). I apparently was responsible via cibuildwheel docs and implementation for some partial specifications and got (rightfully) yelled at by the setuptools authors. :)

If you are not developing a package (like a website, analysis, etc), then it's a bit weird to specify just this, but you aren't building, so it's probably not harmful. A non-package project is more likely to be happy with .python-version, though as I mentioned it's not the only or even most common way like .ruby-version is.

Footnotes

  1. Poetry won't support this until 2.0.

abitrolly added a commit to abitrolly/cibuildwheel that referenced this issue Jun 18, 2022
  checkout@v2 -> v3
  upload/download-artifact@v2 -> v3
  python-setup@v2 -> v3
  pypa/gh-action-pypi-publish@1.4.2 -> v1.5.0

Python latest is v4, but it breaks working with default Python version
actions/setup-python#421
@IvanZosimov
Copy link
Contributor

Hi, everyone 👋 ! This PR was created to fix problems with ResolveVersionInput(). If you have any ideas regarding it, feel free to share them in the comments.

@IvanZosimov
Copy link
Contributor

Hi, everyone 👋 PR with fixes was merged, and by now I'm going to close this issue. If you have any questions or additions, feel free to ping us!

@abitrolly
Copy link

@IvanZosimov so the explicit version is still a hard requirement, right?

@ljmf00
Copy link

ljmf00 commented Jul 5, 2022

@IvanZosimov so the explicit version is still a hard requirement, right?

Apparently, yes 🙃.

@IvanZosimov
Copy link
Contributor

IvanZosimov commented Jul 5, 2022

Hi, @abitrolly and @ljmf00 👋 ! It's not a hard requirement, If you don't specify the python-version the action will use the system's python version and that's it. If the system's python is not what you would like, you can use python-version input and specify the version using semantic versioning notation, including ranges.

@scop
Copy link
Author

scop commented Jul 5, 2022

I suggest linking to the syntax available for ranges from README.md. Simply referring to them as "SemVer ranges" leaves some ambiguity, Python oriented audience is quite likely not aware that this means the particular npm package's range support -- the general concept of ranges does not exist in semantic versioning proper (as in https://semver.org).

@abitrolly
Copy link

@IvanZosimov nice to know. Is it possible to add a changelog to this action to reference to changes made between action versions?

@IvanZosimov
Copy link
Contributor

IvanZosimov commented Jul 6, 2022

Thank you for your suggestions, @scop and @abitrolly ❤️ They really make sense, so we will try to make documentation and change logs clearer.

@ssbarnea
Copy link

ssbarnea commented Jul 8, 2022

We can look at the bright side, that change was introduced in v4, so it did follow the semver --- it was indeed a breaking change.

I don't think that this bug should be closed because there is no reason for dropping the ability to "just install a python version", without mentioning which one you want.

In ~90% of cases I do mention specific python versions but I do also have ~10% of cases, where, on-purpose, I do not mention the python version. Those jobs are those testing the system compatibility.

With v4, that is no longer possible, I bet I am not the only one facing this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

9 participants