Merge pull request #8817 from sbidoul/improve-git-checkout

news/8815.feature

@@ -0,0 +1,2 @@
+When installing a git URL that refers to a commit that is not available locally
+after git clone, attempt to fetch it from the remote.

src/pip/_internal/vcs/git.py

@@ -163,6 +163,29 @@ class Git(VersionControl):
 
         return (sha, False)
 
+    @classmethod
+    def _should_fetch(cls, dest, rev):
+        """
+        Return true if rev is a ref or is a commit that we don't have locally.
+
+        Branches and tags are not considered in this method because they are
+        assumed to be always available locally (which is a normal outcome of
+        ``git clone`` and ``git fetch --tags``).
+        """
+        if rev.startswith("refs/"):
+            # Always fetch remote refs.
+            return True
+
+        if not looks_like_hash(rev):
+            # Git fetch would fail with abbreviated commits.
+            return False
+
+        if cls.has_commit(dest, rev):
+            # Don't fetch if we have the commit locally.
+            return False
+
+        return True
+
     @classmethod
     def resolve_revision(cls, dest, url, rev_options):
         # type: (str, HiddenText, RevOptions) -> RevOptions
@@ -194,10 +217,10 @@ class Git(VersionControl):
                 rev,
             )
 
-        if not rev.startswith('refs/'):
+        if not cls._should_fetch(dest, rev):
             return rev_options
 
-        # If it looks like a ref, we have to fetch it explicitly.
+        # fetch the requested revision
         cls.run_command(
             make_command('fetch', '-q', url, rev_options.to_args()),
             cwd=dest,
@@ -306,6 +329,20 @@ class Git(VersionControl):
         url = found_remote.split(' ')[1]
         return url.strip()
 
+    @classmethod
+    def has_commit(cls, location, rev):
+        """
+        Check if rev is a commit that is available in the local repository.
+        """
+        try:
+            cls.run_command(
+                ['rev-parse', '-q', '--verify', "sha^" + rev], cwd=location
+            )
+        except SubProcessError:
+            return False
+        else:
+            return True
+
     @classmethod
     def get_revision(cls, location, rev=None):
         if rev is None:

tests/functional/test_vcs_git.py

@@ -250,3 +250,35 @@ def test_get_repository_root(script):
 
     root2 = Git.get_repository_root(version_pkg_path.joinpath("tests"))
     assert os.path.normcase(root2) == os.path.normcase(version_pkg_path)
+
+
+def test_resolve_commit_not_on_branch(script, tmp_path):
+    repo_path = tmp_path / "repo"
+    repo_file = repo_path / "file.txt"
+    clone_path = repo_path / "clone"
+    repo_path.mkdir()
+    script.run("git", "init", cwd=str(repo_path))
+
+    repo_file.write_text(u".")
+    script.run("git", "add", "file.txt", cwd=str(repo_path))
+    script.run("git", "commit", "-m", "initial commit", cwd=str(repo_path))
+    script.run("git", "checkout", "-b", "abranch", cwd=str(repo_path))
+
+    # create a commit
+    repo_file.write_text(u"..")
+    script.run("git", "commit", "-a", "-m", "commit 1", cwd=str(repo_path))
+    commit = script.run(
+        "git", "rev-parse", "HEAD", cwd=str(repo_path)
+    ).stdout.strip()
+
+    # make sure our commit is not on a branch
+    script.run("git", "checkout", "master", cwd=str(repo_path))
+    script.run("git", "branch", "-D", "abranch", cwd=str(repo_path))
+
+    # create a ref that points to our commit
+    (repo_path / ".git" / "refs" / "myrefs").mkdir(parents=True)
+    (repo_path / ".git" / "refs" / "myrefs" / "myref").write_text(commit)
+
+    # check we can fetch our commit
+    rev_options = Git.make_rev_options(commit)
+    Git().fetch_new(str(clone_path), repo_path.as_uri(), rev_options)

tests/unit/test_vcs.py

@@ -173,8 +173,9 @@ def test_git_resolve_revision_not_found_warning(get_sha_mock, caplog):
     sha = 40 * 'a'
     rev_options = Git.make_rev_options(sha)
 
-    new_options = Git.resolve_revision('.', url, rev_options)
-    assert new_options.rev == sha
+    # resolve_revision with a full sha would fail here because
+    # it attempts a git fetch. This case is now covered by
+    # test_resolve_commit_not_on_branch.
 
     rev_options = Git.make_rev_options(sha[:6])
     new_options = Git.resolve_revision('.', url, rev_options)