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

Deprecate --install-options #11359

Merged
merged 5 commits into from
Oct 6, 2022
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/11358.removal.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Deprecate ``--install-options`` which forces pip to use the deprecated ``install``
command of ``setuptools``.
25 changes: 0 additions & 25 deletions src/pip/_internal/cli/cmdoptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,6 @@ def make_option_group(group: Dict[str, Any], parser: ConfigOptionParser) -> Opti
return option_group


def check_install_build_global(
options: Values, check_options: Optional[Values] = None
) -> None:
"""Disable wheels if per-setup.py call options are set.

:param options: The OptionParser options to update.
:param check_options: The options to check, if not supplied defaults to
options.
"""
if check_options is None:
check_options = options

def getname(n: str) -> Optional[Any]:
return getattr(check_options, n, None)

names = ["build_options", "global_options", "install_options"]
if any(map(getname, names)):
control = options.format_control
control.disallow_binaries()
logger.warning(
"Disabling all use of wheels due to the use of --build-option "
"/ --global-option / --install-option.",
)


def check_dist_restriction(options: Values, check_target: bool = False) -> None:
"""Function for determining if custom platform options are allowed.

Expand Down
7 changes: 7 additions & 0 deletions src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
from pip._internal.cli.req_command import RequirementCommand, with_cleanup
from pip._internal.cli.status_codes import SUCCESS
from pip._internal.operations.build.build_tracker import get_build_tracker
from pip._internal.req.req_install import (
LegacySetupPyOptionsCheckMode,
check_legacy_setup_py_options,
)
from pip._internal.utils.misc import ensure_dir, normalize_path, write_output
from pip._internal.utils.temp_dir import TempDirectory

Expand Down Expand Up @@ -105,6 +109,9 @@ def run(self, options: Values, args: List[str]) -> int:
)

reqs = self.get_requirements(args, options, finder, session)
check_legacy_setup_py_options(
options, reqs, LegacySetupPyOptionsCheckMode.DOWNLOAD
)

preparer = self.make_requirement_preparer(
temp_build_dir=directory,
Expand Down
12 changes: 9 additions & 3 deletions src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@
from pip._internal.operations.build.build_tracker import get_build_tracker
from pip._internal.operations.check import ConflictDetails, check_install_conflicts
from pip._internal.req import install_given_reqs
from pip._internal.req.req_install import InstallRequirement
from pip._internal.req.req_install import (
InstallRequirement,
LegacySetupPyOptionsCheckMode,
check_legacy_setup_py_options,
)
from pip._internal.utils.compat import WINDOWS
from pip._internal.utils.deprecation import (
LegacyInstallReasonFailedBdistWheel,
Expand Down Expand Up @@ -280,7 +284,6 @@ def run(self, options: Values, args: List[str]) -> int:
if options.use_user_site and options.target_dir is not None:
raise CommandError("Can not combine '--user' and '--target'")

cmdoptions.check_install_build_global(options)
upgrade_strategy = "to-satisfy-only"
if options.upgrade:
upgrade_strategy = options.upgrade_strategy
Expand Down Expand Up @@ -339,6 +342,9 @@ def run(self, options: Values, args: List[str]) -> int:

try:
reqs = self.get_requirements(args, options, finder, session)
check_legacy_setup_py_options(
options, reqs, LegacySetupPyOptionsCheckMode.INSTALL
)

if "no-binary-enable-wheel-cache" in options.features_enabled:
# TODO: remove format_control from WheelCache when the deprecation cycle
Expand Down Expand Up @@ -446,7 +452,7 @@ def run(self, options: Values, args: List[str]) -> int:
wheel_cache=wheel_cache,
verify=True,
build_options=[],
global_options=[],
global_options=global_options,
Comment on lines -449 to +455
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm this one’s unexpected. Why was it []…?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the only way to reach this point when --global-option is present, is to --use-pep517.
By passing it here, we trigger the warning in wheel_builder that global options are ignored when doing pep 517 builds.
This should help nudging users to use --config-settings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess what I’m wondering is mostly, was it a bug global_options wasn’t passed previously?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was it a bug

Yes and no. The only "bug" was that --global-option was ignored without warning in pep 517 mode.

)

# If we're using PEP 517, we cannot do a legacy setup.py install
Expand Down
11 changes: 8 additions & 3 deletions src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@
from pip._internal.cli.status_codes import SUCCESS
from pip._internal.exceptions import CommandError
from pip._internal.operations.build.build_tracker import get_build_tracker
from pip._internal.req.req_install import InstallRequirement
from pip._internal.req.req_install import (
InstallRequirement,
LegacySetupPyOptionsCheckMode,
check_legacy_setup_py_options,
)
from pip._internal.utils.deprecation import deprecated
from pip._internal.utils.misc import ensure_dir, normalize_path
from pip._internal.utils.temp_dir import TempDirectory
Expand Down Expand Up @@ -101,8 +105,6 @@ def add_options(self) -> None:

@with_cleanup
def run(self, options: Values, args: List[str]) -> int:
cmdoptions.check_install_build_global(options)

session = self.get_default_session(options)

finder = self._build_package_finder(options, session)
Expand All @@ -120,6 +122,9 @@ def run(self, options: Values, args: List[str]) -> int:
)

reqs = self.get_requirements(args, options, finder, session)
check_legacy_setup_py_options(
options, reqs, LegacySetupPyOptionsCheckMode.WHEEL
)

if "no-binary-enable-wheel-cache" in options.features_enabled:
# TODO: remove format_control from WheelCache when the deprecation cycle
Expand Down
4 changes: 0 additions & 4 deletions src/pip/_internal/req/req_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,6 @@ def handle_requirement_line(
constraint=line.constraint,
)
else:
if options:
# Disable wheels if the user has specified build options
cmdoptions.check_install_build_global(options, line.opts)

# get the options that apply to requirements
req_options = {}
for dest in SUPPORTED_OPTIONS_REQ_DEST:
Expand Down
64 changes: 64 additions & 0 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import sys
import uuid
import zipfile
from enum import Enum
from optparse import Values
from typing import Any, Collection, Dict, Iterable, List, Optional, Sequence, Union

from pip._vendor.packaging.markers import Marker
Expand Down Expand Up @@ -876,3 +878,65 @@ def check_invalid_constraint_type(req: InstallRequirement) -> str:
)

return problem


def _has_option(options: Values, reqs: List[InstallRequirement], option: str) -> bool:
if getattr(options, option, None):
return True
for req in reqs:
if getattr(req, option, None):
return True
return False


def _install_option_ignored(
install_options: List[str], reqs: List[InstallRequirement]
) -> bool:
for req in reqs:
if (install_options or req.install_options) and not req.use_pep517:
return False
return True


class LegacySetupPyOptionsCheckMode(Enum):
INSTALL = 1
WHEEL = 2
DOWNLOAD = 3


def check_legacy_setup_py_options(
options: Values,
reqs: List[InstallRequirement],
mode: LegacySetupPyOptionsCheckMode,
) -> None:
has_install_options = _has_option(options, reqs, "install_options")
has_build_options = _has_option(options, reqs, "build_options")
has_global_options = _has_option(options, reqs, "global_options")
legacy_setup_py_options_present = (
has_install_options or has_build_options or has_global_options
)
if not legacy_setup_py_options_present:
return

options.format_control.disallow_binaries()
logger.warning(
"Implying --no-binary=:all: due to the presence of "
"--build-option / --global-option / --install-option. "
"Consider using --config-settings for more flexibility.",
)
if mode == LegacySetupPyOptionsCheckMode.INSTALL and has_install_options:
if _install_option_ignored(options.install_options, reqs):
logger.warning(
"Ignoring --install-option when building using PEP 517",
)
else:
deprecated(
reason=(
"--install-option is deprecated because "
"it forces pip to use the 'setup.py install' "
"command which is itself deprecated."
),
issue=11358,
replacement="to use --config-settings",
gone_in="23.1",
)
2 changes: 2 additions & 0 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,8 @@ def test_install_global_option(script: PipTestEnvironment) -> None:
)
assert "INITools==0.1\n" in result.stdout
assert not result.files_created
assert "Implying --no-binary=:all:" in result.stderr
assert "Consider using --config-settings" in result.stderr


def test_install_with_hacked_egg_info(
Expand Down
5 changes: 4 additions & 1 deletion tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def test_install_option_in_requirements_file_overrides_cli(
reqs_file = script.scratch_path.joinpath("reqs.txt")
reqs_file.write_text("simple --install-option='-O0'")

script.pip(
result = script.pip(
"install",
"--no-index",
"-f",
Expand All @@ -355,6 +355,9 @@ def test_install_option_in_requirements_file_overrides_cli(
simple_args = simple_sdist.args()
assert "install" in simple_args
assert simple_args.index("-O1") < simple_args.index("-O0")
assert "Implying --no-binary=:all:" in result.stderr
assert "Consider using --config-settings" in result.stderr
assert "--install-option is deprecated" in result.stderr


def test_constraints_not_installed_by_default(
Expand Down
2 changes: 0 additions & 2 deletions tests/unit/test_req_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -881,5 +881,3 @@ def test_install_requirements_with_options(
< args.index("install")
< args.index(install_option)
)
assert options.format_control.no_binary == {":all:"}
assert options.format_control.only_binary == set()