From a65d4b28b172c8a9a4ace476af4ae60c1ac2c781 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Fri, 25 Oct 2024 21:27:21 -0400 Subject: [PATCH] solves the locally built git wheel wheres the ref? mystery --- news/6281.bugfix.rst | 44 ++++----- pipenv/cli/command.py | 2 +- pipenv/resolver.py | 3 +- pipenv/routines/install.py | 132 +++++++++++++------------- pipenv/routines/sync.py | 1 - pipenv/routines/update.py | 38 ++++---- pipenv/utils/locking.py | 20 ++-- pipenv/utils/pipfile.py | 4 +- pipenv/utils/project.py | 4 +- pipenv/utils/resolver.py | 51 ++++++---- pyproject.toml | 2 +- tests/integration/test_install_vcs.py | 28 ++++++ tests/integration/test_uninstall.py | 4 +- 13 files changed, 190 insertions(+), 143 deletions(-) diff --git a/news/6281.bugfix.rst b/news/6281.bugfix.rst index 9ad06b5ba2..de5467bc93 100644 --- a/news/6281.bugfix.rst +++ b/news/6281.bugfix.rst @@ -1,26 +1,18 @@ -# This patch series improves Pipenv's reverse dependency handling, JSON output support, and upgrade routines, ensuring more accurate dependency management and better error handling. - -## Key Changes - -* **Reverse Dependency Graph**: - * ``graph --reverse`` now supports JSON output, consistent with ``pipdeptree``. - * Enhanced JSON-tree format for reverse dependencies, improving compatibility with JSON-processing tools. - -* **Improved Upgrade Logic**: - * Pre-sync step added before conflict analysis to ensure accurate dependency resolution. - * Early conflict detection prevents incompatible upgrades, improving lock file integrity. - * Enhanced handling of reverse dependencies to avoid unintended conflicts during updates. - -* **Refactoring & Consistent Output**: - * Replaced ``click.echo`` calls with ``console`` and ``err`` utilities for consistent output and error handling. - * Streamlined upgrade logic to reduce installation phases and improve performance. - -## Bug Fixes - -* Fixed incompatibility when using both ``--json`` and ``--json-tree`` flags simultaneously. -* Addressed #6281: Resolved transitive dependency conflicts (e.g., ``google-api-core`` vs. ``protobuf``). -* Updated tests to cover new JSON outputs and compatibility checks. - -## Impact - -These changes improve accuracy and reliability in complex dependency trees, ensuring robust updates and clearer error reporting. +Fix dependency resolution edge cases and versioning constraints handling: +* Allow JSON format options for ``--reverse`` dependency graph output matching pipdeptree +* Improve installation and upgrade routines to better handle dependencies +* Add ability to specify json output as pipdeptree does +* Add more consistent handling of VCS dependencies and references +* Fix synchronization of development and default dependencies during updates +* Ensure proper propagation of version constraints during updates +* Fix handling of ``~=`` and other version specifiers during updates + +Key Changes: +* Improved reverse dependency analysis to catch conflicts earlier in resolution +* Better handling of VCS package lock data, preserving refs and subdirectories +* Fixed issue where VCS references could be lost in lock file when installed via commit hash +* Better handling of pipfile categories during installation and updates +* Corrected logic for development dependency resolution and constraint propagation +* Improved validation and preservation of version specifiers during updates + +This improves stability when working with complex dependency trees and version constraints. diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 247672d811..29fd85ed65 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -219,7 +219,7 @@ def install(state, **kwargs): editable_packages=state.installstate.editables, site_packages=state.site_packages, extra_pip_args=state.installstate.extra_pip_args, - categories=state.installstate.categories, + pipfile_categories=state.installstate.categories, skip_lock=state.installstate.skip_lock, ) diff --git a/pipenv/resolver.py b/pipenv/resolver.py index c478f868a0..0236bd195e 100644 --- a/pipenv/resolver.py +++ b/pipenv/resolver.py @@ -4,7 +4,6 @@ import os import sys from dataclasses import dataclass, field -from functools import cached_property from pathlib import Path from typing import Any, Dict, List, Optional, Set @@ -228,7 +227,7 @@ def _get_pipfile_content(self) -> Dict[str, Any]: return tomlkit_value_to_python(self.project.parsed_pipfile.get(self.category, {})) - @cached_property + @property def get_cleaned_dict(self) -> Dict[str, Any]: """Create a cleaned dictionary representation of the entry.""" cleaned = { diff --git a/pipenv/routines/install.py b/pipenv/routines/install.py index fcd18b83d3..1b13caf1bb 100644 --- a/pipenv/routines/install.py +++ b/pipenv/routines/install.py @@ -35,7 +35,7 @@ def handle_new_packages( system, pypi_mirror, extra_pip_args, - categories, + pipfile_categories, perform_upgrades=True, index=None, ): @@ -77,8 +77,8 @@ def handle_new_packages( sys.exit(1) try: - if categories: - for category in categories: + if pipfile_categories: + for category in pipfile_categories: added, cat, normalized_name = project.add_package_to_pipfile( pkg_requirement, pkg_line, dev, category ) @@ -120,7 +120,7 @@ def handle_new_packages( pypi_mirror=pypi_mirror, index_url=index, extra_pip_args=extra_pip_args, - categories=categories, + categories=pipfile_categories, ) return new_packages, True except Exception: @@ -144,27 +144,34 @@ def handle_lockfile( categories, ): """Handle the lockfile, updating if necessary. Returns True if package updates were applied.""" - if (project.lockfile_exists and not ignore_pipfile) and not skip_lock: + if ( + (project.lockfile_exists and not ignore_pipfile) + and not skip_lock + and not packages + ): old_hash = project.get_lockfile_hash() new_hash = project.calculate_pipfile_hash() if new_hash != old_hash: - return handle_outdated_lockfile( + if deploy: + console.print( + f"Your Pipfile.lock ({old_hash}) is out of date. Expected: ({new_hash}).", + style="red", + ) + raise exceptions.DeployException + handle_outdated_lockfile( project, packages, old_hash=old_hash, new_hash=new_hash, system=system, allow_global=allow_global, - deploy=deploy, + skip_lock=skip_lock, pre=pre, pypi_mirror=pypi_mirror, categories=categories, ) elif not project.lockfile_exists and not skip_lock: - handle_missing_lockfile( - project, system, allow_global, pre, pypi_mirror, categories - ) - return False + handle_missing_lockfile(project, system, allow_global, pre, pypi_mirror) def handle_outdated_lockfile( @@ -174,20 +181,12 @@ def handle_outdated_lockfile( new_hash, system, allow_global, - deploy, + skip_lock, pre, pypi_mirror, categories, ): """Handle an outdated lockfile returning True if package updates were applied.""" - from pipenv.routines.update import do_update - - if deploy: - console.print( - f"Your Pipfile.lock ({old_hash}) is out of date. Expected: ({new_hash}).", - style="red", - ) - raise exceptions.DeployException if (system or allow_global) and not (project.s.PIPENV_VIRTUALENV): err.print( f"Pipfile.lock ({old_hash}) out of date, but installation uses --system so" @@ -206,19 +205,18 @@ def handle_outdated_lockfile( msg.format(old_hash, new_hash), style="bold yellow", ) - do_update( - project, - packages=packages, - pre=pre, - system=system, - pypi_mirror=pypi_mirror, - categories=categories, - ) - return True - return False + if not skip_lock: + do_lock( + project, + system=system, + pre=pre, + write=True, + pypi_mirror=pypi_mirror, + categories=None, + ) -def handle_missing_lockfile(project, system, allow_global, pre, pypi_mirror, categories): +def handle_missing_lockfile(project, system, allow_global, pre, pypi_mirror): if (system or allow_global) and not project.s.PIPENV_VIRTUALENV: raise exceptions.PipenvOptionsError( "--system", @@ -237,7 +235,6 @@ def handle_missing_lockfile(project, system, allow_global, pre, pypi_mirror, cat pre=pre, write=True, pypi_mirror=pypi_mirror, - categories=categories, ) @@ -256,7 +253,7 @@ def do_install( deploy=False, site_packages=None, extra_pip_args=None, - categories=None, + pipfile_categories=None, skip_lock=False, ): requirements_directory = fileutils.create_tracked_tempdir( @@ -266,6 +263,11 @@ def do_install( packages = packages if packages else [] editable_packages = editable_packages if editable_packages else [] package_args = [p for p in packages if p] + [p for p in editable_packages if p] + new_packages = [] + if dev and not pipfile_categories: + pipfile_categories = ["dev-packages"] + elif not pipfile_categories: + pipfile_categories = ["packages"] ensure_project( project, @@ -276,51 +278,51 @@ def do_install( skip_requirements=False, pypi_mirror=pypi_mirror, site_packages=site_packages, - categories=categories, - ) - - do_install_validations( - project, - package_args, - requirements_directory, - dev=dev, - system=system, - ignore_pipfile=ignore_pipfile, - requirementstxt=requirementstxt, - pre=pre, - deploy=deploy, - categories=categories, - skip_lock=skip_lock, + pipfile_categories=pipfile_categories, ) - packages_installed = do_init( + do_init( project, package_args, system=system, allow_global=system, deploy=deploy, pypi_mirror=pypi_mirror, - categories=categories, skip_lock=skip_lock, + categories=pipfile_categories, ) - new_packages, _packages_installed = handle_new_packages( + do_install_validations( project, - packages, - editable_packages, + package_args, + requirements_directory, dev=dev, - pre=pre, system=system, - pypi_mirror=pypi_mirror, - extra_pip_args=extra_pip_args, - categories=categories, - perform_upgrades=not (packages_installed or skip_lock), - index=index, + ignore_pipfile=ignore_pipfile, + requirementstxt=requirementstxt, + pre=pre, + deploy=deploy, + categories=pipfile_categories, + skip_lock=skip_lock, ) - if packages_installed or _packages_installed: - sys.exit(0) + if not deploy: + new_packages, _ = handle_new_packages( + project, + packages, + editable_packages, + dev=dev, + pre=pre, + system=system, + pypi_mirror=pypi_mirror, + extra_pip_args=extra_pip_args, + pipfile_categories=pipfile_categories, + perform_upgrades=not skip_lock, + index=index, + ) try: + if dev: # Install both develop and default package categories from Pipfile. + pipfile_categories = ["default", "develop"] do_install_dependencies( project, dev=dev, @@ -328,7 +330,7 @@ def do_install( requirements_dir=requirements_directory, pypi_mirror=pypi_mirror, extra_pip_args=extra_pip_args, - categories=categories, + categories=pipfile_categories, skip_lock=skip_lock, ) except Exception as e: @@ -680,8 +682,8 @@ def do_init( deploy=False, pre=False, pypi_mirror=None, - categories=None, skip_lock=False, + categories=None, ): """Initialize the project, ensuring that the Pipfile and Pipfile.lock are in place. Returns True if packages were updated + installed. @@ -689,7 +691,7 @@ def do_init( if not deploy: ensure_pipfile(project, system=system) - packages_updated = handle_lockfile( + handle_lockfile( project, packages, ignore_pipfile=ignore_pipfile, @@ -707,5 +709,3 @@ def do_init( "To activate this project's virtualenv, run [yellow]pipenv shell[/yellow].\n" "Alternatively, run a command inside the virtualenv with [yellow]pipenv run[/yellow]." ) - - return packages_updated diff --git a/pipenv/routines/sync.py b/pipenv/routines/sync.py index 84c66f0576..80e0b1ad26 100644 --- a/pipenv/routines/sync.py +++ b/pipenv/routines/sync.py @@ -50,7 +50,6 @@ def do_sync( pypi_mirror=pypi_mirror, deploy=deploy, system=system, - categories=categories, ) do_install_dependencies( project, diff --git a/pipenv/routines/update.py b/pipenv/routines/update.py index df031ec8cf..e15f6a5147 100644 --- a/pipenv/routines/update.py +++ b/pipenv/routines/update.py @@ -7,7 +7,7 @@ from pipenv.exceptions import JSONParseError, PipenvCmdError from pipenv.patched.pip._vendor.packaging.specifiers import SpecifierSet -from pipenv.patched.pip._vendor.packaging.version import Version +from pipenv.patched.pip._vendor.packaging.version import InvalidVersion, Version from pipenv.routines.outdated import do_outdated from pipenv.routines.sync import do_sync from pipenv.utils import err @@ -153,7 +153,10 @@ def check_version_conflicts( Returns set of conflicting packages. """ conflicts = set() - new_version_obj = Version(new_version) + try: + new_version_obj = Version(new_version) + except InvalidVersion: + new_version_obj = SpecifierSet(new_version) for dependent, req_version in reverse_deps.get(package_name, set()): if req_version == "Any": @@ -187,10 +190,20 @@ def upgrade( lockfile = project.lockfile() if not pre: pre = project.settings.get("allow_prereleases") - if dev or "dev-packages" in categories: - categories = ["develop"] - elif not categories or "packages" in categories: - categories = ["default"] + if not categories: + + if dev and not packages: + categories = ["default", "develop"] + elif dev and packages: + categories = ["develop"] + else: + categories = ["default"] + elif "dev-packages" in categories: + categories.remove("dev-packages") + categories.insert(0, "develop") + elif "packages" in categories: + categories.remove("packages") + categories.insert(0, "default") # Get current dependency graph try: @@ -282,7 +295,7 @@ def upgrade( if not package_args: err.print("Nothing to upgrade!") - sys.exit(0) + return else: err.print( f"[bold][green]Upgrading[/bold][/green] {', '.join(package_args)} in [{category}] dependencies." @@ -301,7 +314,7 @@ def upgrade( ) if not upgrade_lock_data: err.print("Nothing to upgrade!") - sys.exit(0) + return complete_packages = project.parsed_pipfile.get(pipfile_category, {}) @@ -323,15 +336,6 @@ def upgrade( if not version: # Either vcs or file package continue - conflicts = check_version_conflicts( - package_name, version, reverse_deps, lockfile - ) - if conflicts: - err.print( - f"[red bold]Error[/red bold]: Resolution introduced conflicts for {package_name} {version} " - f"with: {', '.join(sorted(conflicts))}" - ) - sys.exit(1) # Update lockfile with verified resolution data for package_name in upgrade_lock_data: diff --git a/pipenv/utils/locking.py b/pipenv/utils/locking.py index 1dfdb432aa..413e2fc86c 100644 --- a/pipenv/utils/locking.py +++ b/pipenv/utils/locking.py @@ -10,6 +10,7 @@ from typing import Any, Dict, Iterator, List, Optional, Set, Tuple from pipenv.patched.pip._internal.req.req_install import InstallRequirement +from pipenv.utils.constants import VCS_LIST from pipenv.utils.dependencies import ( clean_resolved_dep, determine_vcs_revision_hash, @@ -54,36 +55,41 @@ def format_requirement_for_lockfile( entry: Dict[str, Any] = {"name": name} pipfile_entry = pipfile_entries.get(name, pipfile_entries.get(req.name, {})) # Handle VCS requirements + is_vcs_dep = next(iter([vcs for vcs in VCS_LIST if vcs in pipfile_entry]), None) if req.link and req.link.is_vcs: - vcs = req.link.scheme.split("+", 1)[0] + is_vcs_dep = True + if is_vcs_dep: + if req.link and req.link.is_vcs: + link = req.link + else: + link = req.cached_wheel_source_link + vcs = link.scheme.split("+", 1)[0] # Get VCS URL from original deps or normalize the link URL if name in original_deps: entry[vcs] = original_deps[name] else: - vcs_url, _ = normalize_vcs_url(req.link.url) + vcs_url, _ = normalize_vcs_url(link.url) entry[vcs] = vcs_url # Handle subdirectory information if pipfile_entry.get("subdirectory"): entry["subdirectory"] = pipfile_entry["subdirectory"] - elif req.link.subdirectory_fragment: - entry["subdirectory"] = req.link.subdirectory_fragment + elif link.subdirectory_fragment: + entry["subdirectory"] = link.subdirectory_fragment # Handle reference information - try multiple sources ref = determine_vcs_revision_hash(req, vcs, pipfile_entry.get("ref")) if ref: entry["ref"] = ref - # Handle non-VCS requirements else: if req.req and req.req.specifier: entry["version"] = str(req.req.specifier) elif req.specifier: entry["version"] = str(req.specifier) - elif req.link and req.link.is_file: + if req.link and req.link.is_file: entry["file"] = req.link.url - # Add index information if name in index_lookup: entry["index"] = index_lookup[name] diff --git a/pipenv/utils/pipfile.py b/pipenv/utils/pipfile.py index 93eb0f69cc..78e43c55ce 100644 --- a/pipenv/utils/pipfile.py +++ b/pipenv/utils/pipfile.py @@ -64,7 +64,7 @@ def find_pipfile(max_depth=3): def ensure_pipfile( - project, validate=True, skip_requirements=False, system=False, categories=None + project, validate=True, skip_requirements=False, system=False, pipfile_categories=None ): """Creates a Pipfile for the project, if it doesn't exist.""" @@ -96,7 +96,7 @@ def ensure_pipfile( ) as st: # Import requirements.txt. try: - import_requirements(project, categories=categories) + import_requirements(project, categories=pipfile_categories) except Exception: err.print(environments.PIPENV_SPINNER_FAIL_TEXT.format("Failed...")) else: diff --git a/pipenv/utils/project.py b/pipenv/utils/project.py index dffc9e2cbf..b6e58119fd 100644 --- a/pipenv/utils/project.py +++ b/pipenv/utils/project.py @@ -31,7 +31,7 @@ def ensure_project( skip_requirements=False, pypi_mirror=None, clear=False, - categories=None, + pipfile_categories=None, ): """Ensures both Pipfile and virtualenv exist for the project.""" @@ -91,7 +91,7 @@ def ensure_project( validate=validate, skip_requirements=skip_requirements, system=system, - categories=categories, + pipfile_categories=pipfile_categories, ) os.environ["PIP_PYTHON_PATH"] = project.python(system=system) diff --git a/pipenv/utils/resolver.py b/pipenv/utils/resolver.py index d26ade24fc..21aaa617fe 100644 --- a/pipenv/utils/resolver.py +++ b/pipenv/utils/resolver.py @@ -340,17 +340,6 @@ def finder(self, ignore_compatibility=False): finder._ignore_compatibility = ignore_compatibility return finder - @property - def parsed_constraints(self): - pip_options = self.pip_options - pip_options.extra_index_urls = [] - return parse_requirements( - self.prepare_constraint_file(), - finder=self.finder(), - session=self.session, - options=pip_options, - ) - @property def parsed_default_constraints(self): pip_options = self.pip_options @@ -364,8 +353,34 @@ def parsed_default_constraints(self): ) return set(parsed_default_constraints) + @property + def parsed_constraints(self): + """Get parsed constraints including those from default packages if needed.""" + pip_options = self.pip_options + pip_options.extra_index_urls = [] + constraints = list( + parse_requirements( + self.prepare_constraint_file(), + finder=self.finder(), + session=self.session, + options=pip_options, + ) + ) + + # Only add default constraints for dev packages if setting allows + if self.category != "default" and self.project.settings.get( + "use_default_constraints", True + ): + constraints.extend(self.parsed_default_constraints) + + return constraints + @property def default_constraints(self): + """Get constraints from default section when installing dev packages.""" + if not self.project.settings.get("use_default_constraints", True): + return set() + possible_default_constraints = [ install_req_from_parsed_requirement( c, @@ -391,14 +406,19 @@ def possible_constraints(self): @property def constraints(self): + """Get all applicable constraints.""" possible_constraints_list = self.possible_constraints constraints_list = set() for c in possible_constraints_list: constraints_list.add(c) - # Only use default_constraints when installing dev-packages - if self.category != "packages": + + # Only use default_constraints when installing dev-packages and setting allows + if self.category != "default" and self.project.settings.get( + "use_default_constraints", True + ): constraints_list |= self.default_constraints - return set(constraints_list) + + return constraints_list @contextlib.contextmanager def get_resolver(self, clear=False): @@ -433,10 +453,9 @@ def get_resolver(self, clear=False): yield resolver def resolve(self): - constraints = self.constraints with temp_environ(), self.get_resolver() as resolver: try: - results = resolver.resolve(constraints, check_supported_wheels=False) + results = resolver.resolve(self.constraints, check_supported_wheels=False) except InstallationError as e: raise ResolutionFailure(message=str(e)) else: diff --git a/pyproject.toml b/pyproject.toml index 325381e01b..f93da5ed6e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -198,7 +198,7 @@ keep_full_version = true max_supported_python = "3.13" [tool.pytest.ini_options] -addopts = "-ra" +addopts = "-ra --no-cov" plugins = "xdist" testpaths = [ "tests" ] # Add vendor and patched in addition to the default list of ignored dirs diff --git a/tests/integration/test_install_vcs.py b/tests/integration/test_install_vcs.py index 390e1782b4..f1ddf7487f 100644 --- a/tests/integration/test_install_vcs.py +++ b/tests/integration/test_install_vcs.py @@ -1,4 +1,5 @@ import os +from pathlib import Path import pytest @@ -43,3 +44,30 @@ def test_install_github_vcs_with_credentials(pipenv_instance_pypi, use_credentia # Verify that the package is installed and usable c = p.pipenv("run python -c \"import dataclass_factory\"") assert c.returncode == 0, f"Failed to import library: {c.stderr}" + + +@pytest.mark.vcs +@pytest.mark.urls +@pytest.mark.install +@pytest.mark.needs_internet +def test_install_vcs_ref_by_commit_hash(pipenv_instance_private_pypi): + with pipenv_instance_private_pypi() as p: + c = p.pipenv("install -e git+https://github.com/benjaminp/six.git@5efb522b0647f7467248273ec1b893d06b984a59#egg=six") + assert c.returncode == 0 + assert "six" in p.pipfile["packages"] + assert "six" in p.lockfile["default"] + assert ( + p.lockfile["default"]["six"]["ref"] + == "5efb522b0647f7467248273ec1b893d06b984a59" + ) + pipfile = Path(p.pipfile_path) + new_content = pipfile.read_text().replace("5efb522b0647f7467248273ec1b893d06b984a59", "15e31431af97e5e64b80af0a3f598d382bcdd49a") + pipfile.write_text(new_content) + c = p.pipenv("lock") + assert c.returncode == 0 + assert ( + p.lockfile["default"]["six"]["ref"] + == "15e31431af97e5e64b80af0a3f598d382bcdd49a" + ) + assert "six" in p.pipfile["packages"] + assert "six" in p.lockfile["default"] diff --git a/tests/integration/test_uninstall.py b/tests/integration/test_uninstall.py index 5f998e8688..5529179aec 100644 --- a/tests/integration/test_uninstall.py +++ b/tests/integration/test_uninstall.py @@ -123,7 +123,7 @@ def test_uninstall_all_dev(pipenv_instance_private_pypi): """ f.write(contents) - c = p.pipenv("install --dev") + c = p.pipenv("install -v --dev") assert c.returncode == 0 assert "tablib" in p.pipfile["packages"] @@ -134,7 +134,7 @@ def test_uninstall_all_dev(pipenv_instance_private_pypi): assert "six" in p.lockfile["develop"] assert c.returncode == 0 - c = p.pipenv("uninstall --all-dev") + c = p.pipenv("uninstall -v --all-dev") assert c.returncode == 0 assert p.pipfile["dev-packages"] == {} assert "jinja2" not in p.lockfile["develop"]