Skip to content

Commit

Permalink
Issue 4371 incorrect dependencies when install dev packages (#5234)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
dqkqd authored Aug 13, 2022
1 parent d8c88ef commit e9dc324
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 31 deletions.
2 changes: 2 additions & 0 deletions news/5234.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Use ``packages`` as contraints when locking ``dev-packages`` in Pipfile.
Use ``packages`` as contraints when installing new ``dev-packages``.
24 changes: 21 additions & 3 deletions pipenv/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
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
Expand All @@ -41,6 +43,7 @@
cmd_list_to_shell,
find_python,
is_python_command,
normalize_path,
project_python,
subprocess_run,
system_which,
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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, {})
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion pipenv/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -769,6 +772,7 @@ def resolve(packages, pre, project, sources, clear, system, requirements_dir=Non
results, resolver = resolve(
packages,
pre=pre,
dev=dev,
project=project,
sources=sources,
clear=clear,
Expand Down
54 changes: 54 additions & 0 deletions pipenv/utils/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
87 changes: 60 additions & 27 deletions pipenv/utils/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
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
Expand All @@ -32,9 +36,11 @@
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
Expand Down Expand Up @@ -117,6 +123,7 @@ def __init__(
skipped=None,
clear=False,
pre=False,
dev=False,
):
self.initial_constraints = constraints
self.req_dir = req_dir
Expand All @@ -126,6 +133,7 @@ def __init__(
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 {}
Expand Down Expand Up @@ -420,6 +428,7 @@ def create(
req_dir: str = None,
clear: bool = False,
pre: bool = False,
dev: bool = False,
) -> "Resolver":

if not req_dir:
Expand Down Expand Up @@ -450,6 +459,7 @@ def create(
skipped=skipped,
clear=clear,
pre=pre,
dev=dev,
)

@classmethod
Expand Down Expand Up @@ -522,36 +532,31 @@ def pip_args(self):
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):
if self._constraint_file is None:
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:
Expand Down Expand Up @@ -630,12 +635,33 @@ def parsed_constraints(self):
)
return self._parsed_constraints

@property
def constraints(self):
from pipenv.patched.pip._internal.req.constructors import (
install_req_from_parsed_requirement,
@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):
if self._constraints is None:
self._constraints = [
install_req_from_parsed_requirement(
Expand All @@ -646,6 +672,9 @@ def constraints(self):
)
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
Expand Down Expand Up @@ -870,6 +899,7 @@ def actually_resolve_deps(
sources,
clear,
pre,
dev,
req_dir=None,
):
if not req_dir:
Expand All @@ -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()
Expand Down Expand Up @@ -1064,6 +1094,7 @@ def resolve_deps(
python=False,
clear=False,
pre=False,
dev=False,
allow_global=False,
req_dir=None,
):
Expand Down Expand Up @@ -1094,6 +1125,7 @@ def resolve_deps(
sources,
clear,
pre,
dev,
req_dir=req_dir,
)
except RuntimeError:
Expand Down Expand Up @@ -1122,6 +1154,7 @@ def resolve_deps(
sources,
clear,
pre,
dev,
req_dir=req_dir,
)
except RuntimeError:
Expand Down
Loading

0 comments on commit e9dc324

Please sign in to comment.