From 578de7d863d22c769aba98a6d7e07c1c78f35546 Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Fri, 11 Oct 2019 21:31:35 -0400 Subject: [PATCH 1/4] Rename wheel install function --- src/pip/_internal/req/req_install.py | 2 +- src/pip/_internal/wheel.py | 2 +- tests/unit/test_wheel.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 5f1fdb74b1e..3603f0666d1 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -484,7 +484,7 @@ def move_wheel_files( pycompile=True # type: bool ): # type: (...) -> None - wheel.move_wheel_files( + wheel.install_unpacked_wheel( self.name, self.req, wheeldir, user=use_user_site, home=home, diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 8f9778c7d29..b8f8721a537 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -330,7 +330,7 @@ def make(self, specification, options=None): return super(PipScriptMaker, self).make(specification, options) -def move_wheel_files( +def install_unpacked_wheel( name, # type: str req, # type: Requirement wheeldir, # type: str diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 2a824c7fd7b..b78cd280fc7 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -619,7 +619,7 @@ def test_version_underscore_conversion(self): assert w.version == '0.1-1' -class TestMoveWheelFiles(object): +class TestInstallUnpackedWheel(object): """ Tests for moving files from wheel src to scheme paths """ @@ -659,14 +659,14 @@ def assert_installed(self): def test_std_install(self, data, tmpdir): self.prep(data, tmpdir) - wheel.move_wheel_files( + wheel.install_unpacked_wheel( self.name, self.req, self.src, scheme=self.scheme) self.assert_installed() def test_install_prefix(self, data, tmpdir): prefix = os.path.join(os.path.sep, 'some', 'path') self.prep(data, tmpdir) - wheel.move_wheel_files( + wheel.install_unpacked_wheel( self.name, self.req, self.src, @@ -688,7 +688,7 @@ def test_dist_info_contains_empty_dir(self, data, tmpdir): self.src_dist_info, 'empty_dir', 'empty_dir') os.makedirs(src_empty_dir) assert os.path.isdir(src_empty_dir) - wheel.move_wheel_files( + wheel.install_unpacked_wheel( self.name, self.req, self.src, scheme=self.scheme) self.assert_installed() assert not os.path.isdir( From 4682f3cb9b3d9d1430429f0917ed9da42868f26b Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Fri, 11 Oct 2019 21:49:39 -0400 Subject: [PATCH 2/4] Pass scheme to install_unpacked_wheel This reduces the number of required arguments and helps establish a convention for install_* functions, which should take a scheme instead of the individual components. --- src/pip/_internal/req/req_install.py | 27 ++++++++++++++------------- src/pip/_internal/wheel.py | 15 ++------------- tests/unit/test_wheel.py | 12 ++++++++++-- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 3603f0666d1..39485c09eb7 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -23,6 +23,7 @@ from pip._internal import pep425tags, wheel from pip._internal.build_env import NoOpBuildEnvironment from pip._internal.exceptions import InstallationError +from pip._internal.locations import distutils_scheme from pip._internal.models.link import Link from pip._internal.operations.generate_metadata import get_metadata_generator from pip._internal.pyproject import load_pyproject_toml, make_pyproject_path @@ -60,7 +61,7 @@ if MYPY_CHECK_RUNNING: from typing import ( - Any, Dict, Iterable, List, Optional, Sequence, Union, + Any, Dict, Iterable, List, Mapping, Optional, Sequence, Union, ) from pip._internal.build_env import BuildEnvironment from pip._internal.cache import WheelCache @@ -476,22 +477,17 @@ def is_wheel(self): def move_wheel_files( self, wheeldir, # type: str - root=None, # type: Optional[str] - home=None, # type: Optional[str] - prefix=None, # type: Optional[str] + scheme, # type: Mapping[str, str] warn_script_location=True, # type: bool - use_user_site=False, # type: bool pycompile=True # type: bool ): # type: (...) -> None wheel.install_unpacked_wheel( - self.name, self.req, wheeldir, - user=use_user_site, - home=home, - root=root, - prefix=prefix, + self.name, + self.req, + wheeldir, + scheme=scheme, pycompile=pycompile, - isolated=self.isolated, warn_script_location=warn_script_location, ) @@ -859,10 +855,15 @@ def install( version = wheel.wheel_version(self.source_dir) wheel.check_compatibility(version, self.name) + scheme = distutils_scheme( + self.name, user=use_user_site, home=home, root=root, + isolated=self.isolated, prefix=prefix, + ) self.move_wheel_files( - self.source_dir, root=root, prefix=prefix, home=home, + self.source_dir, + scheme=scheme, warn_script_location=warn_script_location, - use_user_site=use_user_site, pycompile=pycompile, + pycompile=pycompile, ) self.install_succeeded = True return diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index b8f8721a537..342e111f29e 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -34,7 +34,7 @@ InvalidWheelFilename, UnsupportedWheel, ) -from pip._internal.locations import distutils_scheme, get_major_minor_version +from pip._internal.locations import get_major_minor_version from pip._internal.models.link import Link from pip._internal.utils.logging import indent_log from pip._internal.utils.marker_files import has_delete_marker_file @@ -334,13 +334,8 @@ def install_unpacked_wheel( name, # type: str req, # type: Requirement wheeldir, # type: str - user=False, # type: bool - home=None, # type: Optional[str] - root=None, # type: Optional[str] + scheme, # type: Mapping[str, str] pycompile=True, # type: bool - scheme=None, # type: Optional[Mapping[str, str]] - isolated=False, # type: bool - prefix=None, # type: Optional[str] warn_script_location=True # type: bool ): # type: (...) -> None @@ -349,12 +344,6 @@ def install_unpacked_wheel( # TODO: Look into moving this into a dedicated class for representing an # installation. - if not scheme: - scheme = distutils_scheme( - name, user=user, home=home, root=root, isolated=isolated, - prefix=prefix, - ) - if root_is_purelib(name, wheeldir): lib_dir = scheme['purelib'] else: diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index b78cd280fc7..946e2b4f7f1 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -10,6 +10,7 @@ from pip._internal import pep425tags, wheel from pip._internal.exceptions import InvalidWheelFilename, UnsupportedWheel +from pip._internal.locations import distutils_scheme from pip._internal.models.link import Link from pip._internal.req.req_install import InstallRequirement from pip._internal.utils.compat import WINDOWS @@ -666,12 +667,19 @@ def test_std_install(self, data, tmpdir): def test_install_prefix(self, data, tmpdir): prefix = os.path.join(os.path.sep, 'some', 'path') self.prep(data, tmpdir) + scheme = distutils_scheme( + self.name, + user=False, + home=None, + root=tmpdir, + isolated=False, + prefix=prefix, + ) wheel.install_unpacked_wheel( self.name, self.req, self.src, - root=tmpdir, - prefix=prefix, + scheme=scheme, ) bin_dir = 'Scripts' if WINDOWS else 'bin' From 39572ddd126ccfae546231c9d1d2dfcdcea73fab Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Fri, 11 Oct 2019 21:58:58 -0400 Subject: [PATCH 3/4] Don't pass InstallRequirement to install_unpacked_wheel We are only using this value for logging. Passing a string reduces coupling between InstallRequirement and this function. --- src/pip/_internal/req/req_install.py | 2 +- src/pip/_internal/wheel.py | 4 ++-- tests/unit/test_wheel.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 39485c09eb7..b23f447b5d0 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -484,7 +484,7 @@ def move_wheel_files( # type: (...) -> None wheel.install_unpacked_wheel( self.name, - self.req, + str(self.req), wheeldir, scheme=scheme, pycompile=pycompile, diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 342e111f29e..28bcd14c5c8 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -332,7 +332,7 @@ def make(self, specification, options=None): def install_unpacked_wheel( name, # type: str - req, # type: Requirement + req, # type: str wheeldir, # type: str scheme, # type: Mapping[str, str] pycompile=True, # type: bool @@ -393,7 +393,7 @@ def clobber(source, dest, is_base, fixer=None, filter=None): elif (is_base and s.endswith('.dist-info') and canonicalize_name(s).startswith( - canonicalize_name(req.name))): + canonicalize_name(name))): assert not info_dir, ('Multiple .dist-info directories: ' + destsubdir + ', ' + ', '.join(info_dir)) diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 946e2b4f7f1..fbb492d70af 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -661,7 +661,7 @@ def assert_installed(self): def test_std_install(self, data, tmpdir): self.prep(data, tmpdir) wheel.install_unpacked_wheel( - self.name, self.req, self.src, scheme=self.scheme) + self.name, str(self.req), self.src, scheme=self.scheme) self.assert_installed() def test_install_prefix(self, data, tmpdir): @@ -677,7 +677,7 @@ def test_install_prefix(self, data, tmpdir): ) wheel.install_unpacked_wheel( self.name, - self.req, + str(self.req), self.src, scheme=scheme, ) @@ -697,7 +697,7 @@ def test_dist_info_contains_empty_dir(self, data, tmpdir): os.makedirs(src_empty_dir) assert os.path.isdir(src_empty_dir) wheel.install_unpacked_wheel( - self.name, self.req, self.src, scheme=self.scheme) + self.name, str(self.req), self.src, scheme=self.scheme) self.assert_installed() assert not os.path.isdir( os.path.join(self.dest_dist_info, 'empty_dir')) From 913f85673929612d3f9c6fdfcec094297caa65ed Mon Sep 17 00:00:00 2001 From: Chris Hunt Date: Fri, 11 Oct 2019 22:24:50 -0400 Subject: [PATCH 4/4] Cleanup arguments, add docstring --- src/pip/_internal/req/req_install.py | 2 +- src/pip/_internal/wheel.py | 38 +++++++++++++++++++--------- tests/unit/test_wheel.py | 14 +++++++--- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index b23f447b5d0..fd198078c9c 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -484,9 +484,9 @@ def move_wheel_files( # type: (...) -> None wheel.install_unpacked_wheel( self.name, - str(self.req), wheeldir, scheme=scheme, + req_description=str(self.req), pycompile=pycompile, warn_script_location=warn_script_location, ) diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index 28bcd14c5c8..70d52d29d9d 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -57,7 +57,6 @@ Dict, List, Optional, Sequence, Mapping, Tuple, IO, Text, Any, Iterable, Callable, Set, ) - from pip._vendor.packaging.requirements import Requirement from pip._internal.req.req_install import InstallRequirement from pip._internal.operations.prepare import ( RequirementPreparer @@ -332,14 +331,24 @@ def make(self, specification, options=None): def install_unpacked_wheel( name, # type: str - req, # type: str wheeldir, # type: str scheme, # type: Mapping[str, str] + req_description, # type: str pycompile=True, # type: bool warn_script_location=True # type: bool ): # type: (...) -> None - """Install a wheel""" + """Install a wheel. + + :param name: Name of the project to install + :param wheeldir: Base directory of the unpacked wheel + :param scheme: Distutils scheme dictating the install directories + :param req_description: String used in place of the requirement, for + logging + :param pycompile: Whether to byte-compile installed Python files + :param warn_script_location: Whether to check that scripts are installed + into a directory on PATH + """ # TODO: Investigate and break this up. # TODO: Look into moving this into a dedicated class for representing an # installation. @@ -390,13 +399,16 @@ def clobber(source, dest, is_base, fixer=None, filter=None): if is_base and basedir == '' and destsubdir.endswith('.data'): data_dirs.append(s) continue - elif (is_base and - s.endswith('.dist-info') and - canonicalize_name(s).startswith( - canonicalize_name(name))): - assert not info_dir, ('Multiple .dist-info directories: ' + - destsubdir + ', ' + - ', '.join(info_dir)) + elif ( + is_base and + s.endswith('.dist-info') and + canonicalize_name(s).startswith(canonicalize_name(name)) + ): + assert not info_dir, ( + 'Multiple .dist-info directories: {}, '.format( + destsubdir + ) + ', '.join(info_dir) + ) info_dir.append(destsubdir) for f in files: # Skip unwanted files @@ -449,7 +461,9 @@ def clobber(source, dest, is_base, fixer=None, filter=None): clobber(source, lib_dir, True) - assert info_dir, "%s .dist-info directory not found" % req + assert info_dir, "{} .dist-info directory not found".format( + req_description + ) # Get the defined entry points ep_file = os.path.join(info_dir[0], 'entry_points.txt') @@ -592,7 +606,7 @@ def is_entrypoint_wrapper(name): "Invalid script entry point: {} for req: {} - A callable " "suffix is required. Cf https://packaging.python.org/en/" "latest/distributing.html#console-scripts for more " - "information.".format(entry, req) + "information.".format(entry, req_description) ) if warn_script_location: diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index fbb492d70af..cd658cca007 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -661,7 +661,11 @@ def assert_installed(self): def test_std_install(self, data, tmpdir): self.prep(data, tmpdir) wheel.install_unpacked_wheel( - self.name, str(self.req), self.src, scheme=self.scheme) + self.name, + self.src, + scheme=self.scheme, + req_description=str(self.req), + ) self.assert_installed() def test_install_prefix(self, data, tmpdir): @@ -677,9 +681,9 @@ def test_install_prefix(self, data, tmpdir): ) wheel.install_unpacked_wheel( self.name, - str(self.req), self.src, scheme=scheme, + req_description=str(self.req), ) bin_dir = 'Scripts' if WINDOWS else 'bin' @@ -697,7 +701,11 @@ def test_dist_info_contains_empty_dir(self, data, tmpdir): os.makedirs(src_empty_dir) assert os.path.isdir(src_empty_dir) wheel.install_unpacked_wheel( - self.name, str(self.req), self.src, scheme=self.scheme) + self.name, + self.src, + scheme=self.scheme, + req_description=str(self.req), + ) self.assert_installed() assert not os.path.isdir( os.path.join(self.dest_dist_info, 'empty_dir'))