stash: handle clean workspace on push()

Author: pmrowla

pyproject.toml

@@ -72,6 +72,7 @@ module = [
     "pygtrie",
     "funcy",
     "git",
+    "gitdb.*",
     "fsspec.*",
     "pathspec.patterns",
     "asyncssh.*",

src/scmrepo/git/backend/base.py

@@ -261,7 +261,7 @@ def _stash_push(
         self,
         ref: str,
         message: Optional[str] = None,
-        include_untracked: Optional[bool] = False,
+        include_untracked: bool = False,
     ) -> Tuple[Optional[str], bool]:
         """Push a commit onto the specified stash.
 

src/scmrepo/git/backend/dulwich/__init__.py

@@ -672,12 +672,18 @@ def _stash_push(
         self,
         ref: str,
         message: Optional[str] = None,
-        include_untracked: Optional[bool] = False,
+        include_untracked: bool = False,
     ) -> Tuple[Optional[str], bool]:
         from dulwich.repo import InvalidUserIdentity
 
         from scmrepo.git import Stash
 
+        # dulwich will silently generate an empty stash commit if there is
+        # nothing to stash, we check status here to get consistent behavior
+        # across backends
+        if not self.is_dirty(untracked_files=include_untracked):
+            return None, False
+
         if include_untracked or ref == Stash.DEFAULT_STASH:
             # dulwich stash.push does not support include_untracked and does
             # not touch working tree

src/scmrepo/git/backend/gitpython.py

@@ -359,11 +359,12 @@ def resolve_commit(self, rev: str) -> "GitCommit":
         """Return Commit object for the specified revision."""
         from git.exc import BadName, GitCommandError
         from git.objects.tag import TagObject
+        from gitdb.exc import BadObject
 
         try:
             commit = self.repo.rev_parse(rev)
-        except (BadName, GitCommandError):
-            raise SCMError(f"Invalid commit '{rev}'")
+        except (BadName, BadObject, GitCommandError) as exc:
+            raise SCMError(f"Invalid commit '{rev}'") from exc
         if isinstance(commit, TagObject):
             commit = commit.object
         return GitCommit(
@@ -503,10 +504,13 @@ def _stash_push(
         self,
         ref: str,
         message: Optional[str] = None,
-        include_untracked: Optional[bool] = False,
+        include_untracked: bool = False,
     ) -> Tuple[Optional[str], bool]:
         from scmrepo.git import Stash
 
+        if not self.is_dirty(untracked_files=include_untracked):
+            return None, False
+
         args = ["push"]
         if message:
             args.extend(["-m", message])

src/scmrepo/git/backend/pygit2.py

@@ -534,15 +534,19 @@ def _stash_push(
         self,
         ref: str,
         message: Optional[str] = None,
-        include_untracked: Optional[bool] = False,
+        include_untracked: bool = False,
     ) -> Tuple[Optional[str], bool]:
         from scmrepo.git import Stash
 
-        oid = self.repo.stash(
-            self.default_signature,
-            message=message,
-            include_untracked=include_untracked,
-        )
+        try:
+            oid = self.repo.stash(
+                self.default_signature,
+                message=message,
+                include_untracked=include_untracked,
+            )
+        except KeyError:
+            # GIT_ENOTFOUND, nothing to stash
+            return None, False
         commit = self.repo[oid]
 
         if ref != Stash.DEFAULT_STASH:

src/scmrepo/git/stash.py

@@ -32,14 +32,13 @@ def list(self):
     def push(
         self, message: Optional[str] = None, include_untracked: bool = False
     ) -> Optional[str]:
-        if not self.scm.is_dirty(untracked_files=include_untracked):
-            logger.debug("No changes to stash")
-            return None
-
         logger.debug("Stashing changes in '%s'", self.ref)
         rev, reset = self.scm._stash_push(  # pylint: disable=protected-access
             self.ref, message=message, include_untracked=include_untracked
         )
+        if not rev:
+            logger.debug("No changes to stash")
+            return None
         if reset:
             self.scm.reset(hard=True)
         return rev

tests/test_stash.py

@@ -89,20 +89,13 @@ def test_git_stash_drop(tmp_dir: TmpDir, scm: Git, ref: Optional[str]):
     https://github.com/iterative/dvc/pull/5286#issuecomment-792574294"""
 
 
-@pytest.mark.parametrize(
-    "ref",
-    [
-        pytest.param(
-            None,
-            marks=pytest.mark.xfail(
-                sys.platform in ("linux", "win32"),
-                raises=AssertionError,
-                reason=reason,
-            ),
-        ),
-        "refs/foo/stash",
-    ],
+@pytest.mark.xfail(
+    sys.platform in ("linux", "win32"),
+    raises=AssertionError,
+    strict=False,
+    reason=reason,
 )
+@pytest.mark.parametrize("ref", [None, "refs/foo/stash"])
 def test_git_stash_pop(tmp_dir: TmpDir, scm: Git, ref: Optional[str]):
     tmp_dir.gen({"file": "0"})
     scm.add_commit("file", message="init")
@@ -165,3 +158,16 @@ def test_git_stash_apply_index(
     assert dict(staged) == {"modify": ["file"]}
     assert not dict(unstaged)
     assert not dict(untracked)
+
+
+def test_git_stash_push_clean_workspace(
+    tmp_dir: TmpDir,
+    scm: Git,
+    git: Git,
+):
+    tmp_dir.gen("file", "0")
+    scm.add_commit("file", message="init")
+    assert git._stash_push("refs/stash") == (  # pylint: disable=protected-access
+        None,
+        False,
+    )