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

extra is not honored with the dependency resolver #8285

Closed
gaborbernat opened this issue May 21, 2020 · 4 comments · Fixed by #8286
Closed

extra is not honored with the dependency resolver #8285

gaborbernat opened this issue May 21, 2020 · 4 comments · Fixed by #8286
Labels
auto-locked Outdated issues that have been locked by automation type: bug A confirmed bug or unintended behavior

Comments

@gaborbernat
Copy link

gaborbernat commented May 21, 2020

Steps to reproduce:

env VIRTUALENV_PIP=20.2b1 virtualenv venv --clear; env PIP_UNSTABLE_FEATURE="resolver" venv/bin/pip install 'setuptools_scm >= 3.5[toml]'                                                                           created virtual environment CPython3.8.3.final.0-64 in 1969ms
  creator CPython3Posix(dest=/Users/bgabor8/git/dqc/ecobre/venv, clear=True, global=False)
  seeder FromAppData(download=False, pip=20.2b1, setuptools=latest, wheel=latest, via=copy, app_data_dir=/Users/bgabor8/Library/Application Support/virtualenv/seed-app-data/v1.0.1)
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
Looking in indexes: http://localhost:3141/bernat/bb
Collecting setuptools_scm>=3.5[toml]
  Downloading http://localhost:3141/bernat/bb/%2Bf/0d2/3db3d43e0a43e/setuptools_scm-3.5.0-py2.py3-none-any.whl (26 kB)
Installing collected packages: setuptools-scm
Successfully installed setuptools-scm-3.5.0

without the resolver:

env VIRTUALENV_PIP=20.2b1 virtualenv venv --clear; env venv/bin/pip install 'setuptools_scm >= 3.5[toml]'                                                                                                            created virtual environment CPython3.8.3.final.0-64 in 2233ms
  creator CPython3Posix(dest=/Users/bgabor8/git/dqc/ecobre/venv, clear=True, global=False)
  seeder FromAppData(download=False, pip=20.2b1, setuptools=latest, wheel=latest, via=copy, app_data_dir=/Users/bgabor8/Library/Application Support/virtualenv/seed-app-data/v1.0.1)
  activators BashActivator,CShellActivator,FishActivator,PowerShellActivator,PythonActivator,XonshActivator
Looking in indexes: http://localhost:3141/bernat/bb
Collecting setuptools_scm>=3.5[toml]
  Downloading http://localhost:3141/bernat/bb/%2Bf/0d2/3db3d43e0a43e/setuptools_scm-3.5.0-py2.py3-none-any.whl (26 kB)
Collecting toml; extra == "toml"
  Downloading http://localhost:3141/bernat/bb/%2Bf/bda/89d5935c2eac5/toml-0.10.1-py2.py3-none-any.whl (19 kB)
Installing collected packages: toml, setuptools-scm

This sadly is blocking me from doing more testing, as does not allow me to build a package I'll install afterwards.

@triage-new-issues triage-new-issues bot added the S: needs triage Issues/PRs that need to be triaged label May 21, 2020
@uranusjr
Copy link
Member

uranusjr commented May 21, 2020

Hmm. We read extras here:

def __init__(self, ireq, factory):
# type: (InstallRequirement, Factory) -> None
assert ireq.link is None, "This is a link, not a specifier"
self._ireq = ireq
self._factory = factory
self.extras = ireq.req.extras

But weirdly ireq.req.extras does not contain the extras, only ireq.extra does. I don’t really understand why, but will file a PR to change this anyway. We are already using ireq.extras in many other places in the resolver anyway, and I guess it makes sense to always do the same?

(The weird thing about this is that extras listed in a package’s Requires-Dist are correctly listed in ireq.req.extras, only the root requirements’ aren’t.)

@uranusjr
Copy link
Member

uranusjr commented May 21, 2020

Ah, I think I know what’s going on. setuptools_scm >= 3.5[toml] is actually not valid PEP 508 syntax, but pip accepts it anyway. The parser augments packaging’s Requirement to parse this additional part, and attaches that onto InstallRequirement to go along with the extras in Requirement.

So:

  • setuptools_scm>=3.5[toml] will have ireq.extras == {'toml'} but ireq.req.extras == set().
  • setuptools_scm[toml]>=3.5 will have ireq.extras == ireq.req.extras == {'toml'}.

We currently only tests the valid PEP 508 syntax variant, which is why this slipped. I’ll file the PR to switch to ireq.extras, but @gaborbernat you should be able to use the PEP 508 syntax to work around the issue.

@uranusjr uranusjr added C: new resolver type: bug A confirmed bug or unintended behavior labels May 21, 2020
@triage-new-issues triage-new-issues bot removed the S: needs triage Issues/PRs that need to be triaged label May 21, 2020
@pradyunsg
Copy link
Member

pip accepts it anyway.

Gah. This needs to be fixed. I'm OK to switch us to ireq.extras in the new resolver for now, but we should add a tracking issue to deprecate and remove the code that results in pip accepting the non-PEP 508 format.

@pfmoore
Copy link
Member

pfmoore commented May 21, 2020

but we should add a tracking issue

Done. #8288

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 24, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation type: bug A confirmed bug or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants