Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop --no-binary implying setup.py install #11860

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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