Skip to content

Commit

Permalink
--no-binary does not imply setup.py install anymore
Browse files Browse the repository at this point in the history
  • Loading branch information
sbidoul committed Mar 12, 2023
1 parent 4b14e7c commit 5189a6e
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 103 deletions.
2 changes: 2 additions & 0 deletions news/11451.removal.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
``--no-binary`` does not imply ``setup.py install`` anymore. Instead a wheel will be
built locally and installed.
25 changes: 2 additions & 23 deletions src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from optparse import SUPPRESS_HELP, Values
from typing import Iterable, List, Optional

from pip._vendor.packaging.utils import canonicalize_name
from pip._vendor.rich import print_json

from pip._internal.cache import WheelCache
Expand All @@ -22,7 +21,6 @@
from pip._internal.exceptions import CommandError, InstallationError
from pip._internal.locations import get_scheme
from pip._internal.metadata import get_environment
from pip._internal.models.format_control import FormatControl
from pip._internal.models.installation_report import InstallationReport
from pip._internal.operations.build.build_tracker import get_build_tracker
from pip._internal.operations.check import ConflictDetails, check_install_conflicts
Expand Down Expand Up @@ -52,26 +50,11 @@
running_under_virtualenv,
virtualenv_no_global,
)
from pip._internal.wheel_builder import (
BdistWheelAllowedPredicate,
build,
should_build_for_install_command,
)
from pip._internal.wheel_builder import build, should_build_for_install_command

logger = getLogger(__name__)


def get_check_bdist_wheel_allowed(
format_control: FormatControl,
) -> BdistWheelAllowedPredicate:
def check_binary_allowed(req: InstallRequirement) -> bool:
canonical_name = canonicalize_name(req.name or "")
allowed_formats = format_control.get_allowed_formats(canonical_name)
return "binary" in allowed_formats

return check_binary_allowed


class InstallCommand(RequirementCommand):
"""
Install packages from:
Expand Down Expand Up @@ -455,14 +438,10 @@ def run(self, options: Values, args: List[str]) -> int:
modifying_pip = pip_req.satisfied_by is None
protect_pip_from_modification_on_windows(modifying_pip=modifying_pip)

check_bdist_wheel_allowed = get_check_bdist_wheel_allowed(
finder.format_control
)

reqs_to_build = [
r
for r in requirement_set.requirements.values()
if should_build_for_install_command(r, check_bdist_wheel_allowed)
if should_build_for_install_command(r)
]

_, build_failures = build(
Expand Down
13 changes: 0 additions & 13 deletions src/pip/_internal/utils/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,3 @@ def emit_deprecation(self, name: str) -> None:
issue=8559,
emit_before_install=True,
)

LegacyInstallReasonNoBinaryForcesSetuptoolsInstall = LegacyInstallReason(
reason=(
"{name} is being installed using the legacy "
"'setup.py install' method, because the '--no-binary' option was enabled "
"for it and this currently disables local wheel building for projects that "
"don't have a 'pyproject.toml' file."
),
replacement="to enable the '--use-pep517' option",
gone_in="23.1",
issue=11451,
emit_before_install=True,
)
24 changes: 3 additions & 21 deletions src/pip/_internal/wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import os.path
import re
import shutil
from typing import Callable, Iterable, List, Optional, Tuple
from typing import Iterable, List, Optional, Tuple

from pip._vendor.packaging.utils import canonicalize_name, canonicalize_version
from pip._vendor.packaging.version import InvalidVersion, Version
Expand All @@ -19,10 +19,7 @@
from pip._internal.operations.build.wheel_editable import build_wheel_editable
from pip._internal.operations.build.wheel_legacy import build_wheel_legacy
from pip._internal.req.req_install import InstallRequirement
from pip._internal.utils.deprecation import (
LegacyInstallReasonMissingWheelPackage,
LegacyInstallReasonNoBinaryForcesSetuptoolsInstall,
)
from pip._internal.utils.deprecation import LegacyInstallReasonMissingWheelPackage
from pip._internal.utils.logging import indent_log
from pip._internal.utils.misc import ensure_dir, hash_file, is_wheel_installed
from pip._internal.utils.setuptools_build import make_setuptools_clean_args
Expand All @@ -35,7 +32,6 @@

_egg_info_re = re.compile(r"([a-z0-9_.]+)-([a-z0-9_.!+-]+)", re.IGNORECASE)

BdistWheelAllowedPredicate = Callable[[InstallRequirement], bool]
BuildResult = Tuple[List[InstallRequirement], List[InstallRequirement]]


Expand All @@ -50,7 +46,6 @@ def _contains_egg_info(s: str) -> bool:
def _should_build(
req: InstallRequirement,
need_wheel: bool,
check_bdist_wheel: Optional[BdistWheelAllowedPredicate] = None,
) -> bool:
"""Return whether an InstallRequirement should be built into a wheel."""
if req.constraint:
Expand Down Expand Up @@ -81,16 +76,6 @@ def _should_build(
if req.use_pep517:
return True

assert check_bdist_wheel is not None
if not check_bdist_wheel(req):
# /!\ When we change this to unconditionally return True, we must also remove
# support for `--install-option`. Indeed, `--install-option` implies
# `--no-binary` so we can return False here and run `setup.py install`.
# `--global-option` and `--build-option` can remain until we drop support for
# building with `setup.py bdist_wheel`.
req.legacy_install_reason = LegacyInstallReasonNoBinaryForcesSetuptoolsInstall
return False

if not is_wheel_installed():
# we don't build legacy requirements if wheel is not installed
req.legacy_install_reason = LegacyInstallReasonMissingWheelPackage
Expand All @@ -107,11 +92,8 @@ def should_build_for_wheel_command(

def should_build_for_install_command(
req: InstallRequirement,
check_bdist_wheel_allowed: BdistWheelAllowedPredicate,
) -> bool:
return _should_build(
req, need_wheel=False, check_bdist_wheel=check_bdist_wheel_allowed
)
return _should_build(req, need_wheel=False)


def _should_cache(
Expand Down
11 changes: 3 additions & 8 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -1665,12 +1665,9 @@ def test_install_no_binary_disables_building_wheels(
# Wheels are built for local directories, but not cached across runs
assert "Building wheel for requir" in str(res), str(res)
# Don't build wheel for upper which was blacklisted
assert "Building wheel for upper" not in str(res), str(res)
# Wheels are built for local directories, but not cached across runs
assert "Running setup.py install for requir" not in str(res), str(res)
assert "Building wheel for upper" in str(res), str(res)
# And these two fell back to sdist based installed.
assert "Running setup.py install for wheelb" in str(res), str(res)
assert "Running setup.py install for upper" in str(res), str(res)


@pytest.mark.network
Expand Down Expand Up @@ -1720,10 +1717,8 @@ def test_install_no_binary_disables_cached_wheels(
expect_stderr=True,
)
assert "Successfully installed upper-2.0" in str(res), str(res)
# No wheel building for upper, which was blacklisted
assert "Building wheel for upper" not in str(res), str(res)
# Must have used source, not a cached wheel to install upper.
assert "Running setup.py install for upper" in str(res), str(res)
# upper is built and not obtained from cache
assert "Building wheel for upper" in str(res), str(res)


def test_install_editable_with_wrong_egg_name(
Expand Down
6 changes: 2 additions & 4 deletions tests/functional/test_install_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,8 @@ def test_install_no_binary_via_config_disables_cached_wheels(
finally:
os.unlink(config_file.name)
assert "Successfully installed upper-2.0" in str(res), str(res)
# No wheel building for upper, which was blacklisted
assert "Building wheel for upper" not in str(res), str(res)
# Must have used source, not a cached wheel to install upper.
assert "Running setup.py install for upper" in str(res), str(res)
# upper is built and not obtained from cache
assert "Building wheel for upper" in str(res), str(res)


@pytest.mark.skipif(
Expand Down
45 changes: 11 additions & 34 deletions tests/unit/test_wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,63 +58,42 @@ def supports_pyproject_editable(self) -> bool:


@pytest.mark.parametrize(
"req, disallow_bdist_wheel, expected",
"req, expected",
[
# When binaries are allowed, we build.
(ReqMock(use_pep517=True), False, True),
(ReqMock(use_pep517=False), False, True),
# When binaries are disallowed, we don't build, unless pep517 is
# enabled.
(ReqMock(use_pep517=True), True, True),
(ReqMock(use_pep517=False), True, False),
# We build, whether pep 517 is enabled or not.
(ReqMock(use_pep517=True), True),
(ReqMock(use_pep517=False), True),
# We don't build constraints.
(ReqMock(constraint=True), False, False),
(ReqMock(constraint=True), False),
# We don't build reqs that are already wheels.
(ReqMock(is_wheel=True), False, False),
(ReqMock(editable=True, use_pep517=False), False, False),
(ReqMock(is_wheel=True), False),
# We build editables if the backend supports PEP 660.
(ReqMock(editable=True, use_pep517=False), False),
(
ReqMock(editable=True, use_pep517=True, supports_pyproject_editable=True),
False,
True,
),
(
ReqMock(editable=True, use_pep517=True, supports_pyproject_editable=False),
False,
False,
),
(ReqMock(source_dir=None), False, False),
# We don't build if there is no source dir (whatever that means!).
(ReqMock(source_dir=None), 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"), use_pep517=True),
False,
True,
),
(
ReqMock(link=Link("git+https://g.c/org/repo"), use_pep517=False),
False,
True,
),
# Disallowing binaries, however, should cause them not to be built.
# unless pep517 is enabled.
(
ReqMock(link=Link("git+https://g.c/org/repo"), use_pep517=True),
True,
True,
),
(
ReqMock(link=Link("git+https://g.c/org/repo"), use_pep517=False),
True,
False,
),
],
)
def test_should_build_for_install_command(
req: ReqMock, disallow_bdist_wheel: bool, expected: bool
) -> None:
def test_should_build_for_install_command(req: ReqMock, expected: bool) -> None:
should_build = wheel_builder.should_build_for_install_command(
cast(InstallRequirement, req),
check_bdist_wheel_allowed=lambda req: not disallow_bdist_wheel,
)
assert should_build is expected

Expand Down Expand Up @@ -144,7 +123,6 @@ def test_should_build_legacy_wheel_not_installed(is_wheel_installed: mock.Mock)
legacy_req = ReqMock(use_pep517=False)
should_build = wheel_builder.should_build_for_install_command(
cast(InstallRequirement, legacy_req),
check_bdist_wheel_allowed=lambda req: True,
)
assert not should_build

Expand All @@ -155,7 +133,6 @@ def test_should_build_legacy_wheel_installed(is_wheel_installed: mock.Mock) -> N
legacy_req = ReqMock(use_pep517=False)
should_build = wheel_builder.should_build_for_install_command(
cast(InstallRequirement, legacy_req),
check_bdist_wheel_allowed=lambda req: True,
)
assert should_build

Expand Down

0 comments on commit 5189a6e

Please sign in to comment.