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

PEP 518: enable source installs for build dependencies #5336

Merged
merged 3 commits into from
Jul 20, 2018
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
5 changes: 3 additions & 2 deletions docs/reference/pip.rst
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ appropriately.
installation of build dependencies from source has been disabled until a safe
resolution of this issue is found.

* ``pip<18.0`` does not support the use of environment markers and extras, only
version specifiers are respected.
* ``pip<18.0``: only support installing build requirements from wheels, and
does not support the use of environment markers and extras (only version
specifiers are respected).


Future Developments
Expand Down
1 change: 1 addition & 0 deletions news/5229.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for installing PEP 518 build dependencies from source.
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove the period?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why?

Copy link
Member

Choose a reason for hiding this comment

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

I said that for consistency across news files -- I usually commit them without periods. Looking at the directory currently, it's inconsistent already so I guess this doesn't matter all that much.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'd definitely say this doesn't matter. FWIW personally I prefer complete sentences (i.e., periods).

Copy link
Member Author

Choose a reason for hiding this comment

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

Same.

5 changes: 4 additions & 1 deletion src/pip/_internal/build_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,13 @@ def install_requirements(self, finder, requirements, message):
args = [
sys.executable, '-m', 'pip', 'install', '--ignore-installed',
'--no-user', '--prefix', self.path, '--no-warn-script-location',
'--only-binary', ':all:',
]
if logger.getEffectiveLevel() <= logging.DEBUG:
args.append('-v')
for format_control in ('no_binary', 'only_binary'):
formats = getattr(finder.format_control, format_control)
args.extend(('--' + format_control.replace('_', '-'),
','.join(sorted(formats or {':none:'}))))
if finder.index_urls:
args.extend(['-i', finder.index_urls[0]])
for extra_index in finder.index_urls[1:]:
Expand Down
4 changes: 3 additions & 1 deletion src/pip/_internal/commands/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from pip._internal.index import FormatControl
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.req import RequirementSet
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.resolve import Resolver
from pip._internal.utils.filesystem import check_path_owner
from pip._internal.utils.misc import ensure_dir, normalize_path
Expand Down Expand Up @@ -180,7 +181,7 @@ def run(self, options, args):
)
options.cache_dir = None

with TempDirectory(
with RequirementTracker() as req_tracker, TempDirectory(
options.build_dir, delete=build_delete, kind="download"
) as directory:

Expand All @@ -204,6 +205,7 @@ def run(self, options, args):
wheel_download_dir=None,
progress_bar=options.progress_bar,
build_isolation=options.build_isolation,
req_tracker=req_tracker,
)

resolver = Resolver(
Expand Down
4 changes: 3 additions & 1 deletion src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from pip._internal.operations.check import check_install_conflicts
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.req import RequirementSet, install_given_reqs
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.resolve import Resolver
from pip._internal.status_codes import ERROR
from pip._internal.utils.filesystem import check_path_owner
Expand Down Expand Up @@ -260,7 +261,7 @@ def run(self, options, args):
)
options.cache_dir = None

with TempDirectory(
with RequirementTracker() as req_tracker, TempDirectory(
options.build_dir, delete=build_delete, kind="install"
) as directory:
requirement_set = RequirementSet(
Expand All @@ -279,6 +280,7 @@ def run(self, options, args):
wheel_download_dir=None,
progress_bar=options.progress_bar,
build_isolation=options.build_isolation,
req_tracker=req_tracker,
)

resolver = Resolver(
Expand Down
5 changes: 4 additions & 1 deletion src/pip/_internal/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from pip._internal.exceptions import CommandError, PreviousBuildDirError
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.req import RequirementSet
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.resolve import Resolver
from pip._internal.utils.temp_dir import TempDirectory
from pip._internal.wheel import WheelBuilder
Expand Down Expand Up @@ -120,9 +121,10 @@ def run(self, options, args):
build_delete = (not (options.no_clean or options.build_dir))
wheel_cache = WheelCache(options.cache_dir, options.format_control)

with TempDirectory(
with RequirementTracker() as req_tracker, TempDirectory(
options.build_dir, delete=build_delete, kind="wheel"
) as directory:

requirement_set = RequirementSet(
require_hashes=options.require_hashes,
)
Expand All @@ -140,6 +142,7 @@ def run(self, options, args):
wheel_download_dir=options.wheel_dir,
progress_bar=options.progress_bar,
build_isolation=options.build_isolation,
req_tracker=req_tracker,
)

resolver = Resolver(
Expand Down
9 changes: 6 additions & 3 deletions src/pip/_internal/operations/prepare.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,12 @@ class RequirementPreparer(object):
"""

def __init__(self, build_dir, download_dir, src_dir, wheel_download_dir,
progress_bar, build_isolation):
progress_bar, build_isolation, req_tracker):
super(RequirementPreparer, self).__init__()

self.src_dir = src_dir
self.build_dir = build_dir
self.req_tracker = req_tracker

# Where still packed archives should be written to. If None, they are
# not saved, and are deleted immediately after unpacking.
Expand Down Expand Up @@ -293,7 +294,8 @@ def prepare_linked_requirement(self, req, session, finder,
(req, exc, req.link)
)
abstract_dist = make_abstract_dist(req)
abstract_dist.prep_for_dist(finder, self.build_isolation)
with self.req_tracker.track(req):
abstract_dist.prep_for_dist(finder, self.build_isolation)
if self._download_should_save:
# Make a .zip of the source_dir we already created.
if req.link.scheme in vcs.all_schemes:
Expand All @@ -319,7 +321,8 @@ def prepare_editable_requirement(self, req, require_hashes, use_user_site,
req.update_editable(not self._download_should_save)

abstract_dist = make_abstract_dist(req)
abstract_dist.prep_for_dist(finder, self.build_isolation)
with self.req_tracker.track(req):
abstract_dist.prep_for_dist(finder, self.build_isolation)

if self._download_should_save:
req.archive(self.download_dir)
Expand Down
77 changes: 77 additions & 0 deletions src/pip/_internal/req/req_tracker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from __future__ import absolute_import

import contextlib
import errno
import hashlib
import logging
import os

from pip._internal.utils.temp_dir import TempDirectory


logger = logging.getLogger(__name__)


class RequirementTracker(object):

def __init__(self):
self._root = os.environ.get('PIP_REQ_TRACKER')
if self._root is None:
self._temp_dir = TempDirectory(delete=False, kind='req-tracker')
self._temp_dir.create()
self._root = os.environ['PIP_REQ_TRACKER'] = self._temp_dir.path
logger.debug('Created requirements tracker %r', self._root)
else:
self._temp_dir = None
logger.debug('Re-using requirements tracker %r', self._root)
self._entries = set()

def __enter__(self):
return self

def __exit__(self, exc_type, exc_val, exc_tb):
self.cleanup()

def _entry_path(self, link):
hashed = hashlib.sha224(link.url_without_fragment.encode()).hexdigest()
Copy link
Member

Choose a reason for hiding this comment

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

Noting here that I did briefly think that we should split the folder name, to be like pip's cache.

wheels
[...]
├── bf
│   └── c9
│       └── a3
│           └── c538d90ef17cf7823fa51fc701a7a7a910a80f6a405bf15b1a
│               └── future-0.16.0-cp37-none-any.whl
[...]

I understand the cache might be this way to prevent far too many files in a single directory. I doubt that build-trees would get all that large though, considering that we only have a tracker for what's being built right now.

return os.path.join(self._root, hashed)

def add(self, req):
link = req.link
info = str(req)
entry_path = self._entry_path(link)
try:
with open(entry_path) as fp:
# Error, these's already a build in progress.
raise LookupError('%s is already being built: %s'
% (link, fp.read()))
except IOError as e:
if e.errno != errno.ENOENT:
raise
assert req not in self._entries
with open(entry_path, 'w') as fp:
fp.write(info)
self._entries.add(req)
logger.debug('Added %s to build tracker %r', req, self._root)

def remove(self, req):
link = req.link
self._entries.remove(req)
os.unlink(self._entry_path(link))
logger.debug('Removed %s from build tracker %r', req, self._root)

def cleanup(self):
for req in set(self._entries):
self.remove(req)
remove = self._temp_dir is not None
if remove:
self._temp_dir.cleanup()
logger.debug('%s build tracker %r',
'Removed' if remove else 'Cleaned',
self._root)

@contextlib.contextmanager
def track(self, req):
self.add(req)
yield
self.remove(req)
3 changes: 3 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ def isolate(tmpdir):
# We want to disable the version check from running in the tests
os.environ["PIP_DISABLE_PIP_VERSION_CHECK"] = "true"

# Make sure tests don't share a requirements tracker.
os.environ.pop('PIP_REQ_TRACKER', None)

# FIXME: Windows...
os.makedirs(os.path.join(home_dir, ".config", "git"))
with open(os.path.join(home_dir, ".config", "git", "config"), "wb") as fp:
Expand Down
Binary file added tests/data/packages/pep518_forkbomb-235.tar.gz
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions tests/data/src/pep518_forkbomb-235/MANIFEST.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include pyproject.toml
Empty file.
2 changes: 2 additions & 0 deletions tests/data/src/pep518_forkbomb-235/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build-system]
requires = ["setuptools", "wheel", "pep518_forkbomb"]
Empty file.
5 changes: 5 additions & 0 deletions tests/data/src/pep518_forkbomb-235/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from setuptools import setup

setup(name='pep518_forkbomb',
version='235',
py_modules=['pep518_forkbomb'])
1 change: 1 addition & 0 deletions tests/data/src/pep518_twin_forkbombs_first-234/MANIFEST.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include pyproject.toml
2 changes: 2 additions & 0 deletions tests/data/src/pep518_twin_forkbombs_first-234/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build-system]
requires = ["setuptools", "wheel", "pep518_twin_forkbombs_second"]
Empty file.
5 changes: 5 additions & 0 deletions tests/data/src/pep518_twin_forkbombs_first-234/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from setuptools import setup

setup(name='pep518_twin_forkbombs_first',
version='234',
py_modules=['pep518_twin_forkbombs_first'])
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build-system]
requires = ["setuptools", "wheel", "pep518_twin_forkbombs_first"]
Empty file.
5 changes: 5 additions & 0 deletions tests/data/src/pep518_twin_forkbombs_second-238/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from setuptools import setup

setup(name='pep518_twin_forkbombs_second',
version='238',
py_modules=['pep518_twin_forkbombs_second'])
22 changes: 19 additions & 3 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,30 @@ def test_pep518_with_extra_and_markers(script, data, common_wheels):
'wheel', '--no-index',
'-f', common_wheels,
'-f', data.find_links,
# Add tests/data/packages4, which contains a wheel for
# simple==1.0 (needed by requires_simple_extra[extra]).
'-f', data.find_links4,
data.src.join("pep518_with_extra_and_markers-1.0"),
use_module=True,
)


@pytest.mark.timeout(60)
@pytest.mark.parametrize('command', ('install', 'wheel'))
@pytest.mark.parametrize('package', ('pep518_forkbomb',
'pep518_twin_forkbombs_first',
'pep518_twin_forkbombs_second'))
def test_pep518_forkbombs(script, data, common_wheels, command, package):
package_source = next(data.packages.glob(package + '-[0-9]*.tar.gz'))
result = script.pip(
'wheel', '--no-index', '-v',
'-f', common_wheels,
'-f', data.find_links,
package,
expect_error=True,
)
assert '{1} is already being built: {0} from {1}'.format(
package, path_to_url(package_source),
) in result.stdout, str(result)


@pytest.mark.network
def test_pip_second_command_line_interface_works(script, data):
"""
Expand Down
8 changes: 0 additions & 8 deletions tests/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ def packages2(self):
def packages3(self):
return self.root.join("packages3")

@property
def packages4(self):
Copy link
Member

Choose a reason for hiding this comment

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

bye bye packages4! 👋

return self.root.join("packages4")

@property
def src(self):
return self.root.join("src")
Expand Down Expand Up @@ -132,10 +128,6 @@ def find_links2(self):
def find_links3(self):
return path_to_url(self.packages3)

@property
def find_links4(self):
return path_to_url(self.packages4)

def index_url(self, index="simple"):
return path_to_url(self.root.join("indexes", index))

Expand Down
2 changes: 2 additions & 0 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from pip._internal.req import InstallRequirement, RequirementSet
from pip._internal.req.req_file import process_line
from pip._internal.req.req_install import parse_editable
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.resolve import Resolver
from pip._internal.utils.misc import read_text_file
from tests.lib import DATA_DIR, assert_raises_regexp, requirements_file
Expand Down Expand Up @@ -47,6 +48,7 @@ def _basic_resolver(self, finder):
wheel_download_dir=None,
progress_bar="on",
build_isolation=True,
req_tracker=RequirementTracker(),
)
return Resolver(
preparer=preparer, wheel_cache=None,
Expand Down