diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 864daafd6..62b990ea0 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -6,9 +6,9 @@ import shlex from collections import OrderedDict from contextlib import contextmanager from functools import partialmethod -from typing import Optional +from typing import Iterable, Optional -from funcy import cached_property +from funcy import cached_property, first from pathspec.patterns import GitWildMatchPattern from dvc.exceptions import GitHookAlreadyExistsError @@ -35,22 +35,26 @@ class Git(Base): ) LOCAL_BRANCH_PREFIX = "refs/heads/" - def __init__(self, *args, **kwargs): + def __init__( + self, *args, backends: Optional[Iterable[str]] = None, **kwargs + ): self.ignored_paths = [] self.files_to_track = set() + selected = backends if backends else self.DEFAULT_BACKENDS.keys() self.backends = OrderedDict( [ - (name, cls(self, *args, **kwargs)) - for name, cls in self.DEFAULT_BACKENDS.items() + (name, self.DEFAULT_BACKENDS[name](*args, **kwargs)) + for name in selected ] ) - super().__init__(self.gitpython.root_dir) + first_ = first(self.backends.values()) + super().__init__(first_.root_dir) @property def dir(self): - return self.gitpython.repo.git_dir + return first(self.backends.values()).dir @property def gitpython(self): diff --git a/dvc/scm/git/backend/base.py b/dvc/scm/git/backend/base.py index 49793823a..6bb4b1abf 100644 --- a/dvc/scm/git/backend/base.py +++ b/dvc/scm/git/backend/base.py @@ -15,9 +15,7 @@ class BaseGitBackend(ABC): """Base Git backend class.""" @abstractmethod - def __init__( - self, scm, root_dir=os.curdir, search_parent_directories=True - ): + def __init__(self, root_dir=os.curdir, search_parent_directories=True): pass def close(self): @@ -152,7 +150,9 @@ class BaseGitBackend(ABC): """ @abstractmethod - def get_ref(self, name, follow: Optional[bool] = True) -> Optional[str]: + def get_ref( + self, name: str, follow: Optional[bool] = True + ) -> Optional[str]: """Return the value of specified ref. If follow is false, symbolic refs will not be dereferenced. @@ -231,7 +231,9 @@ class BaseGitBackend(ABC): """Apply the specified stash revision.""" @abstractmethod - def reflog_delete(self, ref: str, updateref: bool = False): + def reflog_delete( + self, ref: str, updateref: bool = False, rewrite: bool = False + ): """Delete the specified reflog entry.""" @abstractmethod diff --git a/dvc/scm/git/backend/dulwich.py b/dvc/scm/git/backend/dulwich.py index 7228fa4b3..6c2635894 100644 --- a/dvc/scm/git/backend/dulwich.py +++ b/dvc/scm/git/backend/dulwich.py @@ -17,13 +17,11 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method """Dulwich Git backend.""" def __init__( # pylint:disable=W0231 - self, scm, root_dir=os.curdir, search_parent_directories=True + self, root_dir=os.curdir, search_parent_directories=True ): from dulwich.errors import NotGitRepository from dulwich.repo import Repo - self.scm = scm - try: if search_parent_directories: self.repo = Repo.discover(start=root_dir) @@ -56,7 +54,7 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method @property def dir(self) -> str: - raise NotImplementedError + return self.repo.commondir() def add(self, paths: Iterable[str]): raise NotImplementedError @@ -308,7 +306,9 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method def _stash_apply(self, rev: str): raise NotImplementedError - def reflog_delete(self, ref: str, updateref: bool = False): + def reflog_delete( + self, ref: str, updateref: bool = False, rewrite: bool = False + ): raise NotImplementedError def describe( @@ -325,7 +325,7 @@ class DulwichBackend(BaseGitBackend): # pylint:disable=abstract-method exclude and fnmatch.fnmatch(ref, exclude) ): continue - if self.scm.get_ref(ref, follow=False) == rev: + if self.get_ref(ref, follow=False) == rev: return ref return None diff --git a/dvc/scm/git/backend/gitpython.py b/dvc/scm/git/backend/gitpython.py index 0a28739aa..403ef26c7 100644 --- a/dvc/scm/git/backend/gitpython.py +++ b/dvc/scm/git/backend/gitpython.py @@ -51,13 +51,11 @@ class GitPythonBackend(BaseGitBackend): # pylint:disable=abstract-method """git-python Git backend.""" def __init__( # pylint:disable=W0231 - self, scm, root_dir=os.curdir, search_parent_directories=True + self, root_dir=os.curdir, search_parent_directories=True ): import git from git.exc import InvalidGitRepositoryError - self.scm = scm - try: self.repo = git.Repo( root_dir, search_parent_directories=search_parent_directories @@ -131,7 +129,7 @@ class GitPythonBackend(BaseGitBackend): # pylint:disable=abstract-method raise CloneError(url, to_path) from exc # NOTE: using our wrapper to make sure that env is fixed in __init__ - repo = GitPythonBackend(None, to_path) + repo = GitPythonBackend(to_path) if rev: try: @@ -297,13 +295,64 @@ class GitPythonBackend(BaseGitBackend): # pylint:disable=abstract-method message: Optional[str] = None, symbolic: Optional[bool] = False, ): - raise NotImplementedError + from git.exc import GitCommandError - def get_ref(self, name, follow: Optional[bool] = True) -> Optional[str]: - raise NotImplementedError + if old_ref and self.get_ref(name) != old_ref: + raise SCMError(f"Failed to set ref '{name}'") + try: + if symbolic: + if message: + self.git.symbolic_ref(name, new_ref, m=message) + else: + self.git.symbolic_ref(name, new_ref) + else: + args = [name, new_ref] + if old_ref: + args.append(old_ref) + if message: + self.git.update_ref(*args, m=message, create_reflog=True) + else: + self.git.update_ref(*args) + except GitCommandError as exc: + raise SCMError(f"Failed to set ref '{name}'") from exc + + def get_ref( + self, name: str, follow: Optional[bool] = True + ) -> Optional[str]: + from git.exc import GitCommandError + + if name.startswith("refs/heads/"): + name = name[11:] + if name in self.repo.heads: + return self.repo.heads[name].commit.hexsha + elif name.startswith("refs/tags/"): + name = name[10:] + if name in self.repo.tags: + return self.repo.tags[name].commit.hexsha + else: + if not follow: + try: + return self.git.symbolic_ref(name).strip() + except GitCommandError: + pass + try: + return self.git.show_ref(name, hash=True).strip() + except GitCommandError: + pass + return None def remove_ref(self, name: str, old_ref: Optional[str] = None): - raise NotImplementedError + from git.exc import GitCommandError + + if old_ref and self.get_ref(name) != old_ref: + raise SCMError(f"Failed to set ref '{name}'") + try: + args = [name] + if old_ref: + args.append(old_ref) + self.git.update_ref(*args, d=True) + except GitCommandError as exc: + raise SCMError(f"Failed to set ref '{name}'") from exc def iter_refs(self, base: Optional[str] = None): from git import Reference @@ -367,19 +416,21 @@ class GitPythonBackend(BaseGitBackend): # pylint:disable=abstract-method # `git stash create` is intended to be used for this kind of # behavior but it doesn't support --include-untracked so we need to # use push - self.scm.dulwich.set_ref( - ref, commit.hexsha, message=commit.message - ) + self.set_ref(ref, commit.hexsha, message=commit.message) self.git.stash("drop") return commit.hexsha, False def _stash_apply(self, rev: str): self.git.stash("apply", rev) - def reflog_delete(self, ref: str, updateref: bool = False): + def reflog_delete( + self, ref: str, updateref: bool = False, rewrite: bool = False + ): args = ["delete"] if updateref: args.append("--updateref") + if rewrite: + args.append("--rewrite") args.append(ref) self.git.reflog(*args) diff --git a/dvc/scm/git/stash.py b/dvc/scm/git/stash.py index 00de7cb80..04a7ef8dc 100644 --- a/dvc/scm/git/stash.py +++ b/dvc/scm/git/stash.py @@ -43,7 +43,7 @@ class Stash: self.ref, message=message, include_untracked=include_untracked ) if reset: - self.scm.gitpython.repo.git.reset(hard=True) + self.scm.reset(hard=True) return rev def pop(self): @@ -63,14 +63,15 @@ class Stash: if index < 0 or index >= len(self): raise SCMError(f"Invalid stash ref '{ref}'") logger.debug("Dropping '%s'", ref) - self.scm.reflog_delete(ref, updateref=True) + self.scm.reflog_delete(ref, updateref=True, rewrite=True) # if we removed the last reflog entry, delete the ref and reflog if len(self) == 0: self.scm.remove_ref(self.ref) parts = self.ref.split("/") reflog = os.path.join(self.scm.root_dir, ".git", "logs", *parts) - remove(reflog) + if os.path.exists(reflog): + remove(reflog) def clear(self): logger.debug("Clear stash '%s'", self.ref) diff --git a/tests/unit/scm/test_git.py b/tests/unit/scm/test_git.py index 6a430156d..e834dcfd1 100644 --- a/tests/unit/scm/test_git.py +++ b/tests/unit/scm/test_git.py @@ -6,6 +6,17 @@ from dvc.scm.base import SCMError from tests.basic_env import TestDvcGit +# Behaves the same as SCM but will test against all suported Git backends. +# tmp_dir.scm will still contain a default SCM instance. +@pytest.fixture(params=["gitpython", "dulwich"]) +def git(tmp_dir, scm, request): + from dvc.scm.git import Git + + git_ = Git(os.fspath(tmp_dir), backends=[request.param]) + yield git_ + git_.close() + + class TestGit(TestDvcGit): def test_belongs_to_scm_true_on_gitignore(self): path = os.path.join("path", "to", ".gitignore") @@ -128,36 +139,36 @@ def test_branch_revs(tmp_dir, scm): assert len(expected) == 0 -def test_set_ref(tmp_dir, scm): +def test_set_ref(tmp_dir, git): tmp_dir.scm_gen({"file": "0"}, commit="init") - init_rev = scm.get_rev() + init_rev = tmp_dir.scm.get_rev() tmp_dir.scm_gen({"file": "1"}, commit="commit") - commit_rev = scm.get_rev() + commit_rev = tmp_dir.scm.get_rev() - scm.set_ref("refs/foo/bar", init_rev) + git.set_ref("refs/foo/bar", init_rev) assert ( init_rev == (tmp_dir / ".git" / "refs" / "foo" / "bar").read_text().strip() ) with pytest.raises(SCMError): - scm.set_ref("refs/foo/bar", commit_rev, old_ref=commit_rev) - scm.set_ref("refs/foo/bar", commit_rev, old_ref=init_rev) + git.set_ref("refs/foo/bar", commit_rev, old_ref=commit_rev) + git.set_ref("refs/foo/bar", commit_rev, old_ref=init_rev) assert ( commit_rev == (tmp_dir / ".git" / "refs" / "foo" / "bar").read_text().strip() ) - scm.set_ref("refs/foo/baz", "refs/heads/master", symbolic=True) + git.set_ref("refs/foo/baz", "refs/heads/master", symbolic=True) assert ( "ref: refs/heads/master" == (tmp_dir / ".git" / "refs" / "foo" / "baz").read_text().strip() ) -def test_get_ref(tmp_dir, scm): +def test_get_ref(tmp_dir, git): tmp_dir.scm_gen({"file": "0"}, commit="init") - init_rev = scm.get_rev() + init_rev = tmp_dir.scm.get_rev() tmp_dir.gen( { os.path.join(".git", "refs", "foo", "bar"): init_rev, @@ -167,22 +178,22 @@ def test_get_ref(tmp_dir, scm): } ) - assert init_rev == scm.get_ref("refs/foo/bar") - assert init_rev == scm.get_ref("refs/foo/baz") - assert "refs/heads/master" == scm.get_ref("refs/foo/baz", follow=False) - assert scm.get_ref("refs/foo/qux") is None + assert init_rev == git.get_ref("refs/foo/bar") + assert init_rev == git.get_ref("refs/foo/baz") + assert "refs/heads/master" == git.get_ref("refs/foo/baz", follow=False) + assert git.get_ref("refs/foo/qux") is None -def test_remove_ref(tmp_dir, scm): +def test_remove_ref(tmp_dir, git): tmp_dir.scm_gen({"file": "0"}, commit="init") - init_rev = scm.get_rev() + init_rev = tmp_dir.scm.get_rev() tmp_dir.gen(os.path.join(".git", "refs", "foo", "bar"), init_rev) tmp_dir.scm_gen({"file": "1"}, commit="commit") - commit_rev = scm.get_rev() + commit_rev = tmp_dir.scm.get_rev() with pytest.raises(SCMError): - scm.remove_ref("refs/foo/bar", old_ref=commit_rev) - scm.remove_ref("refs/foo/bar", old_ref=init_rev) + git.remove_ref("refs/foo/bar", old_ref=commit_rev) + git.remove_ref("refs/foo/bar", old_ref=init_rev) assert not (tmp_dir / ".git" / "refs" / "foo" / "bar").exists()