From 82715d53b8d8324ecfd30c03d62a8ae900a29bfe Mon Sep 17 00:00:00 2001 From: Artem Dudarev Date: Thu, 26 Nov 2020 19:31:53 +0200 Subject: [PATCH 1/4] dvc: more explict error during init when .dvc is ignored by git Fixes #3738 --- .gitignore | 1 + dvc/repo/init.py | 11 +++++++++++ dvc/scm/git.py | 4 ++-- tests/func/test_init.py | 14 ++++++++++++++ tests/func/test_scm.py | 4 ++-- 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 6333faa59d..91becd9ad1 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ neatlynx/__pycache__ *.pyc .env/ .env2.7/ +.python-version .dvc.conf.lock .DS_Store diff --git a/dvc/repo/init.py b/dvc/repo/init.py index 2712b2279a..bfeaac6ecf 100644 --- a/dvc/repo/init.py +++ b/dvc/repo/init.py @@ -7,6 +7,7 @@ from dvc.repo import Repo from dvc.scm import SCM from dvc.scm.base import SCMError +from dvc.scm.git import Git from dvc.utils import relpath from dvc.utils.fs import remove @@ -51,6 +52,16 @@ def init(root_dir=os.curdir, no_scm=False, force=False, subdir=False): "repository.".format(repo=root_dir) ) + if isinstance(scm, Git): + if scm.is_ignored(dvc_dir): + raise InitError( + "{dvc_dir} is ignored by Git. \n" + "Make sure that it's tracked, " + "for example, by adding '!.dvc' to .gitignore.".format( + dvc_dir=dvc_dir + ) + ) + if os.path.isdir(dvc_dir): if not force: raise InitError( diff --git a/dvc/scm/git.py b/dvc/scm/git.py index aa93d715bd..7de2736cb6 100644 --- a/dvc/scm/git.py +++ b/dvc/scm/git.py @@ -183,7 +183,7 @@ def _get_gitignore(self, path): return entry, gitignore - def _ignored(self, path): + def is_ignored(self, path): from dulwich import ignore from dulwich.repo import Repo @@ -194,7 +194,7 @@ def _ignored(self, path): def ignore(self, path): entry, gitignore = self._get_gitignore(path) - if self._ignored(path): + if self.is_ignored(path): return msg = "Adding '{}' to '{}'.".format(relpath(path), relpath(gitignore)) diff --git a/tests/func/test_init.py b/tests/func/test_init.py index 4ea9c9101e..c79db84bf6 100644 --- a/tests/func/test_init.py +++ b/tests/func/test_init.py @@ -111,3 +111,17 @@ def test_gen_dvcignore(tmp_dir): "# https://dvc.org/doc/user-guide/dvcignore\n" ) assert text == (tmp_dir / ".dvcignore").read_text() + + +def test_init_when_ignored_by_git(tmp_dir, scm, monkeypatch, caplog): + # https://github.com/iterative/dvc/issues/3738 + tmp_dir.gen({".gitignore": ".*"}) + with caplog.at_level(logging.ERROR, logger="dvc"): + assert main(["init"]) == 1 + assert ( + "{dvc_dir} is ignored by Git. \n" + "Make sure that it's tracked, " + "for example, by adding '!.dvc' to .gitignore.".format( + dvc_dir=tmp_dir / DvcRepo.DVC_DIR + ) + ) in caplog.text diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index 97530ac740..ef9ec1d2d9 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -92,8 +92,8 @@ def test_ignored(tmp_dir, scm): tmp_dir.gen({"dir1": {"file1.jpg": "cont", "file2.txt": "cont"}}) tmp_dir.gen({".gitignore": "dir1/*.jpg"}) - assert scm._ignored(os.fspath(tmp_dir / "dir1" / "file1.jpg")) - assert not scm._ignored(os.fspath(tmp_dir / "dir1" / "file2.txt")) + assert scm.is_ignored(os.fspath(tmp_dir / "dir1" / "file1.jpg")) + assert not scm.is_ignored(os.fspath(tmp_dir / "dir1" / "file2.txt")) def test_get_gitignore(tmp_dir, scm): From e365f3425e5fc1b033dbd3cb22c266cef7e948e5 Mon Sep 17 00:00:00 2001 From: Artem Dudarev Date: Fri, 27 Nov 2020 15:50:20 +0200 Subject: [PATCH 2/4] dvc: address PR feedback --- dvc/repo/init.py | 23 +++++++++-------------- dvc/scm/base.py | 4 ++++ tests/func/test_init.py | 4 ++-- tests/func/test_scm.py | 4 ++-- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/dvc/repo/init.py b/dvc/repo/init.py index bfeaac6ecf..f46d0590bf 100644 --- a/dvc/repo/init.py +++ b/dvc/repo/init.py @@ -46,28 +46,23 @@ def init(root_dir=os.curdir, no_scm=False, force=False, subdir=False): scm = SCM(root_dir, search_parent_directories=subdir, no_scm=no_scm) except SCMError: raise InitError( - "{repo} is not tracked by any supported SCM tool (e.g. Git). " + f"{root_dir} is not tracked by any supported SCM tool (e.g. Git). " "Use `--no-scm` if you don't want to use any SCM or " "`--subdir` if initializing inside a subdirectory of a parent SCM " - "repository.".format(repo=root_dir) + "repository." ) - if isinstance(scm, Git): - if scm.is_ignored(dvc_dir): - raise InitError( - "{dvc_dir} is ignored by Git. \n" - "Make sure that it's tracked, " - "for example, by adding '!.dvc' to .gitignore.".format( - dvc_dir=dvc_dir - ) - ) + if scm.is_ignored(dvc_dir): + raise InitError( + f"{dvc_dir} is ignored by your SCM tool. \n" + "Make sure that it's tracked, " + "for example, by adding '!.dvc' to .gitignore." + ) if os.path.isdir(dvc_dir): if not force: raise InitError( - "'{repo}' exists. Use `-f` to force.".format( - repo=relpath(dvc_dir) - ) + f"'{relpath(dvc_dir)}' exists. Use `-f` to force." ) remove(dvc_dir) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index d37abbcd98..d50562942d 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -66,6 +66,10 @@ def is_submodule(root_dir): # pylint: disable=unused-argument """ return True + def is_ignored(self, path): + """Returns whether or not path is ignored by SCM.""" + return False + def ignore(self, path): # pylint: disable=unused-argument """Makes SCM ignore a specified path.""" diff --git a/tests/func/test_init.py b/tests/func/test_init.py index c79db84bf6..79f7dc4abb 100644 --- a/tests/func/test_init.py +++ b/tests/func/test_init.py @@ -113,13 +113,13 @@ def test_gen_dvcignore(tmp_dir): assert text == (tmp_dir / ".dvcignore").read_text() -def test_init_when_ignored_by_git(tmp_dir, scm, monkeypatch, caplog): +def test_init_when_ignored_by_git(tmp_dir, scm, caplog): # https://github.com/iterative/dvc/issues/3738 tmp_dir.gen({".gitignore": ".*"}) with caplog.at_level(logging.ERROR, logger="dvc"): assert main(["init"]) == 1 assert ( - "{dvc_dir} is ignored by Git. \n" + "{dvc_dir} is ignored by your SCM tool. \n" "Make sure that it's tracked, " "for example, by adding '!.dvc' to .gitignore.".format( dvc_dir=tmp_dir / DvcRepo.DVC_DIR diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index ef9ec1d2d9..bdfb5a9719 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -92,8 +92,8 @@ def test_ignored(tmp_dir, scm): tmp_dir.gen({"dir1": {"file1.jpg": "cont", "file2.txt": "cont"}}) tmp_dir.gen({".gitignore": "dir1/*.jpg"}) - assert scm.is_ignored(os.fspath(tmp_dir / "dir1" / "file1.jpg")) - assert not scm.is_ignored(os.fspath(tmp_dir / "dir1" / "file2.txt")) + assert scm.is_ignored(tmp_dir / "dir1" / "file1.jpg") + assert not scm.is_ignored(tmp_dir / "dir1" / "file2.txt") def test_get_gitignore(tmp_dir, scm): From 39a0f67c835fdb738eb43aa696a151ee0320b9ff Mon Sep 17 00:00:00 2001 From: Artem Dudarev Date: Fri, 27 Nov 2020 16:15:54 +0200 Subject: [PATCH 3/4] styling --- dvc/repo/init.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dvc/repo/init.py b/dvc/repo/init.py index f46d0590bf..87b5549ecc 100644 --- a/dvc/repo/init.py +++ b/dvc/repo/init.py @@ -7,7 +7,6 @@ from dvc.repo import Repo from dvc.scm import SCM from dvc.scm.base import SCMError -from dvc.scm.git import Git from dvc.utils import relpath from dvc.utils.fs import remove @@ -61,9 +60,7 @@ def init(root_dir=os.curdir, no_scm=False, force=False, subdir=False): if os.path.isdir(dvc_dir): if not force: - raise InitError( - f"'{relpath(dvc_dir)}' exists. Use `-f` to force." - ) + raise InitError(f"'{relpath(dvc_dir)}' exists. Use `-f` to force.") remove(dvc_dir) From c336fe124a0ff060c7c47c22155d70127b826daa Mon Sep 17 00:00:00 2001 From: Artem Dudarev Date: Fri, 27 Nov 2020 16:22:22 +0200 Subject: [PATCH 4/4] styling --- dvc/scm/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index d50562942d..3a7c3f0670 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -66,7 +66,7 @@ def is_submodule(root_dir): # pylint: disable=unused-argument """ return True - def is_ignored(self, path): + def is_ignored(self, path): # pylint: disable=unused-argument """Returns whether or not path is ignored by SCM.""" return False