pipenv icon indicating copy to clipboard operation
pipenv copied to clipboard

Pipenv thinks a hidden path is a python version

Open tucked opened this issue 4 years ago • 11 comments

$ pipenv --python .tox/py39-lock/bin/python install .
Traceback (most recent call last):
  File "/ifs/home/dtucker/.local/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 1256, in invoke
    Command.invoke(self, ctx)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/decorators.py", line 73, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/click/decorators.py", line 21, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/cli/command.py", line 204, in cli
    clear=state.clear,
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/core.py", line 580, in ensure_project
    pypi_mirror=pypi_mirror,
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/core.py", line 498, in ensure_virtualenv
    python = ensure_python(three=three, python=python)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/core.py", line 388, in ensure_python
    path_to_python = find_a_system_python(python)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/core.py", line 354, in find_a_system_python
    python_entry = find_python(finder, line)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/utils.py", line 2196, in find_python
    result = finder.find_python_version(line)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/pythonfinder/pythonfinder.py", line 262, in find_python_version
    version_dict = self.parse_major(major, minor=minor, patch=patch, arch=arch)
  File "/ifs/home/dtucker/.local/pipx/venvs/pipenv/lib/python3.6/site-packages/pipenv/vendor/pythonfinder/pythonfinder.py", line 209, in parse_major
    int(version_dict["major"]) if version_dict.get("major") is not None else major
ValueError: invalid literal for int() with base 10: '.tox/py39-lock/bin/python'
  • Note: This was originally encountered via a tox.ini:
    commands = pipenv --python {envpython} install .
    

https://github.com/pypa/pipenv/blob/deefb637a646b4e5432e1934453da61a9683ad7e/pipenv/utils.py#L2212 Perhaps that should use parentheses instead of brackets (or just omit the . altogether)?

tucked avatar Jan 12 '21 19:01 tucked

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 avatar Jun 14 '22 13:06 hansingt

@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.

matteius avatar Jun 14 '22 13:06 matteius

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 pythonfinder issue if you still have the issue with latest pipenv, as we already have vendored in the latest pythonfinder==1.2.10

matteius avatar Jun 14 '22 13:06 matteius

@matteius I can reproduce the issue on Ubuntu 22.04 using the following versions:

python: 3.10.4 pipenv: 2022.6.7

Simple script to reproduce this, utilizes tox and tox-pipenv:

[tox]
requires = tox-pipenv
envlist = py310

[testenv]
skip_install = true

and the following Pipfile:

[[source]]
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"

[packages]

[dev-packages]
pytest = "*"

[requires]
python_version = "3.10"

tox -e py310 gives the following output:

tox -e py310
.tox create: /home/tha/test/.tox/.tox
.tox installdeps: tox-pipenv, tox >= 3.25.0
py310 create: /home/tha/test/.tox/py310
ERROR: invocation failed (exit code 1), logfile: /home/tha/test/.tox/py310/log/py310-0.log
================================================================= log start ==================================================================
Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/__main__.py", line 4, in <module>
    cli()
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/cli/options.py", line 56, in main
    return super().main(*args, **kwargs, windows_expand_args=False)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1637, in invoke
    super().invoke(ctx)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/decorators.py", line 84, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/cli/command.py", line 193, in cli
    ensure_project(
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/core.py", line 553, in ensure_project
    ensure_virtualenv(
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/core.py", line 473, in ensure_virtualenv
    python = ensure_python(project, three=three, python=python)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/core.py", line 362, in ensure_python
    path_to_python = find_a_system_python(python)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/core.py", line 333, in find_a_system_python
    python_entry = find_python(finder, line)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/utils/shell.py", line 309, in find_python
    result = finder.find_python_version(line)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/pythonfinder/pythonfinder.py", line 262, in find_python_version
    version_dict = self.parse_major(major, minor=minor, patch=patch, arch=arch)
  File "/home/tha/test/.tox/.tox/lib/python3.10/site-packages/pipenv/vendor/pythonfinder/pythonfinder.py", line 209, in parse_major
    int(version_dict["major"]) if version_dict.get("major") is not None else major
ValueError: invalid literal for int() with base 10: '.tox/bin/python'

================================================================== log end ===================================================================
ERROR: InvocationError for command /home/tha/test/.tox/.tox/bin/python -m pipenv --python .tox/bin/python (exited with code 1)

hansingt avatar Jun 14 '22 14:06 hansingt

Thanks @hansingt -- would you mind opening a report with pythonfinder library and link it back here? I don't have access to that repository at this time so we need to go through the proper channels to get it fixed in pythonfinder and a new version that we can vendor into pipenv.

matteius avatar Jun 14 '22 14:06 matteius

@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 pythonfinder but with pipenv:

The traceback mentions https://github.com/pypa/pipenv/blob/16722f3f2cd56b3ee8e920855a32523f82dbf369/pipenv/utils/shell.py#L309, looking at it, it seems to me, that it takes the wrong branch, because in https://github.com/pypa/pipenv/blob/16722f3f2cd56b3ee8e920855a32523f82dbf369/pipenv/utils/shell.py#L308 the re.match(r"[\d.]+", line) matches the dot in the .tox/bin/python line.

hansingt avatar Jun 15 '22 05:06 hansingt

could that just be r"\d[\d.]+"?

ns-rorsten avatar Aug 04 '22 14:08 ns-rorsten

could that just be r"\d[\d.]+"?

Hm.. not sure. At least, this forces the version to start with a digit, but what about strings like 1337. or 5...? I think, we need some groups to make sure, that a dot is followed by at least one digit.

Maybe something like r"\d+(?:\.\d+)*"? This would match strings like 3 and 3.10, but not 3. or 3.fuu.

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 sys.version_info.

hansingt avatar Aug 05 '22 06:08 hansingt

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 packaging.version.VERSION_PATTERN that matches PEP 440 versions, and make sure the version is the whole string, with something like re.match(f'^{VERSION_PATTERN}$').

ns-rorsten avatar Aug 08 '22 15:08 ns-rorsten

I think we want to be able to support pre release versions too like 3.11.0b4. I haven't looked into packaging.version.VERSION_PATTERN yet, but we have it available to us already as from pipenv.patched.pip._vendorpackaging.version import VERSION_PATTERN

matteius avatar Aug 08 '22 22:08 matteius

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 packaging.version.VERSION_PATTERN that matches PEP 440 versions, and make sure the version is the whole string, with something like re.match(f'^{VERSION_PATTERN}$').

That sounds reasonable to me.

I think we want to be able to support pre release versions too like 3.11.0b4. I haven't looked into packaging.version.VERSION_PATTERN yet, but we have it available to us already as from pipenv.patched.pip._vendorpackaging.version import VERSION_PATTERN

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.

hansingt avatar Aug 09 '22 05:08 hansingt

How about something like this?

if Path(line).is_file():
    return line
return finder.find_python_version(line)

tucked avatar Aug 17 '22 18:08 tucked

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: https://github.com/pypa/pipenv/blob/897caca7752a9d2d648ccb3407aca30bd7962941/pipenv/utils/shell.py#L288-L291

Should this be changed to detect file as well?

dqkqd avatar Aug 25 '22 02:08 dqkqd

SGTM 👍

tucked avatar Aug 25 '22 22:08 tucked

There is another potential source of error: tox-pipenv plug-in. I've recently been going through the code and noticed a few errors and places that could generate this problem. Requires more research but wanted to note it here in case I don't get back to it immediately.

kalebmckale avatar Apr 26 '23 08:04 kalebmckale

Could we recheck this behaviro on pipenv==2023.5.19 where I overhauled pythonfinder a lot?

matteius avatar May 20 '23 00:05 matteius

I am still able to reproduce the issue with Python 3.9 + pipenv, version 2023.5.19

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 .

tucked avatar May 23 '23 17:05 tucked

Thanks @tucked -- I was able to reproduce some issues with your example, and have a patch PR out for it: https://github.com/pypa/pipenv/pull/5712

matteius avatar May 31 '23 13:05 matteius

pipenv==2023.6.2 has been released -- should address this issue

matteius avatar Jun 02 '23 10:06 matteius

LGTM! Thanks 👍

tucked avatar Jun 03 '23 00:06 tucked