Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pip/req/req_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ def add_requirement(self, install_req, parent_req_name=None):
# If we're now installing a constraint, mark the existing
# object for real installation.
existing_req.constraint = False
existing_req.extras = tuple(set(existing_req.extras).union(
set(install_req.extras)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A logging.debug of the merge could be helpful.

# And now we need to scan this.
result = [existing_req]
# Canonicalise to the already-added object for the backref
Expand Down
2 changes: 1 addition & 1 deletion tests/data/packages/LocalExtras/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ def path_to_url(path):
name='LocalExtras',
version='0.0.1',
packages=find_packages(),
extras_require={ 'bar': ['simple'] },
extras_require={ 'bar': ['simple'], 'baz': ['singlemodule'] },
dependency_links=[DEP_URL]
)
43 changes: 43 additions & 0 deletions tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,3 +313,46 @@ def test_constrained_to_url_install_same_url(script, data):
'install', '--no-index', '-f', data.find_links, '-c',
script.scratch_path / 'constraints.txt', to_install)
assert 'Running setup.py install for singlemodule' in result.stdout


def test_install_with_extras_from_constraints(script, data):
to_install = data.packages.join("LocalExtras")
script.scratch_path.join("constraints.txt").write(
"file://%s#egg=LocalExtras[bar]" % to_install
)
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras')
assert script.site_packages / 'simple' in result.files_created


def test_install_with_extras_from_install(script, data):
to_install = data.packages.join("LocalExtras")
script.scratch_path.join("constraints.txt").write(
"file://%s#egg=LocalExtras" % to_install
)
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]')
assert script.site_packages / 'singlemodule.py'in result.files_created


def test_install_with_extras_joined(script, data):
to_install = data.packages.join("LocalExtras")
script.scratch_path.join("constraints.txt").write(
"file://%s#egg=LocalExtras[bar]" % to_install
)
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]'
)
assert script.site_packages / 'simple' in result.files_created
assert script.site_packages / 'singlemodule.py'in result.files_created


def test_install_with_extras_editable_joined(script, data):
to_install = data.packages.join("LocalExtras")
script.scratch_path.join("constraints.txt").write(
"-e file://%s#egg=LocalExtras[bar]" % to_install
)
result = script.pip_install_local(
'-c', script.scratch_path / 'constraints.txt', 'LocalExtras[baz]')
assert script.site_packages / 'simple' in result.files_created
assert script.site_packages / 'singlemodule.py'in result.files_created

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth having a case with a requirements file? Not conceptually different to install... but it might be nice to be explicit that

pip install foo foo[bar] foo[quux]

should no longer error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pip install foo foo[bar] foo[quux] would indeed be nice but seems unrelated to this PR that focuses on constraints... Am I missing something ?
Or maybe you thought you were commenting on #3198 ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I realised that the constraints bug was a special case of the general problem (which itself is a sub-special-case of the resolver problem). I think it would be confusing if extras work with constraints but not requirements or cli - so am arguing that we address them all concurrently, within thebounds of not having a resolver).