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

WIP prototype pep517-style editable #8154

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
47 changes: 45 additions & 2 deletions src/pip/_internal/operations/install/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,53 @@ def install_unpacked_wheel(
# TODO: Look into moving this into a dedicated class for representing an
# installation.

source = wheeldir.rstrip(os.path.sep) + os.path.sep

info_dir, metadata = parse_wheel(wheel_zip, name)

return install_unpacked_parsed_wheel(
name,
wheeldir,
info_dir,
metadata,
scheme,
req_description,
pycompile=pycompile,
warn_script_location=warn_script_location,
direct_url=direct_url
)


def install_unpacked_parsed_wheel(
name, # type: str
wheeldir, # type: str
info_dir, # type: str
metadata, # type: Message
scheme, # type: Scheme
req_description, # type: str
pycompile=True, # type: bool
warn_script_location=True, # type: bool
direct_url=None, # type: Optional[DirectUrl]
):
# type: (...) -> None
"""Install a wheel.

:param name: Name of the project to install
:param wheeldir: Base directory of the unpacked wheel
:param info_dir: Wheel's .dist-info directory
:param metadata: Wheel's parsed WHEEL-file
:param scheme: Distutils scheme dictating the install directories
:param req_description: String used in place of the requirement, for
logging
:param pycompile: Whether to byte-compile installed Python files
:param warn_script_location: Whether to check that scripts are installed
into a directory on PATH
:raises UnsupportedWheel:
* when the directory holds an unpacked wheel with incompatible
Wheel-Version
* when the .dist-info dir does not match the wheel
"""

source = wheeldir.rstrip(os.path.sep) + os.path.sep

if wheel_root_is_purelib(metadata):
lib_dir = scheme.purelib
else:
Expand Down
55 changes: 42 additions & 13 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
install_editable as install_editable_legacy
from pip._internal.operations.install.legacy import LegacyInstallFailure
from pip._internal.operations.install.legacy import install as install_legacy
from pip._internal.operations.install.wheel import install_wheel
from pip._internal.operations.install.wheel import install_wheel, install_unpacked_parsed_wheel
from pip._internal.pyproject import load_pyproject_toml, make_pyproject_path
from pip._internal.req.req_uninstall import UninstallPathSet
from pip._internal.utils.deprecation import deprecated
Expand Down Expand Up @@ -777,18 +777,47 @@ def install(

global_options = global_options if global_options is not None else []
if self.editable:
Copy link
Member

@sbidoul sbidoul May 7, 2020

Choose a reason for hiding this comment

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

Somehow I think this should become if self.editable and not self.is_wheel then legacy editable install, else go ahead and install wheel with the regular install_wheel() below, no other change necessary in this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll have to decide if this prototype makes sense or if it is just the way pip works. The prototyped approach is to put extra files into the directory into which we have already generated our *.dist-info directory as part of another hook. It's not quite a wheel. It's not necessarily easy for the build system, accustomed to building a regular wheel, to build a completely different kind of wheel. Stub modules in place of the regular contents, no need for compatibility tags, wheel caching, *.data directories, or the RECORD associated with a full wheel.

install_editable_legacy(
install_options,
global_options,
prefix=prefix,
home=home,
use_user_site=use_user_site,
name=self.name,
setup_py_path=self.setup_py_path,
isolated=self.isolated,
build_env=self.build_env,
unpacked_source_directory=self.unpacked_source_directory,
)
if self.metadata_directory and self.metadata_directory.endswith('.dist-info'):

editable_path = self.pep517_backend.build_editable()

# create empty RECORD
with open(os.path.join(self.metadata_directory, 'RECORD'), 'w+'):
pass

wheeldir = os.path.dirname(self.metadata_directory)

# put .pth file in wheeldir
with open(os.path.join(wheeldir, self.name + '.pth'), 'w+') as pthfile:
pthfile.write(editable_path['src_root'] + '\n')
dholth marked this conversation as resolved.
Show resolved Hide resolved

install_unpacked_parsed_wheel(
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the need for this. Why can't the backend just provide a wheel, which we install using the normal install_wheel route? We already have a lot of concepts and we're now adding an "unpacked parsed wheel" that's used to communicate between backend and frontend. That seems like a recipe for confusion...

Copy link
Member Author

@dholth dholth May 7, 2020

Choose a reason for hiding this comment

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

This is only the easiest way to copy files into site-packages with pip. Remember the older PEP 376 concept which is a directory with a *.dist-info. That concept also appears in the PEP 517 metadata hook. An unhappy accident that the *.data/ directory would technically work?

If it was a complete wheel it would be a strange one that only worked on this machine and shared the name of the package but not its contents. It would be a hassle to re-implement bdist_wheel in the develop command class, or to create an ephemeral ZipFile and compute RECORD in another build system.

As for pip that function should be broken up even without editable installs. It has a TODO. I've split out the only part that requires a ZipFile.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I notice that this doesn't populate RECORD when it's installed. Thought that was happening before, but maybe not.

self.name,
wheeldir,
self.metadata_directory,
{
"Root-Is-Purelib" : "false", # shouldn't matter
},
scheme=scheme,
req_description=str(self.req),
pycompile=pycompile,
warn_script_location=warn_script_location,
direct_url=None,
)

else:
install_editable_legacy(
install_options,
global_options,
prefix=prefix,
home=home,
use_user_site=use_user_site,
name=self.name,
setup_py_path=self.setup_py_path,
isolated=self.isolated,
build_env=self.build_env,
unpacked_source_directory=self.unpacked_source_directory,
)
self.install_succeeded = True
return

Expand Down
10 changes: 10 additions & 0 deletions src/pip/_vendor/pep517/_in_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,12 +238,22 @@ def build_sdist(sdist_directory, config_settings):
raise GotUnsupportedOperation(traceback.format_exc())


def build_editable():
dholth marked this conversation as resolved.
Show resolved Hide resolved
"""Invoke the optional build_editable hook."""
backend = _build_backend()
try:
return backend.build_editable()
except getattr(backend, 'UnsupportedOperation', _DummyException):
raise GotUnsupportedOperation(traceback.format_exc())


HOOK_NAMES = {
'get_requires_for_build_wheel',
'prepare_metadata_for_build_wheel',
'build_wheel',
'get_requires_for_build_sdist',
'build_sdist',
'build_editable',
}


Expand Down
15 changes: 14 additions & 1 deletion src/pip/_vendor/pep517/wrappers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import threading
from contextlib import contextmanager
import os
from os.path import dirname, abspath, join as pjoin
from os.path import dirname, abspath, normpath, join as pjoin
import shutil
from subprocess import check_call, check_output, STDOUT
import sys
Expand Down Expand Up @@ -225,6 +225,19 @@ def build_sdist(self, sdist_directory, config_settings=None):
'config_settings': config_settings,
})


def build_editable(self):
"""Build this project for editable. Usually in-place.

Returns a dict with the source directory (point .pth here)

This calls the 'build_editable' backend hook in a subprocess.
"""
editable = self._call_hook('build_editable', {})
editable['src_root'] = normpath(pjoin(self.source_dir, editable['src_root']))
return editable


def _call_hook(self, hook_name, kwargs):
# On Python 2, pytoml returns Unicode values (which is correct) but the
# environment passed to check_call needs to contain string values. We
Expand Down