-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Pipenv thinks a hidden path is a python version #4588
Comments
This issue is open for over one year now and still persists. Is there any progress on this? Can we expect it to be fixed soon? |
@hansingt Nobody is assigned to working on this particular issue presently AFAIK -- Please see the latest pinned issue in the backlog and feel free to help contribute towards the solution. |
Just noting that the original error report is for py 3.6 in the example, and doesn't call out the version of pipenv tested with. What is the latest version of pipenv you have seen the issue with @hansingt ? Seems this might be a |
@matteius I can reproduce the issue on Ubuntu 22.04 using the following versions: python: 3.10.4 Simple script to reproduce this, utilizes [tox]
requires = tox-pipenv
envlist = py310
[testenv]
skip_install = true and the following [[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
[packages]
[dev-packages]
pytest = "*"
[requires]
python_version = "3.10"
|
Thanks @hansingt -- would you mind opening a report with |
@matteius I opened the issue at pythonfinder, but I took a closer look at the problem and I don't think it's problem with The traceback mentions Line 309 in 16722f3
Line 308 in 16722f3
re.match(r"[\d.]+", line) matches the dot in the .tox/bin/python line.
|
could that just be |
Hm.. not sure. At least, this forces the version to start with a digit, but what about strings like Maybe something like But as long, as we don't make sure, to only match the last part of the path, this does not ensure, that it's really the version of the python interpreter. There might be a version number inside the path as well. I think, if we would want to make it really right, we cannot get past calling the interpreter and fetching the information from |
well, the goal is to prevent it from treating a path as a python version; it's supposed to use a python of its own if it's a version and the specified python if it's a path, and I think in that latter case people are taking responsibility for ensuring that executable is appropriate. So maybe in this case it would be appropriate to borrow |
I think we want to be able to support pre release versions too like |
That sounds reasonable to me.
Look quite complicated (maybe too complicated for our case?) to me: https://github.com/pypa/packaging/blob/main/packaging/version.py#L113 But yes, it does support pre- and post-releases as well as epochs. So, everything PEP 440 specifies as a "valid version". I'm not sure, what might occurr in a python interpreter version, but it does not hurt to support them. |
How about something like this? if Path(line).is_file():
return line
return finder.find_python_version(line) |
It's already had, but with absolute path: Lines 288 to 291 in 897caca
Should this be changed to detect file as well? |
SGTM 👍 |
There is another potential source of error: |
Could we recheck this behaviro on |
I am still able to reproduce the issue with Python 3.9 + tox -e py39-lock --notest # tox 4.5.1 fwiw
python3.9 -m venv venv
venv/bin/python -m pip install 'pipenv==2023.5.19'
venv/bin/pipenv --python .tox/py39-lock/bin/python install . |
pipenv==2023.6.2 has been released -- should address this issue |
LGTM! Thanks 👍 |
tox.ini
:commands = pipenv --python {envpython} install .
pipenv/pipenv/utils.py
Line 2212 in deefb63
Perhaps that should use parentheses instead of brackets (or just omit the
.
altogether)?The text was updated successfully, but these errors were encountered: