`pipenv requirements` now requiring installation of VCS modules #5755

mgmarino · 2023-06-28T08:32:21Z

Issue description

This is less a "bug" per se, but it is absolutely a change in behavior that was not marked in the release notes, so I'm not sure if it was intended or not. Also, it's not clear why pipenv requirements needs to install via VCS, I would've assumed it's a simple translation of the Pipfile.lock file.

As a side note, this was also a breaking change for us, because we use pipenv requirements in places where ssh keys are not always available.

Steps to replicate

Pipfile:

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

[packages]
pyjwt = {extras = ["crypto"], editable = true, ref = "2.6.0", git = "ssh://git@github.com/jpadilla/pyjwt.git"}

[dev-packages]

[requires]
python_version = "3.9"

Lock the above with pipenv lock. Then run pipenv requirements with the two versions:

> pip install "pipenv==2023.6.18"
> pipenv requirements
...
cffi==1.15.1
cryptography==41.0.1 ; python_version >= '3.7'
pycparser==2.21
-e git+ssh://git@github.com/jpadilla/pyjwt.git@7665aa625506a11bae50b56d3e04413a3dc6fdf8#egg=pyjwt[crypto]

This is also very fast and shows no indication of network calls

> pip install "pipenv==2023.6.26"
> pipenv requirements
...
INFO:root:running egg_info
INFO:root:creating /var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info
INFO:root:writing /var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/PKG-INFO
INFO:root:writing dependency_links to /var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/dependency_links.txt
INFO:root:writing requirements to /var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/requires.txt
INFO:root:writing top-level names to /var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/top_level.txt
INFO:root:writing manifest file '/var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/SOURCES.txt'
INFO:root:reading manifest file '/var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/SOURCES.txt'
INFO:root:reading manifest template 'MANIFEST.in'
WARNING:root:warning: no previously-included files found matching 'codecov.yml'
WARNING:root:warning: no previously-included files matching '*' found under directory 'docs/_build'
WARNING:root:warning: no previously-included files matching '*.py[co]' found under directory '*'
WARNING:root:warning: no previously-included files matching '__pycache__' found under directory '*'
INFO:root:adding license file 'LICENSE'
INFO:root:adding license file 'AUTHORS.rst'
INFO:root:writing manifest file '/var/folders/jh/0_l94d8n51n2gjzt_5l8mv8r0000gq/T/reqlib-src9gkpd_ke/pyjwt/reqlib-metadata/PyJWT.egg-info/SOURCES.txt'
/.../.pyenv/versions/3.9.16/lib/python3.9/site-packages/pipenv/patched/pip/_internal/models/link.py:391: PipDeprecationWarning: DEPRECATION: git+ssh://****@github.com/jpadilla/pyjwt.git@7665aa625506a11bae50b56d3e04413a3dc6fdf8#egg=pyjwt[crypto] contains an egg fragment with a non-PEP 508 name pip 25.0 will enforce this behaviour change. A possible replacement is to use the req @ url syntax, and remove the egg fragment. Discussion can be found at https://github.com/pypa/pip/issues/11617
  deprecated(
cffi==1.15.1
cryptography==41.0.1 ; python_version >= '3.7'
pycparser==2.21
-e git+ssh://git@github.com/jpadilla/pyjwt.git@7665aa625506a11bae50b56d3e04413a3dc6fdf8#egg=pyjwt[crypto]

This takes quite a bit longer, and includes the network calls, etc.

Note, I have removed the deprecation warnings mentioned in #5626.

The text was updated successfully, but these errors were encountered:

matteius · 2023-06-28T10:58:07Z

This was a side-effect in my larger refactor of requirementslib to version 3.0.0 -- while the goal was to migrate away from attrs to pydantic, I encountered some weird behaviors with some of the prior requirementslib code. The problem here is the requirementslib method is used to just go with the name pyjwt from your Pipfile line if it could, but that might actually be a different name than the package metadata so now requirementslib now has to obtain the package and generate the actual metadata to get the name, which I think is technically the more correct thing to do. I didn't really see a way around moving requirementslib away from the idea of working with fake package lines.

oz123 · 2023-06-28T11:25:41Z

Another thing is that Pipfile still uses the old syntax which raises a warning:

 git+ssh://git@github.com/jpadilla/pyjwt.git@7665aa625506a11bae50b56d3e04413a3dc6fdf8#egg=pyjwt[crypto]

Both of these are accepted without a warning:

pip install "pyjwt[crypto] @ git+https://github.com/jpadilla/pyjwt.git"
pip install " "pyjwt[crypto] @ git+ssh://git@github.com/jpadilla/pyjwt.git"

However, these trigger an exception from requirementslib.

$ pipenv lock
Locking [packages] dependencies...
Building requirements...
Traceback (most recent call last):
  File "/home/oznt/.local/share/virtualenvs/pipenv-H8EVR25f/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/oznt/Software/pypa/pipenv/pipenv/cli/options.py", line 58, in main
    return super().main(*args, **kwargs, windows_expand_args=False)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/decorators.py", line 84, in new_func
    return ctx.invoke(f, obj, *args, **kwargs)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/click/decorators.py", line 26, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/oznt/Software/pypa/pipenv/pipenv/cli/command.py", line 369, in lock
    do_lock(
  File "/home/oznt/Software/pypa/pipenv/pipenv/routines/lock.py", line 79, in do_lock
    venv_resolve_deps(
  File "/home/oznt/Software/pypa/pipenv/pipenv/utils/resolver.py", line 1029, in venv_resolve_deps
    deps = convert_deps_to_pip(deps, project, include_index=True)
  File "/home/oznt/Software/pypa/pipenv/pipenv/utils/dependencies.py", line 285, in convert_deps_to_pip
    new_dep = Requirement.from_pipfile(dep_name, dep)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/requirementslib/models/requirements.py", line 2749, in from_pipfile
    r = VCSRequirement.from_pipfile(name, pipfile)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/requirementslib/models/requirements.py", line 2274, in from_pipfile
    cls_inst = cls(**creation_args)  # type: ignore
  File "<attrs generated init pipenv.vendor.requirementslib.models.requirements.VCSRequirement>", line 38, in __init__
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/attr/validators.py", line 269, in __call__
    self.validator(inst, attr, value)
  File "/home/oznt/Software/pypa/pipenv/pipenv/vendor/requirementslib/models/utils.py", line 571, in validate_path
    raise ValueError("Invalid path {0!r}".format(value))
ValueError: Invalid path 'pyjwt[crypto] @ git+https://github.com/jpadilla/pyjwt.git'

matteius · 2023-06-28T11:29:41Z

@oz123 that issue is being tracked by #5626

mgmarino · 2023-06-28T12:01:11Z

Hi @matteius Thanks for the response here. I understand your point about trying to determine the "real" name, but I have a few questions:

Can this not rather be determined at the point of installation/lock and appropriately stored in the Pipfile.lock? It seems like it could then "fail" if the names were inconsistent. I think it is more expected that a pipenv install or pipenv lock can initiate network calls and require access to the underlying modules.
Does PEP 508 give guidance here about what to do in the case of inconsistency?

My general issue is that, as a user, I would expect pipenv requirements to simply translate the Pipfile.lock file. I would also expect the Pipfile.lock to have sufficient information so as not to require network calls.

matteius · 2023-06-28T12:11:32Z

Does PEP 508 give guidance here about what to do in the case of inconsistency?

Not as far as I can tell.

I would expect pipenv requirements to simply translate the Pipfile.lock file.

@mgmarino This is a good point, and I think it ultimately should be doing that, but it was built re-using convert_deps_to_pip which uses new_dep = Requirement.from_pipfile(dep_name, dep) which generates the requirements line with new_dep.as_line and those are the same functions that are used to generate the lock file and the install lines.

Ultimately I'll treat this as a regression, and would be fine with moving towards requirements having its own functions that live outside requirementslib, since I am not convinced the re-use there is worth it. There may be a couple other requirements command issue reports that are hard to solve because of this re-use as well. Plus requirementslib needs additional refactor in the long run, and you are right that the lock should contain everything needed to generate the requirements.

mgmarino · 2023-06-28T12:16:57Z

@matteius Ok, thanks for all your efforts here! For the moment, where this causes us issues, we will pin pipenv to 2023.6.18.

matteius · 2023-06-28T12:42:16Z

@mgmarino Could you try out this branch and see if you notice anything Chat GPT and I missed in this half hour attempt at a patch 😅 #5757

matteius added the Type: Regression This issue is a regression of a previous behavior. label Jun 28, 2023

matteius mentioned this issue Jun 28, 2023

requirements command improvements, fixes issue 5755. #5757

Merged

2 tasks

matteius closed this as completed in #5757 Jun 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`pipenv requirements` now requiring installation of VCS modules #5755

`pipenv requirements` now requiring installation of VCS modules #5755

mgmarino commented Jun 28, 2023

matteius commented Jun 28, 2023

oz123 commented Jun 28, 2023

matteius commented Jun 28, 2023

mgmarino commented Jun 28, 2023 •

edited

Loading

matteius commented Jun 28, 2023

mgmarino commented Jun 28, 2023

matteius commented Jun 28, 2023 •

edited

Loading

pipenv requirements now requiring installation of VCS modules #5755

pipenv requirements now requiring installation of VCS modules #5755

Comments

mgmarino commented Jun 28, 2023

Issue description

Steps to replicate

matteius commented Jun 28, 2023

oz123 commented Jun 28, 2023

matteius commented Jun 28, 2023

mgmarino commented Jun 28, 2023 • edited Loading

matteius commented Jun 28, 2023

mgmarino commented Jun 28, 2023

matteius commented Jun 28, 2023 • edited Loading

`pipenv requirements` now requiring installation of VCS modules #5755

`pipenv requirements` now requiring installation of VCS modules #5755

mgmarino commented Jun 28, 2023 •

edited

Loading

matteius commented Jun 28, 2023 •

edited

Loading