Merge pull request #6851 from sbidoul/pip6640-sbi

Cache wheels built from immutable VCS requirements

Author: chrahunt

docs/html/reference/pip_install.rst

@@ -573,6 +573,9 @@ and use any packages found there. This is disabled via the same
 of that is not part of the pip API. As of 7.0, pip makes a subdirectory for
 each sdist that wheels are built from and places the resulting wheels inside.
 
+As of version 20.0, pip also caches wheels when building from an immutable Git
+reference (i.e. a commit hash).
+
 Pip attempts to choose the best wheels from those built in preference to
 building a new wheel. Note that this means when a package has both optional
 C extensions and builds ``py`` tagged wheels when the C extension can't be built

news/6640.feature

@@ -0,0 +1,2 @@
+Cache wheels built from Git requirements that are considered immutable,
+because they point to a commit hash.

src/pip/_internal/vcs/git.py

@@ -12,7 +12,7 @@
 from pip._vendor.six.moves.urllib import request as urllib_request
 
 from pip._internal.exceptions import BadCommand
-from pip._internal.utils.misc import display_path
+from pip._internal.utils.misc import display_path, hide_url
 from pip._internal.utils.subprocess import make_command
 from pip._internal.utils.temp_dir import TempDirectory
 from pip._internal.utils.typing import MYPY_CHECK_RUNNING
@@ -59,6 +59,23 @@ class Git(VersionControl):
     def get_base_rev_args(rev):
         return [rev]
 
+    def is_immutable_rev_checkout(self, url, dest):
+        # type: (str, str) -> bool
+        _, rev_options = self.get_url_rev_options(hide_url(url))
+        if not rev_options.rev:
+            return False
+        if not self.is_commit_id_equal(dest, rev_options.rev):
+            # the current commit is different from rev,
+            # which means rev was something else than a commit hash
+            return False
+        # return False in the rare case rev is both a commit hash
+        # and a tag or a branch; we don't want to cache in that case
+        # because that branch/tag could point to something else in the future
+        is_tag_or_branch = bool(
+            self.get_revision_sha(dest, rev_options.rev)[0]
+        )
+        return not is_tag_or_branch
+
     def get_git_version(self):
         VERSION_PFX = 'git version '
         version = self.run_command(['version'], show_stdout=False)

src/pip/_internal/vcs/versioncontrol.py

@@ -329,6 +329,20 @@ def get_base_rev_args(rev):
         """
         raise NotImplementedError
 
+    def is_immutable_rev_checkout(self, url, dest):
+        # type: (str, str) -> bool
+        """
+        Return true if the commit hash checked out at dest matches
+        the revision in url.
+
+        Always return False, if the VCS does not support immutable commit
+        hashes.
+
+        This method does not check if there are local uncommitted changes
+        in dest after checkout, as pip currently has no use case for that.
+        """
+        return False
+
     @classmethod
     def make_rev_options(cls, rev=None, extra_args=None):
         # type: (Optional[str], Optional[CommandArgs]) -> RevOptions

src/pip/_internal/wheel.py

@@ -52,6 +52,7 @@
 from pip._internal.utils.ui import open_spinner
 from pip._internal.utils.unpacking import unpack_file
 from pip._internal.utils.urls import path_to_url
+from pip._internal.vcs import vcs
 
 if MYPY_CHECK_RUNNING:
     from typing import (
@@ -838,7 +839,15 @@ def should_cache(
         return False
 
     if req.link and req.link.is_vcs:
-        # VCS checkout. Build wheel just for this run.
+        # VCS checkout. Build wheel just for this run
+        # unless it points to an immutable commit hash in which
+        # case it can be cached.
+        assert not req.editable
+        assert req.source_dir
+        vcs_backend = vcs.get_backend_for_scheme(req.link.scheme)
+        assert vcs_backend
+        if vcs_backend.is_immutable_rev_checkout(req.link.url, req.source_dir):
+            return True
         return False
 
     link = req.link

tests/functional/test_install_vcs_git.py

@@ -67,7 +67,7 @@ def _github_checkout(url_path, temp_dir, rev=None, egg=None, scheme=None):
     return local_url
 
 
-def _make_version_pkg_url(path, rev=None):
+def _make_version_pkg_url(path, rev=None, name="version_pkg"):
     """
     Return a "git+file://" URL to the version_pkg test package.
 
@@ -78,7 +78,7 @@ def _make_version_pkg_url(path, rev=None):
     """
     file_url = _test_path_to_file_url(path)
     url_rev = '' if rev is None else '@{}'.format(rev)
-    url = 'git+{}{}#egg=version_pkg'.format(file_url, url_rev)
+    url = 'git+{}{}#egg={}'.format(file_url, url_rev, name)
 
     return url
 
@@ -476,3 +476,40 @@ def test_check_submodule_addition(script):
         script.venv / 'src/version-pkg/testpkg/static/testfile2'
         in update_result.files_created
     )
+
+
+def test_install_git_branch_not_cached(script, with_wheel):
+    """
+    Installing git urls with a branch revision does not cause wheel caching.
+    """
+    PKG = "gitbranchnotcached"
+    repo_dir = _create_test_package(script, name=PKG)
+    url = _make_version_pkg_url(repo_dir, rev="master", name=PKG)
+    result = script.pip("install", url, "--only-binary=:all:")
+    assert "Successfully built {}".format(PKG) in result.stdout, result.stdout
+    script.pip("uninstall", "-y", PKG)
+    # build occurs on the second install too because it is not cached
+    result = script.pip("install", url)
+    assert (
+        "Successfully built {}".format(PKG) in result.stdout
+    ), result.stdout
+
+
+def test_install_git_sha_cached(script, with_wheel):
+    """
+    Installing git urls with a sha revision does cause wheel caching.
+    """
+    PKG = "gitshacached"
+    repo_dir = _create_test_package(script, name=PKG)
+    commit = script.run(
+        'git', 'rev-parse', 'HEAD', cwd=repo_dir
+    ).stdout.strip()
+    url = _make_version_pkg_url(repo_dir, rev=commit, name=PKG)
+    result = script.pip("install", url)
+    assert "Successfully built {}".format(PKG) in result.stdout, result.stdout
+    script.pip("uninstall", "-y", PKG)
+    # build does not occur on the second install because it is cached
+    result = script.pip("install", url)
+    assert (
+        "Successfully built {}".format(PKG) not in result.stdout
+    ), result.stdout

tests/functional/test_vcs_git.py

@@ -221,3 +221,20 @@ def test_is_commit_id_equal(script):
     assert not Git.is_commit_id_equal(version_pkg_path, 'abc123')
     # Also check passing a None value.
     assert not Git.is_commit_id_equal(version_pkg_path, None)
+
+
+def test_is_immutable_rev_checkout(script):
+    version_pkg_path = _create_test_package(script)
+    commit = script.run(
+        'git', 'rev-parse', 'HEAD',
+        cwd=version_pkg_path
+    ).stdout.strip()
+    assert Git().is_immutable_rev_checkout(
+        "git+https://g.c/o/r@" + commit, version_pkg_path
+    )
+    assert not Git().is_immutable_rev_checkout(
+        "git+https://g.c/o/r", version_pkg_path
+    )
+    assert not Git().is_immutable_rev_checkout(
+        "git+https://g.c/o/r@master", version_pkg_path
+    )

tests/unit/test_wheel.py

@@ -20,7 +20,7 @@
     MissingCallableSuffix,
     _raise_for_invalid_entrypoint,
 )
-from tests.lib import DATA_DIR, assert_paths_equal
+from tests.lib import DATA_DIR, _create_test_package, assert_paths_equal
 
 
 class ReqMock:
@@ -166,6 +166,21 @@ def check_binary_allowed(req):
     assert should_cache is expected
 
 
+def test_should_cache_git_sha(script, tmpdir):
+    repo_path = _create_test_package(script, name="mypkg")
+    commit = script.run(
+        "git", "rev-parse", "HEAD", cwd=repo_path
+    ).stdout.strip()
+    # a link referencing a sha should be cached
+    url = "git+https://g.c/o/r@" + commit + "#egg=mypkg"
+    req = ReqMock(link=Link(url), source_dir=repo_path)
+    assert wheel.should_cache(req, check_binary_allowed=lambda r: True)
+    # a link not referencing a sha should not be cached
+    url = "git+https://g.c/o/r@master#egg=mypkg"
+    req = ReqMock(link=Link(url), source_dir=repo_path)
+    assert not wheel.should_cache(req, check_binary_allowed=lambda r: True)
+
+
 def test_format_command_result__INFO(caplog):
     caplog.set_level(logging.INFO)
     actual = wheel.format_command_result(