diff --git a/news/6852.bugfix b/news/6852.bugfix new file mode 100644 index 00000000000..f9ae543ada9 --- /dev/null +++ b/news/6852.bugfix @@ -0,0 +1,2 @@ +Cache wheels that ``pip wheel`` built locally after downloading +them as sdist. This matches what ``pip install`` does. diff --git a/src/pip/_internal/wheel.py b/src/pip/_internal/wheel.py index bc0cdd260b1..e698cb371fe 100644 --- a/src/pip/_internal/wheel.py +++ b/src/pip/_internal/wheel.py @@ -773,41 +773,37 @@ def _contains_egg_info( return bool(_egg_info_re.search(s)) -def should_use_ephemeral_cache( +def should_build( req, # type: InstallRequirement + need_wheel, # type: bool format_control, # type: FormatControl - should_unpack, # type: bool - cache_available # type: bool ): - # type: (...) -> Optional[bool] """ - Return whether to build an InstallRequirement object using the - ephemeral cache. - - :param cache_available: whether a cache directory is available for the - should_unpack=True case. - - :return: True or False to build the requirement with ephem_cache=True - or False, respectively; or None not to build the requirement. + Return whether a wheel should be built based on the requirement + characteristics, and if a wheel is ultimately needed or not. """ if req.constraint: # never build requirements that are merely constraints - return None + return False + if req.is_wheel: - if not should_unpack: + if need_wheel: logger.info( 'Skipping %s, due to already being wheel.', req.name, ) - return None - if not should_unpack: - # i.e. pip wheel, not pip install; - # return False, knowing that the caller will never cache - # in this case anyway, so this return merely means "build it". - # TODO improve this behavior + # never build a requirement that is already a wheel return False - if req.editable or not req.source_dir: - return None + if need_wheel: + return True + + if req.editable: + # don't build an editable if no wheel is needed + return False + + if not req.source_dir: + # TODO explain what no source_dir means + return False if "binary" not in format_control.get_allowed_formats( canonicalize_name(req.name)): @@ -815,21 +811,49 @@ def should_use_ephemeral_cache( "Skipping bdist_wheel for %s, due to binaries " "being disabled for it.", req.name, ) - return None + return False + + return True + + +def should_cache( + req, # type: InstallRequirement + format_control, # type: FormatControl +): + # type: (...) -> Optional[bool] + """ + Return whether to build an InstallRequirement object using the + wheel cache, assuming the wheel cache is available. + """ + if req.editable: + # don't cache editable requirements because they can + # be locally modified + return False + + if not req.source_dir: + # TODO explain what no source_dir means + return False + + if "binary" not in format_control.get_allowed_formats( + canonicalize_name(req.name)): + # TODO explain this + return False if req.link and req.link.is_vcs: - # VCS checkout. Build wheel just for this run. - return True + # VCS checkout. Build wheel just for this run + # unless it points to an immutable commit hash in which + # case it can be cached. + # TODO cache against the commit hash if we can. + return False link = req.link base, ext = link.splitext() - if cache_available and _contains_egg_info(base): - return False + if _contains_egg_info(base): + return True # Otherwise, build the wheel just for this run using the ephemeral - # cache since we are either in the case of e.g. a local directory, or - # no cache directory is available to use. - return True + # cache since we are either in the case of e.g. a local directory. + return False def format_command_result( @@ -1062,22 +1086,23 @@ def build( cache_available = bool(self.wheel_cache.cache_dir) for req in requirements: - ephem_cache = should_use_ephemeral_cache( + if not should_build( req, + need_wheel=not should_unpack, format_control=format_control, - should_unpack=should_unpack, - cache_available=cache_available, - ) - if ephem_cache is None: + ): continue - - buildset.append((req, ephem_cache)) + use_ephemeral_cache = ( + not cache_available or + not should_cache(req, format_control=format_control) + ) + buildset.append((req, use_ephemeral_cache)) if not buildset: return [] # Is any wheel build not using the ephemeral cache? - if any(not ephem_cache for _, ephem_cache in buildset): + if any(not use_ephemeral_cache for _, use_ephemeral_cache in buildset): have_directory_for_build = self._wheel_dir or ( should_unpack and self.wheel_cache.cache_dir ) @@ -1094,23 +1119,21 @@ def build( _cache = self.wheel_cache # shorter name with indent_log(): build_success, build_failure = [], [] - for req, ephem in buildset: + for req, use_ephemeral_cache in buildset: python_tag = None if should_unpack: python_tag = pep425tags.implementation_tag - if ephem: - output_dir = _cache.get_ephem_path_for_link(req.link) - else: - output_dir = _cache.get_path_for_link(req.link) - try: - ensure_dir(output_dir) - except OSError as e: - logger.warning("Building wheel for %s failed: %s", - req.name, e) - build_failure.append(req) - continue + if use_ephemeral_cache: + output_dir = _cache.get_ephem_path_for_link(req.link) else: - output_dir = self._wheel_dir + output_dir = _cache.get_path_for_link(req.link) + try: + ensure_dir(output_dir) + except OSError as e: + logger.warning("Building wheel for %s failed: %s", + req.name, e) + build_failure.append(req) + continue wheel_file = self._build_one( req, output_dir, python_tag=python_tag, @@ -1139,6 +1162,9 @@ def build( assert req.link.is_wheel # extract the wheel into the dir unpack_file_url(link=req.link, location=req.source_dir) + else: + shutil.copy(wheel_file, self._wheel_dir) + logger.info('Stored in directory: %s', self._wheel_dir) else: build_failure.append(req) diff --git a/tests/functional/test_wheel.py b/tests/functional/test_wheel.py index 5ebc9ea4c21..00959f65dd6 100644 --- a/tests/functional/test_wheel.py +++ b/tests/functional/test_wheel.py @@ -61,6 +61,30 @@ def test_pip_wheel_success(script, data): assert "Successfully built simple" in result.stdout, result.stdout +def test_pip_wheel_build_cache(script, data): + """ + Test 'pip wheel' builds and caches. + """ + result = script.pip( + 'wheel', '--no-index', '-f', data.find_links, + 'simple==3.0', + ) + wheel_file_name = 'simple-3.0-py%s-none-any.whl' % pyversion[0] + wheel_file_path = script.scratch / wheel_file_name + assert wheel_file_path in result.files_created, result.stdout + assert "Successfully built simple" in result.stdout, result.stdout + # remove target file + (script.scratch_path / wheel_file_name).unlink() + # pip wheel again and test that no build occurs since + # we get the wheel from cache + result = script.pip( + 'wheel', '--no-index', '-f', data.find_links, + 'simple==3.0', + ) + assert wheel_file_path in result.files_created, result.stdout + assert "Successfully built simple" not in result.stdout, result.stdout + + def test_basic_pip_wheel_downloads_wheels(script, data): """ Test 'pip wheel' downloads wheels diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index 449da28c199..c3accdf4e57 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -38,6 +38,25 @@ def test_contains_egg_info(s, expected): assert result == expected +class ReqMock: + + def __init__( + self, + name="pendulum", + is_wheel=False, + editable=False, + link=None, + constraint=False, + source_dir="/tmp/pip-install-123/pendulum", + ): + self.name = name + self.is_wheel = is_wheel + self.editable = editable + self.link = link + self.constraint = constraint + self.source_dir = source_dir + + def make_test_install_req(base_name=None): """ Return an InstallRequirement object for testing purposes. @@ -78,76 +97,61 @@ def test_format_tag(file_tag, expected): @pytest.mark.parametrize( - "base_name, should_unpack, cache_available, expected", + "req, need_wheel, disallow_binaries, expected", [ - ('pendulum-2.0.4', False, False, False), - # The following cases test should_unpack=True. - # Test _contains_egg_info() returning True. - ('pendulum-2.0.4', True, True, False), - ('pendulum-2.0.4', True, False, True), - # Test _contains_egg_info() returning False. - ('pendulum', True, True, True), - ('pendulum', True, False, True), + # pip wheel (need_wheel=True) + (ReqMock(), True, False, True), + (ReqMock(), True, True, True), + (ReqMock(constraint=True), True, False, False), + (ReqMock(is_wheel=True), True, False, False), + (ReqMock(editable=True), True, False, True), + (ReqMock(source_dir=None), True, False, True), + (ReqMock(link=Link("git+https://g.c/org/repo")), True, False, True), + (ReqMock(link=Link("git+https://g.c/org/repo")), True, True, True), + # pip install (need_wheel=False) + (ReqMock(), False, False, True), + (ReqMock(), False, True, False), + (ReqMock(constraint=True), False, False, False), + (ReqMock(is_wheel=True), False, False, False), + (ReqMock(editable=True), False, False, False), + (ReqMock(source_dir=None), False, False, False), + # By default (i.e. when binaries are allowed), VCS requirements + # should be built in install mode. + (ReqMock(link=Link("git+https://g.c/org/repo")), False, False, True), + # Disallowing binaries, however, should cause them not to be built. + (ReqMock(link=Link("git+https://g.c/org/repo")), False, True, False), ], ) -def test_should_use_ephemeral_cache__issue_6197( - base_name, should_unpack, cache_available, expected, -): - """ - Regression test for: https://github.com/pypa/pip/issues/6197 - """ - req = make_test_install_req(base_name=base_name) - assert not req.is_wheel - assert req.link.is_artifact - +def test_should_build(req, need_wheel, disallow_binaries, expected): format_control = FormatControl() - ephem_cache = wheel.should_use_ephemeral_cache( - req, format_control=format_control, should_unpack=should_unpack, - cache_available=cache_available, + if disallow_binaries: + format_control.disallow_binaries() + should_build = wheel.should_build( + req, need_wheel=need_wheel, format_control=format_control ) - assert ephem_cache is expected + assert should_build is expected @pytest.mark.parametrize( - "disallow_binaries, expected", + "req, disallow_binaries, expected", [ - # By default (i.e. when binaries are allowed), VCS requirements - # should be built. - (False, True), - # Disallowing binaries, however, should cause them not to be built. - (True, None), + (ReqMock(editable=True), False, False), + (ReqMock(source_dir=None), False, False), + (ReqMock(link=Link("git+https://g.c/org/repo")), False, False), + (ReqMock(link=Link("https://g.c/dist.tgz")), False, False), + (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True), ], ) -def test_should_use_ephemeral_cache__disallow_binaries_and_vcs_checkout( - disallow_binaries, expected, +def test_should_cache( + req, disallow_binaries, expected ): - """ - Test that disallowing binaries (e.g. from passing --global-option) - causes should_use_ephemeral_cache() to return None for VCS checkouts. - """ - req = Requirement('pendulum') - link = Link(url='git+https://git.example.com/pendulum.git') - req = InstallRequirement( - req=req, - comes_from=None, - constraint=False, - editable=False, - link=link, - source_dir='/tmp/pip-install-9py5m2z1/pendulum', - ) - assert not req.is_wheel - assert req.link.is_vcs - format_control = FormatControl() if disallow_binaries: format_control.disallow_binaries() - - # The cache_available value doesn't matter for this test. - ephem_cache = wheel.should_use_ephemeral_cache( - req, format_control=format_control, should_unpack=True, - cache_available=True, + should_cache = wheel.should_cache( + req, format_control=format_control, ) - assert ephem_cache is expected + assert should_cache is expected def test_format_command_result__INFO(caplog):