Issue 4371 incorrect dependencies when install dev packages (#5234)

* Add test, ensure dev lock use default packages as constraints.

* Use default packages as constraints when locking develop packages.

* Add test, ensure installing dev-packages use default packages as constraints. (#4371) (#2987)

* Use default packages as constraints when installing provided dev packages.

* change vistir.path.normalize_path to pipenv.utils.shell.normalize_path

* Add function that get contraints from packages.

* Add test for get_constraints_from_deps function

* Use get_constraints_from_deps to get constraints

* Use @cached_property instead of @property

* Use standalone utility to write constraints file

* prepare_constraint_file use precomputed constraints.

* Add news fragment.

news/5234.bugfix.rst

@@ -0,0 +1,2 @@
+Use ``packages`` as contraints when locking ``dev-packages`` in Pipfile.
+Use ``packages`` as contraints when installing new ``dev-packages``.

pipenv/core.py

@@ -28,10 +28,12 @@ from pipenv.utils.constants import MYPY_RUNNING
 from pipenv.utils.dependencies import (
     convert_deps_to_pip,
     get_canonical_names,
+    get_constraints_from_deps,
     is_pinned,
     is_required_version,
     is_star,
     pep423_name,
+    prepare_constraint_file,
     python_version,
 )
 from pipenv.utils.indexes import get_source_list, parse_indexes, prepare_pip_source_args
@@ -41,6 +43,7 @@ from pipenv.utils.shell import (
     cmd_list_to_shell,
     find_python,
     is_python_command,
+    normalize_path,
     project_python,
     subprocess_run,
     system_which,
@@ -789,6 +792,7 @@ def batch_install(
                 trusted_hosts=trusted_hosts,
                 extra_indexes=extra_indexes,
                 use_pep517=use_pep517,
+                use_constraint=False,  # no need to use constraints, it's written in lockfile
             )
             c.dep = dep
 
@@ -1096,8 +1100,9 @@ def do_lock(
         for k, v in lockfile[section].copy().items():
             if not hasattr(v, "keys"):
                 del lockfile[section][k]
-    # Resolve dev-package dependencies followed by packages dependencies.
-    for is_dev in [True, False]:
+
+    # Resolve package to generate constraints before resolving dev-packages
+    for is_dev in [False, True]:
         pipfile_section = "dev-packages" if is_dev else "packages"
         if project.pipfile_exists:
             packages = project.parsed_pipfile.get(pipfile_section, {})
@@ -1453,12 +1458,14 @@ def pip_install(
     block=True,
     index=None,
     pre=False,
+    dev=False,
     selective_upgrade=False,
     requirements_dir=None,
     extra_indexes=None,
     pypi_mirror=None,
     trusted_hosts=None,
     use_pep517=True,
+    use_constraint=False,
 ):
     piplogger = logging.getLogger("pipenv.patched.pip._internal.commands.install")
     if not trusted_hosts:
@@ -1539,9 +1546,18 @@ def pip_install(
     )
     pip_command.extend(pip_args)
     if r:
-        pip_command.extend(["-r", vistir.path.normalize_path(r)])
+        pip_command.extend(["-r", normalize_path(r)])
     elif line:
         pip_command.extend(line)
+    if dev and use_constraint:
+        default_constraints = get_constraints_from_deps(project.packages)
+        constraint_filename = prepare_constraint_file(
+            default_constraints,
+            directory=requirements_dir,
+            sources=None,
+            pip_args=None,
+        )
+        pip_command.extend(["-c", normalize_path(constraint_filename)])
     pip_command.extend(prepare_pip_source_args(sources))
     if project.s.is_verbose():
         click.echo(f"$ {cmd_list_to_shell(pip_command)}", err=True)
@@ -2128,9 +2144,11 @@ def do_install(
                         selective_upgrade=selective_upgrade,
                         no_deps=False,
                         pre=pre,
+                        dev=dev,
                         requirements_dir=requirements_directory,
                         index=index_url,
                         pypi_mirror=pypi_mirror,
+                        use_constraint=True,
                     )
                     if c.returncode:
                         sp.write_err(

pipenv/resolver.py

@@ -745,12 +745,15 @@ def resolve_packages(pre, clear, verbose, system, write, requirements_dir, packa
         else None
     )
 
-    def resolve(packages, pre, project, sources, clear, system, requirements_dir=None):
+    def resolve(
+        packages, pre, project, sources, clear, system, dev, requirements_dir=None
+    ):
         return resolve_deps(
             packages,
             which,
             project=project,
             pre=pre,
+            dev=dev,
             sources=sources,
             clear=clear,
             allow_global=system,
@@ -769,6 +772,7 @@ def resolve_packages(pre, clear, verbose, system, write, requirements_dir, packa
     results, resolver = resolve(
         packages,
         pre=pre,
+        dev=dev,
         project=project,
         sources=sources,
         clear=clear,

pipenv/utils/dependencies.py

@@ -280,6 +280,60 @@ def convert_deps_to_pip(
     return f.name
 
 
+def get_constraints_from_deps(deps):
+    """Get contraints from Pipfile-formatted dependency"""
+    from pipenv.vendor.requirementslib.models.requirements import Requirement
+
+    def is_constraint(dep):
+        # https://pip.pypa.io/en/stable/user_guide/#constraints-files
+        # constraints must have a name, they cannot be editable, and they cannot specify extras.
+        return dep.name and not dep.editable and not dep.extras
+
+    constraints = []
+    for dep_name, dep in deps.items():
+        new_dep = Requirement.from_pipfile(dep_name, dep)
+        if is_constraint(new_dep):
+            c = new_dep.as_line().strip()
+            constraints.append(c)
+    return constraints
+
+
+def prepare_constraint_file(
+    constraints,
+    directory=None,
+    sources=None,
+    pip_args=None,
+):
+    from pipenv.vendor.vistir.path import (
+        create_tracked_tempdir,
+        create_tracked_tempfile,
+    )
+
+    if not directory:
+        directory = create_tracked_tempdir(suffix="-requirements", prefix="pipenv-")
+
+    constraints_file = create_tracked_tempfile(
+        mode="w",
+        prefix="pipenv-",
+        suffix="-constraints.txt",
+        dir=directory,
+        delete=False,
+    )
+
+    if sources and pip_args:
+        skip_args = ("build-isolation", "use-pep517", "cache-dir")
+        args_to_add = [
+            arg for arg in pip_args if not any(bad_arg in arg for bad_arg in skip_args)
+        ]
+        requirementstxt_sources = " ".join(args_to_add) if args_to_add else ""
+        requirementstxt_sources = requirementstxt_sources.replace(" --", "\n--")
+        constraints_file.write(f"{requirementstxt_sources}\n")
+
+    constraints_file.write("\n".join([c for c in constraints]))
+    constraints_file.close()
+    return constraints_file.name
+
+
 def is_required_version(version, specified_version):
     """Check to see if there's a hard requirement for version
     number provided in the Pipfile.

pipenv/utils/resolver.py

@@ -17,11 +17,15 @@ from pipenv.patched.pip._internal.network.cache import SafeFileCache
 from pipenv.patched.pip._internal.operations.build.build_tracker import (
     get_build_tracker,
 )
+from pipenv.patched.pip._internal.req.constructors import (
+    install_req_from_parsed_requirement,
+)
 from pipenv.patched.pip._internal.req.req_file import parse_requirements
 from pipenv.patched.pip._internal.utils.hashes import FAVORITE_HASH
 from pipenv.patched.pip._internal.utils.temp_dir import global_tempdir_manager
 from pipenv.project import Project
 from pipenv.vendor import click
+from pipenv.vendor.cached_property import cached_property
 from pipenv.vendor.requirementslib import Pipfile, Requirement
 from pipenv.vendor.requirementslib.models.requirements import Line
 from pipenv.vendor.requirementslib.models.utils import DIRECT_URL_RE
@@ -32,9 +36,11 @@ from .dependencies import (
     HackedPythonVersion,
     clean_pkg_version,
     convert_deps_to_pip,
+    get_constraints_from_deps,
     get_vcs_deps,
     is_pinned_requirement,
     pep423_name,
+    prepare_constraint_file,
     translate_markers,
 )
 from .indexes import parse_indexes, prepare_pip_source_args
@@ -117,6 +123,7 @@ class Resolver:
         skipped=None,
         clear=False,
         pre=False,
+        dev=False,
     ):
         self.initial_constraints = constraints
         self.req_dir = req_dir
@@ -126,6 +133,7 @@ class Resolver:
         self.hashes = {}
         self.clear = clear
         self.pre = pre
+        self.dev = dev
         self.results = None
         self.markers_lookup = markers_lookup if markers_lookup is not None else {}
         self.index_lookup = index_lookup if index_lookup is not None else {}
@@ -420,6 +428,7 @@ class Resolver:
         req_dir: str = None,
         clear: bool = False,
         pre: bool = False,
+        dev: bool = False,
     ) -> "Resolver":
 
         if not req_dir:
@@ -450,6 +459,7 @@ class Resolver:
             skipped=skipped,
             clear=clear,
             pre=pre,
+            dev=dev,
         )
 
     @classmethod
@@ -522,29 +532,13 @@ class Resolver:
         return self._pip_args
 
     def prepare_constraint_file(self):
-        from pipenv.vendor.vistir.path import create_tracked_tempfile
-
-        constraints_file = create_tracked_tempfile(
-            mode="w",
-            prefix="pipenv-",
-            suffix="-constraints.txt",
-            dir=self.req_dir,
-            delete=False,
+        constraint_filename = prepare_constraint_file(
+            self.initial_constraints,
+            directory=self.req_dir,
+            sources=self.sources,
+            pip_args=self.pip_args,
         )
-        skip_args = ("build-isolation", "use-pep517", "cache-dir")
-        args_to_add = [
-            arg
-            for arg in self.pip_args
-            if not any(bad_arg in arg for bad_arg in skip_args)
-        ]
-        if self.sources:
-            requirementstxt_sources = " ".join(args_to_add) if args_to_add else ""
-            requirementstxt_sources = requirementstxt_sources.replace(" --", "\n--")
-            constraints_file.write(f"{requirementstxt_sources}\n")
-        constraints = self.initial_constraints
-        constraints_file.write("\n".join([c for c in constraints]))
-        constraints_file.close()
-        return constraints_file.name
+        return constraint_filename
 
     @property
     def constraint_file(self):
@@ -552,6 +546,17 @@ class Resolver:
             self._constraint_file = self.prepare_constraint_file()
         return self._constraint_file
 
+    @cached_property
+    def default_constraint_file(self):
+        default_constraints = get_constraints_from_deps(self.project.packages)
+        default_constraint_filename = prepare_constraint_file(
+            default_constraints,
+            directory=self.req_dir,
+            sources=None,
+            pip_args=None,
+        )
+        return default_constraint_filename
+
     @property
     def pip_options(self):
         if self._pip_options is None:
@@ -630,12 +635,33 @@ class Resolver:
             )
         return self._parsed_constraints
 
+    @cached_property
+    def parsed_default_constraints(self):
+        pip_options = self.pip_options
+        pip_options.extra_index_urls = []
+        parsed_default_constraints = parse_requirements(
+            self.default_constraint_file,
+            constraint=True,
+            finder=self.finder,
+            session=self.session,
+            options=pip_options,
+        )
+        return parsed_default_constraints
+
+    @cached_property
+    def default_constraints(self):
+        default_constraints = [
+            install_req_from_parsed_requirement(
+                c,
+                isolated=self.pip_options.build_isolation,
+                user_supplied=False,
+            )
+            for c in self.parsed_default_constraints
+        ]
+        return default_constraints
+
     @property
     def constraints(self):
-        from pipenv.patched.pip._internal.req.constructors import (
-            install_req_from_parsed_requirement,
-        )
-
         if self._constraints is None:
             self._constraints = [
                 install_req_from_parsed_requirement(
@@ -646,6 +672,9 @@ class Resolver:
                 )
                 for c in self.parsed_constraints
             ]
+            # Only use default_constraints when installing dev-packages
+            if self.dev:
+                self._constraints += self.default_constraints
         return self._constraints
 
     @contextlib.contextmanager
@@ -870,6 +899,7 @@ def actually_resolve_deps(
     sources,
     clear,
     pre,
+    dev,
     req_dir=None,
 ):
     if not req_dir:
@@ -878,7 +908,7 @@ def actually_resolve_deps(
 
     with warnings.catch_warnings(record=True) as warning_list:
         resolver = Resolver.create(
-            deps, project, index_lookup, markers_lookup, sources, req_dir, clear, pre
+            deps, project, index_lookup, markers_lookup, sources, req_dir, clear, pre, dev
         )
         resolver.resolve()
         hashes = resolver.resolve_hashes()
@@ -1064,6 +1094,7 @@ def resolve_deps(
     python=False,
     clear=False,
     pre=False,
+    dev=False,
     allow_global=False,
     req_dir=None,
 ):
@@ -1094,6 +1125,7 @@ def resolve_deps(
                 sources,
                 clear,
                 pre,
+                dev,
                 req_dir=req_dir,
             )
         except RuntimeError:
@@ -1122,6 +1154,7 @@ def resolve_deps(
                     sources,
                     clear,
                     pre,
+                    dev,
                     req_dir=req_dir,
                 )
             except RuntimeError:

tests/integration/test_install_basic.py

@@ -509,3 +509,29 @@ requests = {version="*", index="pypi"}
             f.write(contents)
         c = p.pipenv("install")
         assert c.returncode == 0
+
+@pytest.mark.dev
+@pytest.mark.install
+def test_install_dev_use_default_constraints(PipenvInstance):
+    # See https://github.com/pypa/pipenv/issues/4371
+    # See https://github.com/pypa/pipenv/issues/2987
+    with PipenvInstance(chdir=True) as p:
+
+        c = p.pipenv("install requests==2.14.0")
+        assert c.returncode == 0
+        assert "requests" in p.lockfile["default"]
+        assert p.lockfile["default"]["requests"]["version"] == "==2.14.0"
+
+        c = p.pipenv("install --dev requests")
+        assert c.returncode == 0
+        assert "requests" in p.lockfile["develop"]
+        assert p.lockfile["develop"]["requests"]["version"] == "==2.14.0"
+
+        # requests 2.14.0 doesn't require these packages
+        assert "idna" not in p.lockfile["develop"]
+        assert "certifi" not in p.lockfile["develop"]
+        assert "urllib3" not in p.lockfile["develop"]
+        assert "chardet" not in p.lockfile["develop"]
+
+        c = p.pipenv("run python -c 'import urllib3'")
+        assert c.returncode != 0

tests/integration/test_lock.py

@@ -791,3 +791,38 @@ requests = {requirement}
                                       'sha256:ec22d826a36ed72a7358ff3fe56cbd4ba69dd7a6718ffd450ff0e9df7a47ce6a'],
                            'index': 'local', 'version': '==2.19.1'}
         assert p.lockfile['default']['requests'] == expected_result
+
+
+@pytest.mark.dev
+@pytest.mark.lock
+@pytest.mark.install
+def test_dev_lock_use_default_packages_as_constraint(PipenvInstance):
+    # See https://github.com/pypa/pipenv/issues/4371
+    # See https://github.com/pypa/pipenv/issues/2987
+    with PipenvInstance(chdir=True) as p:
+        with open(p.pipfile_path, 'w') as f:
+            contents = """
+[packages]
+requests = "<=2.14.0"
+
+[dev-packages]
+requests = "*"
+                """.strip()
+            f.write(contents)
+
+        c = p.pipenv("lock --dev")
+        assert c.returncode == 0
+        assert "requests" in p.lockfile["default"]
+        assert p.lockfile["default"]["requests"]["version"] == "==2.14.0"
+        assert "requests" in p.lockfile["develop"]
+        assert p.lockfile["develop"]["requests"]["version"] == "==2.14.0"
+
+        # requests 2.14.0 doesn't require these packages
+        assert "idna" not in p.lockfile["develop"]
+        assert "certifi" not in p.lockfile["develop"]
+        assert "urllib3" not in p.lockfile["develop"]
+        assert "chardet" not in p.lockfile["develop"]
+
+        c = p.pipenv("install --dev")
+        c = p.pipenv("run python -c 'import urllib3'")
+        assert c.returncode != 0

tests/unit/test_utils.py

@@ -131,6 +131,20 @@ def test_convert_deps_to_pip(deps, expected):
 def test_convert_deps_to_pip_one_way(deps, expected):
     assert dependencies.convert_deps_to_pip(deps, r=False) == [expected.lower()]
 
+@pytest.mark.utils
+@pytest.mark.parametrize(
+    "deps, expected",
+    [
+        ({"requests": {}}, ["requests"]),
+        ({"FooProject": {"path": ".", "editable" : "true"}}, []),
+        ({"FooProject": {"version": "==1.2"}}, ["fooproject==1.2"]),
+        ({"requests": {"extras": ["security"]}}, []),
+        ({"requests": {"extras": []}}, ["requests"]),
+        ({"extras" : {}}, ["extras"]),
+    ],
+)
+def test_get_constraints_from_deps(deps, expected):
+    assert dependencies.get_constraints_from_deps(deps) == expected
 
 @pytest.mark.parametrize("line,result", [
     ("-i https://example.com/simple/", ("https://example.com/simple/", None, None, [])),