From be046bfbb85b208769d5cf5872ced76743d610b2 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Fri, 2 Jun 2023 00:40:40 -0400 Subject: [PATCH] This pre pip install path should no longer be neccessary when adding packages. (#5700) * This pre pip install path should no longer be neccessary when adding packages. * Fix test -- not sure how this was ever correct. * Address test edge case --- pipenv/project.py | 4 + pipenv/routines/install.py | 103 +++++++----------------- pipenv/utils/resolver.py | 2 +- tests/integration/test_install_basic.py | 2 +- 4 files changed, 36 insertions(+), 75 deletions(-) diff --git a/pipenv/project.py b/pipenv/project.py index 7ff5b7e6..973c4d76 100644 --- a/pipenv/project.py +++ b/pipenv/project.py @@ -974,6 +974,7 @@ class Project: def add_package_to_pipfile(self, package, dev=False, category=None): from .vendor.requirementslib import Requirement + newly_added = False # Read and append Pipfile. p = self.parsed_pipfile # Don't re-capitalize file URLs or VCSs. @@ -989,9 +990,12 @@ class Project: normalized_name = pep423_name(req_name) if name and name != normalized_name: self.remove_package_from_pipfile(name, category=category) + if normalized_name not in p[category]: + newly_added = True p[category][normalized_name] = converted # Write Pipfile. self.write_toml(p) + return newly_added, category def src_name_from_url(self, index_url): name, _, tld_guess = urllib.parse.urlsplit(index_url).netloc.rpartition(".") diff --git a/pipenv/routines/install.py b/pipenv/routines/install.py index f33749bc..3ef79c25 100644 --- a/pipenv/routines/install.py +++ b/pipenv/routines/install.py @@ -13,10 +13,7 @@ from pipenv.utils.dependencies import convert_deps_to_pip, is_star from pipenv.utils.indexes import get_source_list from pipenv.utils.internet import download_file, is_valid_url from pipenv.utils.pip import ( - format_pip_error, - format_pip_output, get_trusted_hosts, - pip_install, pip_install_deps, ) from pipenv.utils.pipfile import ensure_pipfile @@ -190,6 +187,7 @@ def do_install( # Install all dependencies, if none was provided. # This basically ensures that we have a pipfile and lockfile, then it locks and # installs from the lockfile + new_packages = [] if not packages and not editable_packages: # Update project settings with pre preference. if pre: @@ -256,60 +254,7 @@ def do_install( ) ) sys.exit(1) - st.console.print("Installing...") - try: - st.update(f"Installing {pkg_requirement.name}...") - if project.s.is_verbose(): - st.console.print( - f"Installing package: {pkg_requirement.as_line(include_hashes=False)}" - ) - c = pip_install( - project, - pkg_requirement, - ignore_hashes=True, - allow_global=system, - 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, - extra_pip_args=extra_pip_args, - ) - if c.returncode: - err.print( - "{} An error occurred while installing {}!".format( - click.style("Error: ", fg="red", bold=True), - click.style(pkg_line, fg="green"), - ), - ) - err.print(f"Error text: {c.stdout}") - err.print(click.style(format_pip_error(c.stderr), fg="cyan")) - if project.s.is_verbose(): - err.print(click.style(format_pip_output(c.stdout), fg="cyan")) - if "setup.py egg_info" in c.stderr: - err.print( - "This is likely caused by a bug in {}. " - "Report this to its maintainers.".format( - click.style(pkg_requirement.name, fg="green") - ) - ) - err.print( - environments.PIPENV_SPINNER_FAIL_TEXT.format( - "Installation Failed" - ) - ) - sys.exit(1) - except (ValueError, RuntimeError) as e: - err.print("{}: {}".format(click.style("WARNING", fg="red"), e)) - err.print( - environments.PIPENV_SPINNER_FAIL_TEXT.format( - "Installation Failed", - ) - ) - sys.exit(1) + st.update(f"Installing {pkg_requirement.name}...") # Warn if --editable wasn't passed. if ( pkg_requirement.is_vcs @@ -343,9 +288,15 @@ def do_install( try: if categories: for category in categories: - project.add_package_to_pipfile(pkg_requirement, dev, category) + added, cat = project.add_package_to_pipfile( + pkg_requirement, dev, category + ) + if added: + new_packages.append((pkg_requirement.name, cat)) else: - project.add_package_to_pipfile(pkg_requirement, dev) + added, cat = project.add_package_to_pipfile(pkg_requirement, dev) + if added: + new_packages.append((pkg_requirement.name, cat)) except ValueError: import traceback @@ -360,26 +311,32 @@ def do_install( "Failed adding package to Pipfile" ) ) - # ok has a nice v in front, should do something similir with rich + # ok has a nice v in front, should do something similar with rich st.console.print( environments.PIPENV_SPINNER_OK_TEXT.format("Installation Succeeded") ) # Update project settings with pre-release preference. if pre: project.update_settings({"allow_prereleases": pre}) - do_init( - project, - dev=dev, - system=system, - allow_global=system, - keep_outdated=keep_outdated, - requirements_dir=requirements_directory, - deploy=deploy, - pypi_mirror=pypi_mirror, - skip_lock=skip_lock, - extra_pip_args=extra_pip_args, - categories=categories, - ) + try: + do_init( + project, + dev=dev, + system=system, + allow_global=system, + keep_outdated=keep_outdated, + requirements_dir=requirements_directory, + deploy=deploy, + pypi_mirror=pypi_mirror, + skip_lock=skip_lock, + extra_pip_args=extra_pip_args, + categories=categories, + ) + except RuntimeError: + # If we fail to install, remove the package from the Pipfile. + for pkg_name, category in new_packages: + project.remove_package_from_pipfile(pkg_name, category) + sys.exit(1) sys.exit(0) diff --git a/pipenv/utils/resolver.py b/pipenv/utils/resolver.py index 63718a35..5cc689b3 100644 --- a/pipenv/utils/resolver.py +++ b/pipenv/utils/resolver.py @@ -987,7 +987,7 @@ def resolve(cmd, st, project): echo(out.strip(), err=True) if not is_verbose: echo(err, err=True) - sys.exit(returncode) + raise RuntimeError("Failed to lock Pipfile.lock!") if is_verbose: echo(out.strip(), err=True) return subprocess.CompletedProcess(c.args, returncode, out, err) diff --git a/tests/integration/test_install_basic.py b/tests/integration/test_install_basic.py index 521ce89f..afd0bd99 100644 --- a/tests/integration/test_install_basic.py +++ b/tests/integration/test_install_basic.py @@ -482,7 +482,7 @@ def test_install_does_not_exclude_packaging(pipenv_instance_pypi): @pytest.mark.needs_internet def test_install_will_supply_extra_pip_args(pipenv_instance_pypi): with pipenv_instance_pypi(chdir=True) as p: - c = p.pipenv("""install dataclasses-json --extra-pip-args=""--use-feature=truststore --proxy=test""") + c = p.pipenv("""install dataclasses-json --extra-pip-args="--use-feature=truststore --proxy=test" """) assert c.returncode == 1 assert "truststore feature" in c.stderr