Update ambiguous oid parsing to detect candidates on Git 2.31+ (#4897)

* Update ambiguous oid parsing to detect candidates on Git 2.31+

* Update tests

redbot/cogs/downloader/repo_manager.py

@@ -476,11 +476,20 @@ class Repo(RepoJSONMixin):
 
         if p.returncode != 0:
             stderr = p.stderr.decode(**DECODE_PARAMS).strip()
-            ambiguous_error = f"error: short SHA1 {rev} is ambiguous\nhint: The candidates are:\n"
-            if not stderr.startswith(ambiguous_error):
+            ambiguous_errors = (
+                # Git 2.31.0-rc0 and newer
+                f"error: short object ID {rev} is ambiguous\nhint: The candidates are:\n",
+                # Git 2.11.0-rc0 and newer
+                f"error: short SHA1 {rev} is ambiguous\nhint: The candidates are:\n",
+            )
+            for substring in ambiguous_errors:
+                if stderr.startswith(substring):
+                    pos = len(substring)
+                    break
+            else:
                 raise errors.UnknownRevision(f"Revision {rev} cannot be found.", git_command)
             candidates = []
-            for match in self.AMBIGUOUS_ERROR_REGEX.finditer(stderr, len(ambiguous_error)):
+            for match in self.AMBIGUOUS_ERROR_REGEX.finditer(stderr, pos):
                 candidates.append(Candidate(match["rev"], match["type"], match["desc"]))
             if candidates:
                 raise errors.AmbiguousRevision(

redbot/pytest/downloader.py

@@ -10,6 +10,7 @@ from redbot.cogs.downloader.repo_manager import RepoManager, Repo, ProcessFormat
 from redbot.cogs.downloader.installable import Installable, InstalledModule
 
 __all__ = [
+    "GIT_VERSION",
     "repo_manager",
     "repo",
     "bot_repo",
@@ -27,6 +28,17 @@ __all__ = [
 ]
 
 
+def _get_git_version():
+    """Returns version tuple in format: (major, minor)"""
+    raw_version = sp.check_output(("git", "version"), text=True)[12:]
+    # we're only interested in major and minor version if we will ever need micro
+    # there's more handling needed for versions like `2.25.0-rc1` and `2.25.0.windows.1`
+    return tuple(int(n) for n in raw_version.split(".", maxsplit=3)[:2])
+
+
+GIT_VERSION = _get_git_version()
+
+
 async def fake_run_noprint(*args, **kwargs):
     fake_result_tuple = namedtuple("fake_result", "returncode result")
     res = fake_result_tuple(0, (args, kwargs))

tests/cogs/downloader/test_git.py

@@ -5,6 +5,7 @@ import pytest
 
 from redbot.cogs.downloader.repo_manager import ProcessFormatter, Repo
 from redbot.pytest.downloader import (
+    GIT_VERSION,
     cloned_git_repo,
     git_repo,
     git_repo_with_remote,
@@ -314,8 +315,57 @@ async def test_git_get_full_sha1_from_invalid_ref(git_repo):
     assert p.stderr.decode().strip() == "fatal: Needed a single revision"
 
 
+@pytest.mark.skipif(
+    GIT_VERSION < (2, 31), reason="This is test for output from Git 2.31 and newer."
+)
 @pytest.mark.asyncio
 async def test_git_get_full_sha1_from_ambiguous_commits(git_repo):
+    # 2 ambiguous refs:
+    # branch ambiguous_1 - 95da0b576271cb5bee5f3e075074c03ee05fed05
+    # branch ambiguous_2 - 95da0b57a416d9c8ce950554228d1fc195c30b43
+    p = await git_repo._run(
+        ProcessFormatter().format(
+            git_repo.GIT_GET_FULL_SHA1, path=git_repo.folder_path, rev="95da0b57"
+        )
+    )
+    assert p.returncode == 128
+    assert p.stderr.decode().strip() == (
+        "error: short object ID 95da0b57 is ambiguous\n"
+        "hint: The candidates are:\n"
+        "hint:   95da0b576 commit 2019-10-22 - Ambiguous commit 16955\n"
+        "hint:   95da0b57a commit 2019-10-22 - Ambiguous commit 44414\n"
+        "fatal: Needed a single revision"
+    )
+
+
+@pytest.mark.skipif(
+    GIT_VERSION < (2, 31), reason="This is test for output from Git 2.31 and newer."
+)
+@pytest.mark.asyncio
+async def test_git_get_full_sha1_from_ambiguous_tag_and_commit(git_repo):
+    # 2 ambiguous refs:
+    # branch ambiguous_with_tag - c6f0e5ec04d99bdf8c6c78ff20d66d286eecb3ea
+    # tag ambiguous_tag_66387 - c6f0e5ec04d99bdf8c6c78ff20d66d286eecb3ea
+    p = await git_repo._run(
+        ProcessFormatter().format(
+            git_repo.GIT_GET_FULL_SHA1, path=git_repo.folder_path, rev="c6f0"
+        )
+    )
+    assert p.returncode == 128
+    assert p.stderr.decode().strip() == (
+        "error: short object ID c6f0 is ambiguous\n"
+        "hint: The candidates are:\n"
+        "hint:   c6f028f tag ambiguous_tag_66387\n"
+        "hint:   c6f0e5e commit 2019-10-24 - Commit ambiguous with tag.\n"
+        "fatal: Needed a single revision"
+    )
+
+
+@pytest.mark.skipif(
+    GIT_VERSION >= (2, 31), reason="This is test for output from Git older than 2.31."
+)
+@pytest.mark.asyncio
+async def test_git_get_full_sha1_from_ambiguous_commits_pre_2_31(git_repo):
     # 2 ambiguous refs:
     # branch ambiguous_1 - 95da0b576271cb5bee5f3e075074c03ee05fed05
     # branch ambiguous_2 - 95da0b57a416d9c8ce950554228d1fc195c30b43
@@ -334,8 +384,11 @@ async def test_git_get_full_sha1_from_ambiguous_commits(git_repo):
     )
 
 
+@pytest.mark.skipif(
+    GIT_VERSION >= (2, 31), reason="This is test for output from Git older than 2.31."
+)
 @pytest.mark.asyncio
-async def test_git_get_full_sha1_from_ambiguous_tag_and_commit(git_repo):
+async def test_git_get_full_sha1_from_ambiguous_tag_and_commit_pre_2_31(git_repo):
     # 2 ambiguous refs:
     # branch ambiguous_with_tag - c6f0e5ec04d99bdf8c6c78ff20d66d286eecb3ea
     # tag ambiguous_tag_66387 - c6f0e5ec04d99bdf8c6c78ff20d66d286eecb3ea