_should_cache does not depend on check_binary_allowed

_should_cache is only called by _get_cache_dir.

In pip install mode, _get_cache_dir is never called when
check_binary_allowed returns False because in that case
should_build_for_install_command has returned False before
and the build was skipped.

In pip wheel mode, check_binary_allowed always returns True
(because it is not passed to the build function).

So _should_cache can use _always_true for check_binary_allowed.

*Alternative*

Alternatively, we could have passed check_binary_allowed
to build in pip wheel mode. The only difference is that wheels built
locally from *legacy* packages would then not be cached,
when pip wheel is used with --no-binary.
This commit is contained in:
Stéphane Bidoul (ACSONE) 2020-01-12 14:20:58 +01:00 committed by Xavier Fernandez
parent c55eee4188
commit 0e1e0ef566
3 changed files with 13 additions and 48 deletions

View File

@ -358,7 +358,6 @@ class InstallCommand(RequirementCommand):
wheel_cache=wheel_cache, wheel_cache=wheel_cache,
build_options=[], build_options=[],
global_options=[], global_options=[],
check_binary_allowed=check_binary_allowed,
) )
# If we're using PEP 517, we cannot do a direct install # If we're using PEP 517, we cannot do a direct install

View File

@ -107,7 +107,6 @@ def should_build_for_install_command(
def _should_cache( def _should_cache(
req, # type: InstallRequirement req, # type: InstallRequirement
check_binary_allowed, # type: BinaryAllowedPredicate
): ):
# type: (...) -> Optional[bool] # type: (...) -> Optional[bool]
""" """
@ -116,7 +115,7 @@ def _should_cache(
has determined a wheel needs to be built. has determined a wheel needs to be built.
""" """
if not should_build_for_install_command( if not should_build_for_install_command(
req, check_binary_allowed=check_binary_allowed req, check_binary_allowed=_always_true
): ):
# never cache if pip install would not have built # never cache if pip install would not have built
# (editable mode, etc) # (editable mode, etc)
@ -144,17 +143,13 @@ def _should_cache(
def _get_cache_dir( def _get_cache_dir(
req, # type: InstallRequirement req, # type: InstallRequirement
wheel_cache, # type: WheelCache wheel_cache, # type: WheelCache
check_binary_allowed, # type: BinaryAllowedPredicate
): ):
# type: (...) -> str # type: (...) -> str
"""Return the persistent or temporary cache directory where the built """Return the persistent or temporary cache directory where the built
wheel need to be stored. wheel need to be stored.
""" """
cache_available = bool(wheel_cache.cache_dir) cache_available = bool(wheel_cache.cache_dir)
if ( if cache_available and _should_cache(req):
cache_available and
_should_cache(req, check_binary_allowed)
):
cache_dir = wheel_cache.get_path_for_link(req.link) cache_dir = wheel_cache.get_path_for_link(req.link)
else: else:
cache_dir = wheel_cache.get_ephem_path_for_link(req.link) cache_dir = wheel_cache.get_ephem_path_for_link(req.link)
@ -263,7 +258,6 @@ def build(
wheel_cache, # type: WheelCache wheel_cache, # type: WheelCache
build_options, # type: List[str] build_options, # type: List[str]
global_options, # type: List[str] global_options, # type: List[str]
check_binary_allowed=None, # type: Optional[BinaryAllowedPredicate]
): ):
# type: (...) -> BuildResult # type: (...) -> BuildResult
"""Build wheels. """Build wheels.
@ -271,10 +265,6 @@ def build(
:return: The list of InstallRequirement that succeeded to build and :return: The list of InstallRequirement that succeeded to build and
the list of InstallRequirement that failed to build. the list of InstallRequirement that failed to build.
""" """
if check_binary_allowed is None:
# Binaries allowed by default.
check_binary_allowed = _always_true
if not requirements: if not requirements:
return [], [] return [], []
@ -287,9 +277,7 @@ def build(
with indent_log(): with indent_log():
build_successes, build_failures = [], [] build_successes, build_failures = [], []
for req in requirements: for req in requirements:
cache_dir = _get_cache_dir( cache_dir = _get_cache_dir(req, wheel_cache)
req, wheel_cache, check_binary_allowed
)
wheel_file = _build_one( wheel_file = _build_one(
req, cache_dir, build_options, global_options req, cache_dir, build_options, global_options
) )

View File

@ -113,35 +113,17 @@ def test_should_build_legacy_wheel_installed(is_wheel_installed):
@pytest.mark.parametrize( @pytest.mark.parametrize(
"req, disallow_binaries, expected", "req, expected",
[ [
(ReqMock(editable=True), False, False), (ReqMock(editable=True), False),
(ReqMock(source_dir=None), False, False), (ReqMock(source_dir=None), False),
(ReqMock(link=Link("git+https://g.c/org/repo")), False, False), (ReqMock(link=Link("git+https://g.c/org/repo")), False),
(ReqMock(link=Link("https://g.c/dist.tgz")), False, False), (ReqMock(link=Link("https://g.c/dist.tgz")), False),
(ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), False, True), (ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), True),
(ReqMock(editable=True), True, False),
(ReqMock(source_dir=None), True, False),
(ReqMock(link=Link("git+https://g.c/org/repo")), True, False),
(ReqMock(link=Link("https://g.c/dist.tgz")), True, False),
(ReqMock(link=Link("https://g.c/dist-2.0.4.tgz")), True, False),
], ],
) )
def test_should_cache( def test_should_cache(req, expected):
req, disallow_binaries, expected assert wheel_builder._should_cache(req) is expected
):
def check_binary_allowed(req):
return not disallow_binaries
should_cache = wheel_builder._should_cache(
req, check_binary_allowed
)
if not wheel_builder.should_build_for_install_command(
req, check_binary_allowed=check_binary_allowed
):
# never cache if pip install would not have built)
assert not should_cache
assert should_cache is expected
def test_should_cache_git_sha(script, tmpdir): def test_should_cache_git_sha(script, tmpdir):
@ -153,16 +135,12 @@ def test_should_cache_git_sha(script, tmpdir):
# a link referencing a sha should be cached # a link referencing a sha should be cached
url = "git+https://g.c/o/r@" + commit + "#egg=mypkg" url = "git+https://g.c/o/r@" + commit + "#egg=mypkg"
req = ReqMock(link=Link(url), source_dir=repo_path) req = ReqMock(link=Link(url), source_dir=repo_path)
assert wheel_builder._should_cache( assert wheel_builder._should_cache(req)
req, check_binary_allowed=lambda r: True,
)
# a link not referencing a sha should not be cached # a link not referencing a sha should not be cached
url = "git+https://g.c/o/r@master#egg=mypkg" url = "git+https://g.c/o/r@master#egg=mypkg"
req = ReqMock(link=Link(url), source_dir=repo_path) req = ReqMock(link=Link(url), source_dir=repo_path)
assert not wheel_builder._should_cache( assert not wheel_builder._should_cache(req)
req, check_binary_allowed=lambda r: True,
)
def test_format_command_result__INFO(caplog): def test_format_command_result__INFO(caplog):