From 93313ec22cd0e8109ed878c4240a741daadcb77b Mon Sep 17 00:00:00 2001 From: Robert Collins Date: Wed, 1 Apr 2015 14:39:45 +1300 Subject: [PATCH] Issue #2140: Build wheels automatically Building wheels before installing elminates a cause of broken environments - where install fails after we've already installed one or more packages. If a package fails to wheel, we run setup.py install as normally. --- CHANGES.txt | 2 + docs/reference/pip_install.rst | 16 +++ pip/commands/install.py | 25 +++- pip/req/req_set.py | 3 +- pip/wheel.py | 111 +++++++++++++++--- tests/conftest.py | 7 +- tests/data/packages/README.txt | 5 + .../requires_wheelbroken_upper/__init__.py | 0 .../requires_wheelbroken_upper/setup.py | 5 + tests/functional/test_install.py | 51 +++++++- 10 files changed, 198 insertions(+), 27 deletions(-) create mode 100644 tests/data/packages/requires_wheelbroken_upper/requires_wheelbroken_upper/__init__.py create mode 100644 tests/data/packages/requires_wheelbroken_upper/setup.py diff --git a/CHANGES.txt b/CHANGES.txt index 51ee9eb3413..87e66e26df2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -19,6 +19,8 @@ * Ignores bz2 archives if Python wasn't compiled with bz2 support. Fixes :issue:`497` +* Build Wheels prior to installing from sdist, caching them in the pip cache + directory to speed up subsequent installs. (:pull:`2618`) **6.1.1 (2015-04-07)** diff --git a/docs/reference/pip_install.rst b/docs/reference/pip_install.rst index 29612593e88..6f3ac11b11e 100644 --- a/docs/reference/pip_install.rst +++ b/docs/reference/pip_install.rst @@ -16,6 +16,15 @@ Description .. pip-command-description:: install +Overview +++++++++ + +Pip install has several stages: + +1. Resolve dependencies. What will be installed is determined here. +2. Build wheels. All the dependencies that can be are built into wheels. +3. Install the packages (and uninstall anything being upgraded/replaced). + Installation Order ++++++++++++++++++ @@ -404,6 +413,9 @@ that pip will not attempt to build a better wheel for Python's that would have supported it, once any generic wheel is built. To correct this, make sure that the wheel's are built with Python specific tags - e.g. pp on Pypy. +When no wheels are found for an sdist, pip will attempt to build a wheel +automatically and insert it into the wheel cache. + Hash Verification +++++++++++++++++ @@ -499,6 +511,10 @@ implement the following command:: This should implement the complete process of installing the package in "editable" mode. +All packages will be attempted to built into wheels:: + + setup.py bdist_wheel -d XXX + One further ``setup.py`` command is invoked by ``pip install``:: setup.py clean diff --git a/pip/commands/install.py b/pip/commands/install.py index 44b1e5351a7..09b554a4c85 100644 --- a/pip/commands/install.py +++ b/pip/commands/install.py @@ -6,10 +6,14 @@ import tempfile import shutil import warnings +try: + import wheel +except ImportError: + wheel = None from pip.req import RequirementSet -from pip.locations import virtualenv_no_global, distutils_scheme from pip.basecommand import RequirementCommand +from pip.locations import virtualenv_no_global, distutils_scheme from pip.index import PackageFinder from pip.exceptions import ( InstallationError, CommandError, PreviousBuildDirError, @@ -18,6 +22,7 @@ from pip.utils import ensure_dir from pip.utils.build import BuildDirectory from pip.utils.deprecation import RemovedInPip8Warning +from pip.wheel import WheelBuilder logger = logging.getLogger(__name__) @@ -233,7 +238,6 @@ def run(self, options, args): with self._build_session(options) as session: finder = self._build_package_finder(options, index_urls, session) - build_delete = (not (options.no_clean or options.build_dir)) with BuildDirectory(options.build_dir, delete=build_delete) as build_dir: @@ -262,7 +266,22 @@ def run(self, options, args): return try: - requirement_set.prepare_files(finder) + if options.download_dir or not wheel: + # on -d don't do complex things like building + # wheels, and don't try to build wheels when wheel is + # not installed. + requirement_set.prepare_files(finder) + else: + # build wheels before install. + wb = WheelBuilder( + requirement_set, + finder, + build_options=[], + global_options=[], + ) + # Ignore the result: a failed wheel will be + # installed from the sdist/vcs whatever. + wb.build(autobuilding=True) if not options.download_dir: requirement_set.install( diff --git a/pip/req/req_set.py b/pip/req/req_set.py index 751c85bd123..d601c17a661 100644 --- a/pip/req/req_set.py +++ b/pip/req/req_set.py @@ -162,7 +162,8 @@ def __init__(self, build_dir, src_dir, download_dir, upgrade=False, self.build_dir = build_dir self.src_dir = src_dir # XXX: download_dir and wheel_download_dir overlap semantically and may - # be combinable. + # be combined if we're willing to have non-wheel archives present in + # the wheelhouse output by 'pip wheel'. self.download_dir = download_dir self.upgrade = upgrade self.ignore_installed = ignore_installed diff --git a/pip/wheel.py b/pip/wheel.py index 6b9129c2944..24b4244d6e5 100644 --- a/pip/wheel.py +++ b/pip/wheel.py @@ -14,6 +14,7 @@ import shutil import stat import sys +import tempfile import warnings from base64 import urlsafe_b64encode @@ -22,12 +23,13 @@ from pip._vendor.six import StringIO import pip -from pip.download import path_to_url +from pip.download import path_to_url, unpack_url from pip.exceptions import InvalidWheelFilename, UnsupportedWheel -from pip.locations import distutils_scheme +from pip.locations import distutils_scheme, PIP_DELETE_MARKER_FILENAME from pip import pep425tags from pip.utils import ( - call_subprocess, ensure_dir, make_path_relative, captured_stdout) + call_subprocess, ensure_dir, make_path_relative, captured_stdout, + rmtree) from pip.utils.logging import indent_log from pip._vendor.distlib.scripts import ScriptMaker from pip._vendor import pkg_resources @@ -49,6 +51,12 @@ def _cache_for_filename(cache_dir, sdistfilename): to cache them in, and then consult that directory when looking up cache hits. + We only insert things into the cache if they have plausible version + numbers, so that we don't contaminate the cache with things that were not + unique. E.g. ./package might have dozens of installs done for it and build + a version of 0.0...and if we built and cached a wheel, we'd end up using + the same wheel even if the source has been edited. + :param cache_dir: The cache_dir being used by pip. :param sdistfilename: The filename of the sdist for which this will cache wheels. @@ -590,16 +598,35 @@ class WheelBuilder(object): """Build wheels from a RequirementSet.""" def __init__(self, requirement_set, finder, build_options=None, - global_options=None): + global_options=None, cache_root=None): self.requirement_set = requirement_set self.finder = finder - self.wheel_dir = requirement_set.wheel_download_dir + self._cache_root = requirement_set._cache_root + self._wheel_dir = requirement_set.wheel_download_dir self.build_options = build_options or [] self.global_options = global_options or [] - def _build_one(self, req): - """Build one wheel.""" + def _build_one(self, req, output_dir): + """Build one wheel. + :return: The filename of the built wheel, or None if the build failed. + """ + tempd = tempfile.mkdtemp('pip-wheel-') + try: + if self.__build_one(req, tempd): + try: + wheel_name = os.listdir(tempd)[0] + wheel_path = os.path.join(output_dir, wheel_name) + os.rename(os.path.join(tempd, wheel_name), wheel_path) + logger.info('Stored in directory: %s', output_dir) + return wheel_path + except: + return None + return None + finally: + rmtree(tempd) + + def __build_one(self, req, tempd): base_args = [ sys.executable, '-c', "import setuptools;__file__=%r;" @@ -608,8 +635,8 @@ def _build_one(self, req): ] + list(self.global_options) logger.info('Running setup.py bdist_wheel for %s', req.name) - logger.info('Destination directory: %s', self.wheel_dir) - wheel_args = base_args + ['bdist_wheel', '-d', self.wheel_dir] \ + logger.debug('Destination directory: %s', tempd) + wheel_args = base_args + ['bdist_wheel', '-d', tempd] \ + self.build_options try: call_subprocess(wheel_args, cwd=req.source_dir, show_stdout=False) @@ -618,10 +645,15 @@ def _build_one(self, req): logger.error('Failed building wheel for %s', req.name) return False - def build(self): - """Build wheels.""" + def build(self, autobuilding=False): + """Build wheels. - # unpack and constructs req set + :param unpack: If True, replace the sdist we built from the with the + newly built wheel, in preparation for installation. + :return: True if all the wheels built correctly. + """ + assert self._wheel_dir or (autobuilding and self._cache_root) + # unpack sdists and constructs req set self.requirement_set.prepare_files(self.finder) reqset = self.requirement_set.requirements.values() @@ -629,14 +661,24 @@ def build(self): buildset = [] for req in reqset: if req.is_wheel: - logger.info( - 'Skipping %s, due to already being wheel.', req.name, - ) + if not autobuilding: + logger.info( + 'Skipping %s, due to already being wheel.', req.name) elif req.editable: - logger.info( - 'Skipping %s, due to being editable', req.name, - ) + if not autobuilding: + logger.info( + 'Skipping bdist_wheel for %s, due to being editable', + req.name) + elif autobuilding and not req.source_dir: + pass else: + if autobuilding: + link = req.link + base, ext = link.splitext() + if pip.index.egg_info_matches(base, None, link) is None: + # Doesn't look like a package - don't autobuild a wheel + # because we'll have no way to lookup the result sanely + continue buildset.append(req) if not buildset: @@ -650,8 +692,39 @@ def build(self): with indent_log(): build_success, build_failure = [], [] for req in buildset: - if self._build_one(req): + if autobuilding: + output_dir = _cache_for_filename( + self._cache_root, req.link.filename) + ensure_dir(output_dir) + else: + output_dir = self._wheel_dir + wheel_file = self._build_one(req, output_dir) + if wheel_file: build_success.append(req) + if autobuilding: + # XXX: This is mildly duplicative with prepare_files, + # but not close enough to pull out to a single common + # method. + # The code below assumes temporary source dirs - + # prevent it doing bad things. + if req.source_dir and not os.path.exists(os.path.join( + req.source_dir, PIP_DELETE_MARKER_FILENAME)): + raise AssertionError( + "bad source dir - missing marker") + # Delete the source we built the wheel from + req.remove_temporary_source() + # set the build directory again - name is known from + # the work prepare_files did. + req.source_dir = req.build_location( + self.requirement_set.build_dir) + # Update the link for this. + req.link = pip.index.Link( + path_to_url(wheel_file), trusted=True) + assert req.link.is_wheel + # extract the wheel into the dir + unpack_url( + req.link, req.source_dir, None, False, + session=self.requirement_set.session) else: build_failure.append(req) diff --git a/tests/conftest.py b/tests/conftest.py index 84019dd06bc..3e05bff420f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,8 @@ import py import pytest +from pip.utils import appdirs + from tests.lib import SRC_DIR, TestData from tests.lib.path import Path from tests.lib.scripttest import PipTestEnvironment @@ -119,7 +121,7 @@ def isolate(tmpdir): @pytest.fixture -def virtualenv(tmpdir, monkeypatch): +def virtualenv(tmpdir, monkeypatch, isolate): """ Return a virtual environment which is unique to each test function invocation created inside of a sub directory of the test function's @@ -148,6 +150,9 @@ def virtualenv(tmpdir, monkeypatch): pip_source_dir=pip_src, ) + # Clean out our cache: creating the venv injects wheels into it. + shutil.rmtree(appdirs.user_cache_dir("pip")) + # Undo our monkeypatching of shutil monkeypatch.undo() diff --git a/tests/data/packages/README.txt b/tests/data/packages/README.txt index b93d92f2332..7fd5cd3a7fc 100644 --- a/tests/data/packages/README.txt +++ b/tests/data/packages/README.txt @@ -103,3 +103,8 @@ Is an empty package which install_requires the simple and simple2 packages. requires_simple_extra-0.1-py2.py3-none-any.whl ---------------------------------------------- requires_simple_extra[extra] requires simple==1.0 + +requires_wheelbroken_upper +-------------------------- +Requires wheelbroken and upper - used for testing implicit wheel building +during install. diff --git a/tests/data/packages/requires_wheelbroken_upper/requires_wheelbroken_upper/__init__.py b/tests/data/packages/requires_wheelbroken_upper/requires_wheelbroken_upper/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/data/packages/requires_wheelbroken_upper/setup.py b/tests/data/packages/requires_wheelbroken_upper/setup.py new file mode 100644 index 00000000000..255cf2219eb --- /dev/null +++ b/tests/data/packages/requires_wheelbroken_upper/setup.py @@ -0,0 +1,5 @@ +import setuptools +setuptools.setup( + name="requires_wheelbroken_upper", + version="0", + install_requires=['wheelbroken', 'upper']) diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index a294d598fcd..21703858352 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -7,7 +7,7 @@ import pytest -from pip.utils import rmtree +from pip.utils import appdirs, rmtree from tests.lib import (pyversion, pyversion_tuple, _create_test_package, _create_svn_repo, path_to_url) from tests.lib.local_repos import local_checkout @@ -144,14 +144,24 @@ def test_install_dev_version_from_pypi(script): ) -def test_install_editable_from_git(script, tmpdir): +def _test_install_editable_from_git(script, tmpdir, wheel): """Test cloning from Git.""" + if wheel: + script.pip('install', 'wheel') pkg_path = _create_test_package(script, name='testpackage', vcs='git') args = ['install', '-e', 'git+%s#egg=testpackage' % path_to_url(pkg_path)] result = script.pip(*args, **{"expect_error": True}) result.assert_installed('testpackage', with_files=['.git']) +def test_install_editable_from_git(script, tmpdir): + _test_install_editable_from_git(script, tmpdir, False) + + +def test_install_editable_from_git_autobuild_wheel(script, tmpdir): + _test_install_editable_from_git(script, tmpdir, True) + + def test_install_editable_from_hg(script, tmpdir): """Test cloning from Mercurial.""" pkg_path = _create_test_package(script, name='testpackage', vcs='hg') @@ -667,5 +677,40 @@ def test_install_topological_sort(script, data): def test_install_wheel_broken(script, data): script.pip('install', 'wheel') res = script.pip( - 'install', '--no-index', '-f', data.find_links, 'wheelbroken') + 'install', '--no-index', '-f', data.find_links, 'wheelbroken', + expect_stderr=True) assert "Successfully installed wheelbroken-0.1" in str(res), str(res) + + +def test_install_builds_wheels(script, data): + # NB This incidentally tests a local tree + tarball inputs + # see test_install_editable_from_git_autobuild_wheel for editable + # vcs coverage. + script.pip('install', 'wheel') + to_install = data.packages.join('requires_wheelbroken_upper') + res = script.pip( + 'install', '--no-index', '-f', data.find_links, + to_install, expect_stderr=True) + expected = ("Successfully installed requires-wheelbroken-upper-0" + " upper-2.0 wheelbroken-0.1") + # Must have installed it all + assert expected in str(res), str(res) + root = appdirs.user_cache_dir('pip') + wheels = [] + for top, dirs, files in os.walk(root): + wheels.extend(files) + # and built wheels for upper and wheelbroken + assert "Running setup.py bdist_wheel for upper" in str(res), str(res) + assert "Running setup.py bdist_wheel for wheelb" in str(res), str(res) + # But not requires_wheel... which is a local dir and thus uncachable. + assert "Running setup.py bdist_wheel for requir" not in str(res), str(res) + # wheelbroken has to run install + # into the cache + assert wheels != [], str(res) + # and installed from the wheel + assert "Running setup.py install for upper" not in str(res), str(res) + # the local tree can't build a wheel (because we can't assume that every + # build will have a suitable unique key to cache on). + assert "Running setup.py install for requires-wheel" in str(res), str(res) + # wheelbroken has to run install + assert "Running setup.py install for wheelb" in str(res), str(res)