diff --git a/news/6239.bugfix b/news/6239.bugfix new file mode 100644 index 00000000000..d157641e838 --- /dev/null +++ b/news/6239.bugfix @@ -0,0 +1,2 @@ +Always try to merge extras from same-named requirements to prevent ``package`` +from shadowing ``package[extra]``. diff --git a/src/pip/_internal/req/req_set.py b/src/pip/_internal/req/req_set.py index d1410e93514..e1dea071173 100644 --- a/src/pip/_internal/req/req_set.py +++ b/src/pip/_internal/req/req_set.py @@ -130,11 +130,27 @@ def add_requirement( # We'd want to rescan this requirements later return [install_req], install_req - # Assume there's no need to scan, and that we've already - # encountered this for scanning. - if install_req.constraint or not existing_req.constraint: + # If the incoming requirement is a constraint, its job is done. + if install_req.constraint: return [], existing_req + # Merge extras from existing and incoming requirements. This needs to + # be done always to avoid an existing req "swallowing" new extras given + # by a new req. (pypa/pip#6239) + cur_extras = set(existing_req.extras) + new_extras = set(install_req.extras) | cur_extras + if new_extras != cur_extras: + existing_req.extras = tuple(sorted(new_extras)) + logger.debug( + "Setting %s extras to: %s", + existing_req, existing_req.extras, + ) + + # If the existing requirement is NOT a constraint, we're done. + # Otherwise the incoming one needs to be checked against it. + if not existing_req.constraint: + return [existing_req], existing_req + does_not_satisfy_constraint = ( install_req.link and not ( @@ -152,13 +168,7 @@ def add_requirement( # If we're now installing a constraint, mark the existing # object for real installation. existing_req.constraint = False - existing_req.extras = tuple(sorted( - set(existing_req.extras) | set(install_req.extras) - )) - logger.debug( - "Setting %s extras to: %s", - existing_req, existing_req.extras, - ) + # Return the existing requirement for addition to the parent and # scanning again. return [existing_req], existing_req diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 32cc1b36d2d..515e07fba9d 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -316,6 +316,33 @@ def test_hashed_deps_on_require_hashes(self): lineno=2 )) + @pytest.mark.parametrize( + ('req1_constraint', 'req2_constraint', 'expected_extras'), + [ + (False, False, ['jwt']), + (True, False, ['jwt']), + (False, True, []), + (True, True, []), + ], + ) + def test_add_requirement_extend_extra( + self, req1_constraint, req2_constraint, expected_extras): + """Make sure adding a requirement with extra merges it with + an existing one with the same name (if exists). + + See pypa/pip#6239. + """ + req1 = get_processed_req_from_line('boxsdk', lineno=1) + req1.constraint = req1_constraint + + req2 = get_processed_req_from_line('boxsdk[jwt]', lineno=1) + req2.constraint = req2_constraint + + reqset = RequirementSet() + reqset.add_requirement(req1) + reqset.add_requirement(req2) + assert list(reqset.get_requirement('boxsdk').extras) == expected_extras + @pytest.mark.parametrize(('file_contents', 'expected'), [ (b'\xf6\x80', b'\xc3\xb6\xe2\x82\xac'), # cp1252 diff --git a/tests/yaml/install/extras.yml b/tests/yaml/install/extras.yml index ee23b5e4af0..e813a2becce 100644 --- a/tests/yaml/install/extras.yml +++ b/tests/yaml/install/extras.yml @@ -39,4 +39,3 @@ cases: - D 1.0.0 - E 1.0.0 - F 1.0.0 - skip: true