From 976a0f2b59f33052172115f4c31eca7f08034cd2 Mon Sep 17 00:00:00 2001
From: karajan1001 <mishanyo1001@gmail.com>
Date: Mon, 6 Dec 2021 20:38:38 +0800
Subject: [PATCH] describe: make active branch always be the first choice of
 `describe`(#4)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fix: #4 and iterative/dvc#6051

In dvc we get a random toggling branch name in exp show. It is because
when we iter branches the dulwich.refs.keys gives an unordered Set
of refs. Here we choose the active branch if we are `describe` its
commit rev.

1. Make active branch always in the first place.
2. Add a new test to it.

Co-authored-by: Peter Rowlands (변기호) <peter@pmrowla.com>
---
 scmrepo/git/__init__.py                 | 17 +++++++++++++++-
 scmrepo/git/backend/base.py             |  2 +-
 scmrepo/git/backend/dulwich/__init__.py |  2 +-
 scmrepo/git/backend/gitpython.py        |  2 +-
 scmrepo/git/backend/pygit2.py           |  2 +-
 tests/test_git.py                       | 27 +++++++++++++++++++++++++
 6 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/scmrepo/git/__init__.py b/scmrepo/git/__init__.py
index 7c738601..4ea741d3 100644
--- a/scmrepo/git/__init__.py
+++ b/scmrepo/git/__init__.py
@@ -314,7 +314,7 @@ def add_commit(
     _stash_push = partialmethod(_backend_func, "_stash_push")
     _stash_apply = partialmethod(_backend_func, "_stash_apply")
     _stash_drop = partialmethod(_backend_func, "_stash_drop")
-    describe = partialmethod(_backend_func, "describe")
+    _describe = partialmethod(_backend_func, "_describe")
     diff = partialmethod(_backend_func, "diff")
     reset = partialmethod(_backend_func, "reset")
     checkout_index = partialmethod(_backend_func, "checkout_index")
@@ -397,3 +397,18 @@ def stash_workspace(self, **kwargs):
 
     def _reset(self) -> None:
         self.backends.reset_all()
+
+    def describe(
+        self,
+        rev: str,
+        base: Optional[str] = None,
+        match: Optional[str] = None,
+        exclude: Optional[str] = None,
+    ) -> Optional[str]:
+        if (
+            base == "refs/heads"
+            and self.get_rev() == rev
+            and self.get_ref("HEAD", follow=False).startswith(base)
+        ):
+            return self.get_ref("HEAD", follow=False)
+        return self._describe(rev, base, match, exclude)
diff --git a/scmrepo/git/backend/base.py b/scmrepo/git/backend/base.py
index 9877c6da..8592d5ea 100644
--- a/scmrepo/git/backend/base.py
+++ b/scmrepo/git/backend/base.py
@@ -278,7 +278,7 @@ def _stash_drop(self, ref: str, index: int):
         """Drop the specified stash revision."""
 
     @abstractmethod
-    def describe(
+    def _describe(
         self,
         rev: str,
         base: Optional[str] = None,
diff --git a/scmrepo/git/backend/dulwich/__init__.py b/scmrepo/git/backend/dulwich/__init__.py
index fdcbc364..866914e1 100644
--- a/scmrepo/git/backend/dulwich/__init__.py
+++ b/scmrepo/git/backend/dulwich/__init__.py
@@ -590,7 +590,7 @@ def _stash_drop(self, ref: str, index: int):
         except ValueError as exc:
             raise SCMError("Failed to drop stash entry") from exc
 
-    def describe(
+    def _describe(
         self,
         rev: str,
         base: Optional[str] = None,
diff --git a/scmrepo/git/backend/gitpython.py b/scmrepo/git/backend/gitpython.py
index 60cbc963..ca0fa235 100644
--- a/scmrepo/git/backend/gitpython.py
+++ b/scmrepo/git/backend/gitpython.py
@@ -552,7 +552,7 @@ def _stash_drop(self, ref: str, index: int):
         except GitCommandError:
             self.remove_ref(ref)
 
-    def describe(
+    def _describe(
         self,
         rev: str,
         base: Optional[str] = None,
diff --git a/scmrepo/git/backend/pygit2.py b/scmrepo/git/backend/pygit2.py
index b7e2990f..8408161c 100644
--- a/scmrepo/git/backend/pygit2.py
+++ b/scmrepo/git/backend/pygit2.py
@@ -459,7 +459,7 @@ def _stash_drop(self, ref: str, index: int):
 
         self.repo.stash_drop(index)
 
-    def describe(
+    def _describe(
         self,
         rev: str,
         base: Optional[str] = None,
diff --git a/tests/test_git.py b/tests/test_git.py
index 58b520fc..9b858704 100644
--- a/tests/test_git.py
+++ b/tests/test_git.py
@@ -607,3 +607,30 @@ def test_checkout_subdir(tmp_dir: TmpDir, scm: Git, git: Git):
     with (tmp_dir / "dir").chdir():
         git.checkout(rev)
         assert not (tmp_dir / "dir" / "bar").exists()
+
+
+@pytest.mark.skip_git_backend("pygit2", "gitpython")
+def test_describe(tmp_dir: TmpDir, scm: Git, git: Git):
+    tmp_dir.gen({"foo": "foo"})
+    scm.add_commit("foo", message="foo")
+    rev_foo = scm.get_rev()
+
+    tmp_dir.gen({"foo": "bar"})
+    scm.add_commit("foo", message="bar")
+    rev_bar = scm.get_rev()
+
+    assert git.describe(rev_foo, "refs/heads") is None
+
+    scm.checkout("branch", create_new=True)
+    assert git.describe(rev_bar, "refs/heads") == "refs/heads/branch"
+
+    tmp_dir.gen({"foo": "foobar"})
+    scm.add_commit("foo", message="foobar")
+    rev_foobar = scm.get_rev()
+
+    scm.checkout("master")
+    assert git.describe(rev_bar, "refs/heads") == "refs/heads/master"
+    assert git.describe(rev_foobar, "refs/heads") == "refs/heads/branch"
+
+    scm.tag("tag")
+    assert git.describe(rev_bar) == "refs/tags/tag"