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

Add always-install-via-wheel feature flag #9778

Closed
Closed
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
9 changes: 7 additions & 2 deletions docs/html/cli/pip_install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,9 @@ Per-requirement Overrides
-------------------------

Since version 7.0 pip supports controlling the command line options given to
``setup.py`` via requirements files. This disables the use of wheels (cached or
otherwise) for that package, as ``setup.py`` does not exist for wheels.
``setup.py`` via requirements files. Unless the ``always-install-via-wheel``
feature is enabled, this disables the use of wheels (cached or otherwise),
as ``setup.py`` does not exist for wheels.

The ``--global-option`` and ``--install-option`` options are used to pass
options to ``setup.py``. For example:
Expand All @@ -334,6 +335,10 @@ script as:

python setup.py --no-user-cfg install --prefix='/usr/local' --no-compile

When the ``always-install-via-wheel`` feature is enabled, ``--install-option``
is ignored, and ``--global-option`` is passed to the ``setup.py bdist_wheel``
command that is used to create a wheel that will be installed.

Note that the only way of giving more than one option to ``setup.py``
is through multiple ``--global-option`` and ``--install-option``
options, as shown in the example above. The value of each option is
Expand Down
6 changes: 6 additions & 0 deletions news/9778.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Add an ``always-install-via-wheel`` feature flag. When enabled, ``--no-binary``
no longer implies using the legacy ``setup.py install`` path, and instead
a wheel is built locally before installing. ``--build-option`` and
``--global-option`` are now passed to ``setup.py bdist_wheel`` in ``pip
install``, as it was already done in ``pip wheel``. ``--install-option`` is
ignored (because there is no ``setup.py install`` involved).
22 changes: 16 additions & 6 deletions src/pip/_internal/cli/cmdoptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,12 +501,15 @@ def no_binary():
callback=_handle_no_binary,
type="str",
default=format_control,
help="Do not use binary packages. Can be supplied multiple times, and "
help="Do not download binary packages nor use the wheel cache. "
"Can be supplied multiple times, and "
'each time adds to the existing value. Accepts either ":all:" to '
'disable all binary packages, ":none:" to empty the set (notice '
"the colons), or one or more package names with commas between "
"them (no colons). Note that some packages are tricky to compile "
"and may fail to install when this option is used on them.",
"and may fail to install when this option is used on them. "
"Additionally, this option forces the use of the legacy 'setup.py install' "
"path, unless the 'always-install-via-wheel' feature is enabled.",
)


Expand Down Expand Up @@ -821,7 +824,10 @@ def _handle_no_use_pep517(option, opt, value, parser):
'command (use like --install-option="--install-scripts=/usr/local/'
'bin"). Use multiple --install-option options to pass multiple '
"options to setup.py install. If you are using an option with a "
"directory path, be sure to use absolute path.",
"directory path, be sure to use absolute path. "
"This option implies '--no-binary :all:'. "
"It is ignored by the 'pip install' command when the "
"'always-install-via-wheel' feature is enabled.",
) # type: Callable[..., Option]

build_options = partial(
Expand All @@ -830,7 +836,10 @@ def _handle_no_use_pep517(option, opt, value, parser):
dest="build_options",
metavar="options",
action="append",
help="Extra arguments to be supplied to 'setup.py bdist_wheel'.",
help="Extra arguments to be supplied to 'setup.py bdist_wheel'. "
"This option implies '--no-binary :all:'."
"It is ignored by the 'pip install' command unless the "
"'always-install-via-wheel' feature is enabled.",
) # type: Callable[..., Option]

global_options = partial(
Expand All @@ -840,7 +849,8 @@ def _handle_no_use_pep517(option, opt, value, parser):
action="append",
metavar="options",
help="Extra global options to be supplied to the setup.py "
"call before the install or bdist_wheel command.",
"call before the install or bdist_wheel command. "
"This option implies '--no-binary :all:'.",
) # type: Callable[..., Option]

no_clean = partial(
Expand Down Expand Up @@ -965,7 +975,7 @@ def check_list_path_option(options):
metavar="feature",
action="append",
default=[],
choices=["2020-resolver", "fast-deps", "in-tree-build"],
choices=["2020-resolver", "fast-deps", "in-tree-build", "always-install-via-wheel"],
help="Enable new functionality, that may be backward incompatible.",
) # type: Callable[..., Option]

Expand Down
13 changes: 8 additions & 5 deletions src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@
logger = logging.getLogger(__name__)


def get_check_binary_allowed(format_control):
# type: (FormatControl) -> BinaryAllowedPredicate
def get_check_binary_allowed(format_control, features_enabled):
# type: (FormatControl, List[str]) -> BinaryAllowedPredicate
def check_binary_allowed(req):
# type: (InstallRequirement) -> bool
if "always-install-via-wheel" in features_enabled:
return True
Copy link
Member

Choose a reason for hiding this comment

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

Would it be cleaner to have this logic outside of this function, in run() instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so, because the point is to get a BinaryAllowedPredicate that is passed around down to _should_build.

canonical_name = canonicalize_name(req.name or "")
allowed_formats = format_control.get_allowed_formats(canonical_name)
return "binary" in allowed_formats
Expand Down Expand Up @@ -174,6 +176,7 @@ def add_options(self):
self.cmd_opts.add_option(cmdoptions.no_use_pep517())

self.cmd_opts.add_option(cmdoptions.install_options())
self.cmd_opts.add_option(cmdoptions.build_options())
self.cmd_opts.add_option(cmdoptions.global_options())

self.cmd_opts.add_option(
Expand Down Expand Up @@ -329,7 +332,7 @@ def run(self, options, args):
)

check_binary_allowed = get_check_binary_allowed(
finder.format_control
finder.format_control, options.features_enabled
)

reqs_to_build = [
Expand All @@ -343,8 +346,8 @@ def run(self, options, args):
reqs_to_build,
wheel_cache=wheel_cache,
verify=True,
build_options=[],
global_options=[],
build_options=options.build_options or [],
global_options=options.global_options or [],
)

# If we're using PEP 517, we cannot do a direct install
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/wheel_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def _should_build(

if not check_binary_allowed(req):
logger.info(
"Skipping wheel build for %s, due to binaries "
"Using legacy 'setup.py install' for %s, due to binaries "
"being disabled for it.", req.name,
)
return False
Expand Down
22 changes: 22 additions & 0 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,17 @@ def test_install_global_option(script):
assert not result.files_created


def test_install_global_option_does_not_disable_wheel_building(
script, data, with_wheel
):
res = script.pip(
'install', '--no-index', '--global-option=-v', '-f', data.find_links,
'--use-feature=always-install-via-wheel',
'upper', expect_stderr=True)
assert "Successfully installed upper-2.0" in str(res), str(res)
assert "Building wheel for upper" in str(res), str(res)


def test_install_with_hacked_egg_info(script, data):
"""
test installing a package which defines its own egg_info class
Expand Down Expand Up @@ -1439,6 +1450,17 @@ def test_install_no_binary_disables_building_wheels(script, data, with_wheel):
assert "Running setup.py install for upper" in str(res), str(res)


def test_install_no_binary_does_not_disable_building_wheels(
script, data, with_wheel
):
res = script.pip(
'install', '--no-index', '--no-binary=upper', '-f', data.find_links,
'--use-feature=always-install-via-wheel',
'upper', expect_stderr=True)
assert "Successfully installed upper-2.0" in str(res), str(res)
assert "Building wheel for upper" in str(res), str(res)


@pytest.mark.network
def test_install_no_binary_builds_pep_517_wheel(script, data, with_wheel):
to_install = data.packages.joinpath('pep517_setup_and_pyproject')
Expand Down