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

Fix editable-mode logic when pyproject.toml present #6370

Merged
merged 2 commits into from
Apr 6, 2019
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
3 changes: 3 additions & 0 deletions news/6370.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix the handling of editable mode during installs when ``pyproject.toml`` is
present but PEP 517 doesn't require the source tree to be treated as
``pyproject.toml``-style.
66 changes: 43 additions & 23 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,48 @@ class IsSDist(DistAbstraction):
def dist(self):
return self.req.get_dist()

def _raise_conflicts(self, conflicting_with, conflicting_reqs):
conflict_messages = [
'%s is incompatible with %s' % (installed, wanted)
for installed, wanted in sorted(conflicting_reqs)
]
raise InstallationError(
"Some build dependencies for %s conflict with %s: %s." % (
self.req, conflicting_with, ', '.join(conflict_messages))
)

def install_backend_dependencies(self, finder):
# type: (PackageFinder) -> None
"""
Install any extra build dependencies that the backend requests.

:param finder: a PackageFinder object.
"""
req = self.req
with req.build_env:
# We need to have the env active when calling the hook.
req.spin_message = "Getting requirements to build wheel"
reqs = req.pep517_backend.get_requires_for_build_wheel()
conflicting, missing = req.build_env.check_requirements(reqs)
if conflicting:
self._raise_conflicts("the backend dependencies", conflicting)
req.build_env.install_requirements(
finder, missing, 'normal',
"Installing backend dependencies"
)

def prep_for_dist(self, finder, build_isolation):
# type: (PackageFinder, bool) -> None
# Prepare for building. We need to:
# 1. Load pyproject.toml (if it exists)
# 2. Set up the build environment

self.req.load_pyproject_toml()
should_isolate = self.req.use_pep517 and build_isolation

def _raise_conflicts(conflicting_with, conflicting_reqs):
raise InstallationError(
"Some build dependencies for %s conflict with %s: %s." % (
self.req, conflicting_with, ', '.join(
'%s is incompatible with %s' % (installed, wanted)
for installed, wanted in sorted(conflicting))))
should_isolate = (
(self.req.use_pep517 or self.req.pyproject_requires) and
build_isolation
)

if should_isolate:
# Isolate in a BuildEnvironment and install the build-time
Expand All @@ -127,8 +154,8 @@ def _raise_conflicts(conflicting_with, conflicting_reqs):
self.req.requirements_to_check
)
if conflicting:
_raise_conflicts("PEP 517/518 supported requirements",
conflicting)
self._raise_conflicts("PEP 517/518 supported requirements",
conflicting)
if missing:
logger.warning(
"Missing build requirements in pyproject.toml for %s.",
Expand All @@ -139,20 +166,13 @@ def _raise_conflicts(conflicting_with, conflicting_reqs):
"pip cannot fall back to setuptools without %s.",
" and ".join(map(repr, sorted(missing)))
)
# Install any extra build dependencies that the backend requests.
# This must be done in a second pass, as the pyproject.toml
# dependencies must be installed before we can call the backend.
with self.req.build_env:
# We need to have the env active when calling the hook.
self.req.spin_message = "Getting requirements to build wheel"
reqs = self.req.pep517_backend.get_requires_for_build_wheel()
conflicting, missing = self.req.build_env.check_requirements(reqs)
if conflicting:
_raise_conflicts("the backend dependencies", conflicting)
self.req.build_env.install_requirements(
finder, missing, 'normal',
"Installing backend dependencies"
)

if self.req.use_pep517:
# If we're using PEP 517, then install any extra build
# dependencies that the backend requested. This must be
# done in a second pass, as the pyproject.toml dependencies
# must be installed before we can call the backend.
self.install_backend_dependencies(finder=finder)

self.req.prepare_metadata()
self.req.assert_source_matches_version()
Expand Down
105 changes: 68 additions & 37 deletions src/pip/_internal/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
if MYPY_CHECK_RUNNING:
from typing import Any, Dict, List, Optional, Tuple

Pep517Data = Tuple[str, List[str]]


def _is_list_of_str(obj):
# type: (Any) -> bool
Expand Down Expand Up @@ -64,6 +66,37 @@ def make_editable_error(req_name, reason):
return InstallationError(message)


def get_build_system_requires(build_system, req_name):
if build_system is None:
return None

# Ensure that the build-system section in pyproject.toml conforms
# to PEP 518.
error_template = (
"{package} has a pyproject.toml file that does not comply "
"with PEP 518: {reason}"
)

# Specifying the build-system table but not the requires key is invalid
if "requires" not in build_system:
raise InstallationError(
error_template.format(package=req_name, reason=(
"it has a 'build-system' table but not "
"'build-system.requires' which is mandatory in the table"
))
)

# Error out if requires is not a list of strings
requires = build_system["requires"]
if not _is_list_of_str(requires):
raise InstallationError(error_template.format(
package=req_name,
reason="'build-system.requires' is not a list of strings.",
))

return requires


def resolve_pyproject_toml(
build_system, # type: Optional[Dict[str, Any]]
has_pyproject, # type: bool
Expand All @@ -72,7 +105,7 @@ def resolve_pyproject_toml(
editable, # type: bool
req_name, # type: str
):
# type: (...) -> Optional[Tuple[List[str], str, List[str]]]
# type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]]
"""
Return how a pyproject.toml file's contents should be interpreted.

Expand All @@ -86,6 +119,13 @@ def resolve_pyproject_toml(
:param editable: whether editable mode was requested for the requirement.
:param req_name: the name of the requirement we're processing (for
error reporting).

:return: a tuple (requires, pep517_data), where `requires` is the list
of build requirements from pyproject.toml (or else None). The value
`pep517_data` is None if `use_pep517` is False. Otherwise, it is the
tuple (backend, check), where `backend` is the name of the PEP 517
backend and `check` is the list of requirements we should check are
installed after setting up the build environment.
"""
# The following cases must use PEP 517
# We check for use_pep517 being non-None and falsey because that means
Expand Down Expand Up @@ -126,19 +166,34 @@ def resolve_pyproject_toml(
req_name, 'PEP 517 processing was explicitly requested'
)

# If we haven't worked out whether to use PEP 517 yet,
# and the user hasn't explicitly stated a preference,
# we do so if the project has a pyproject.toml file.
# If we haven't worked out whether to use PEP 517 yet, and the user
# hasn't explicitly stated a preference, we do so if the project has
# a pyproject.toml file (provided editable mode wasn't requested).
elif use_pep517 is None:
if has_pyproject and editable:
message = (
'Error installing {!r}: editable mode is not supported for '
'pyproject.toml-style projects. pip is processing this '
'project as pyproject.toml-style because it has a '
'pyproject.toml file. Since the project has a setup.py and '
'the pyproject.toml has no "build-backend" key for the '
'"build_system" value, you may pass --no-use-pep517 to opt '
'out of pyproject.toml-style processing. '
'See PEP 517 for details on pyproject.toml-style projects.'
).format(req_name)
raise InstallationError(message)

use_pep517 = has_pyproject

# At this point, we know whether we're going to use PEP 517.
assert use_pep517 is not None

requires = get_build_system_requires(build_system, req_name=req_name)

# If we're using the legacy code path, there is nothing further
# for us to do here.
if not use_pep517:
return None
return (requires, None)

if build_system is None:
# Either the user has a pyproject.toml with no build-system
Expand All @@ -149,8 +204,8 @@ def resolve_pyproject_toml(
# traditional direct setup.py execution, and require wheel and
# a version of setuptools that supports that backend.

requires = ["setuptools>=40.8.0", "wheel"]
build_system = {
"requires": ["setuptools>=40.8.0", "wheel"],
"build-backend": "setuptools.build_meta:__legacy__",
}

Expand All @@ -160,30 +215,6 @@ def resolve_pyproject_toml(
# specified a backend, though.
assert build_system is not None

# Ensure that the build-system section in pyproject.toml conforms
# to PEP 518.
error_template = (
"{package} has a pyproject.toml file that does not comply "
"with PEP 518: {reason}"
)

# Specifying the build-system table but not the requires key is invalid
if "requires" not in build_system:
raise InstallationError(
error_template.format(package=req_name, reason=(
"it has a 'build-system' table but not "
"'build-system.requires' which is mandatory in the table"
))
)

# Error out if requires is not a list of strings
requires = build_system["requires"]
if not _is_list_of_str(requires):
raise InstallationError(error_template.format(
package=req_name,
reason="'build-system.requires' is not a list of strings.",
))

backend = build_system.get("build-backend")
check = [] # type: List[str]
if backend is None:
Expand All @@ -202,7 +233,7 @@ def resolve_pyproject_toml(
backend = "setuptools.build_meta:__legacy__"
check = ["setuptools>=40.8.0", "wheel"]

return (requires, backend, check)
return (requires, (backend, check))


def load_pyproject_toml(
Expand All @@ -212,7 +243,7 @@ def load_pyproject_toml(
setup_py, # type: str
req_name # type: str
):
# type: (...) -> Optional[Tuple[List[str], str, List[str]]]
# type: (...) -> Tuple[Optional[List[str]], Optional[Pep517Data]]
"""Load the pyproject.toml file.

Parameters:
Expand All @@ -224,13 +255,13 @@ def load_pyproject_toml(
req_name - The name of the requirement we're processing (for
error reporting)

Returns:
None if we should use the legacy code path, otherwise a tuple
Returns: (requires, pep_517_data)
requires: requirements from pyproject.toml (can be None).
pep_517_data: None if we should use the legacy code path, otherwise:
(
requirements from pyproject.toml,
name of PEP 517 backend,
requirements we should check are installed after setting
up the build environment
requirements we should check are installed after setting up
the build environment
)
"""
has_pyproject = os.path.isfile(pyproject_toml)
Expand Down
15 changes: 8 additions & 7 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,21 +485,22 @@ def load_pyproject_toml(self):
use_pep517 attribute can be used to determine whether we should
follow the PEP 517 or legacy (setup.py) code path.
"""
pep517_data = load_pyproject_toml(
requires, pep517_data = load_pyproject_toml(
self.use_pep517,
self.editable,
self.pyproject_toml,
self.setup_py,
str(self)
)

if pep517_data is None:
self.use_pep517 = False
else:
self.use_pep517 = True
requires, backend, check = pep517_data
use_pep517 = bool(pep517_data)

self.use_pep517 = use_pep517
self.pyproject_requires = requires

if use_pep517:
backend, check = pep517_data
self.requirements_to_check = check
self.pyproject_requires = requires
self.pep517_backend = Pep517HookCaller(self.setup_py_dir, backend)

# Use a custom function to call subprocesses
Expand Down
6 changes: 5 additions & 1 deletion tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,12 @@ def test_constraints_local_editable_install_pep518(script, data):
to_install = data.src.join("pep518-3.0")

script.pip('download', 'setuptools', 'wheel', '-d', data.packages)
# --no-use-pep517 has to be passed since a pyproject.toml file is
# present but PEP 517 doesn't support editable mode.
script.pip(
'install', '--no-index', '-f', data.find_links, '-e', to_install)
'install', '--no-use-pep517', '--no-index', '-f', data.find_links,
'-e', to_install,
)


def test_constraints_local_install_causes_error(script, data):
Expand Down
Loading