From 7a01d5fe47be00ae6aec1d5091237ae2ac0de6e3 Mon Sep 17 00:00:00 2001 From: pared Date: Thu, 28 Feb 2019 16:15:53 +0100 Subject: [PATCH] add: cleanup .gitignore decorator --- dvc/repo/add.py | 7 +++---- dvc/scm/__init__.py | 19 +++++++++++++++++++ dvc/scm/base.py | 12 ++++++++++++ dvc/scm/git.py | 12 ++++++++++++ tests/test_add.py | 22 +++++++++++++++++++++- tests/test_scm.py | 9 ++++----- tests/utils/__init__.py | 6 ++++++ 7 files changed, 77 insertions(+), 10 deletions(-) diff --git a/dvc/repo/add.py b/dvc/repo/add.py index b85883f6d..be91a9b45 100644 --- a/dvc/repo/add.py +++ b/dvc/repo/add.py @@ -1,9 +1,11 @@ import os + +from dvc.scm import scm_context from dvc.stage import Stage from dvc.utils import walk_files from dvc.exceptions import RecursiveAddingWhileUsingFilename - +@scm_context def add(repo, target, recursive=False, no_commit=False, fname=None): if recursive and fname: raise RecursiveAddingWhileUsingFilename() @@ -25,9 +27,6 @@ def add(repo, target, recursive=False, no_commit=False, fname=None): for stage in stages: stage.dump() - - repo.remind_to_git_add() - return stages diff --git a/dvc/scm/__init__.py b/dvc/scm/__init__.py index 3ebd2e67b..2ae851edb 100644 --- a/dvc/scm/__init__.py +++ b/dvc/scm/__init__.py @@ -5,6 +5,8 @@ from __future__ import unicode_literals from dvc.scm.base import Base from dvc.scm.git import Git +import dvc + def SCM(root_dir, repo=None): # pylint: disable=invalid-name """Returns SCM instance that corresponds to a repo at the specified @@ -21,3 +23,20 @@ def SCM(root_dir, repo=None): # pylint: disable=invalid-name return Git(root_dir, repo=repo) return Base(root_dir, repo=repo) + + +def scm_context(method): + def run(*args, **kw): + repo = args[0] + assert type(repo) is dvc.repo.Repo + scm = repo.scm + try: + result = method(*args, **kw) + scm.reset_ignores() + repo.remind_to_git_add() + return result + except Exception as e: + scm.cleanup_ignores() + raise e + + return run diff --git a/dvc/scm/base.py b/dvc/scm/base.py index 59d082a1e..fba7b2711 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -139,3 +139,15 @@ class Base(object): def install(self): """Adds dvc commands to SCM hooks for the repo.""" + + def cleanup_ignores(self): + """ + This method should clean up ignores (eg. entries in .gitignore), + use, when method editing ignores (eg. add, run, import) fails to + perform its task. + """ + + def reset_ignores(self): + """ + Method to reset in-memory ignore storing mechanism. + """ diff --git a/dvc/scm/git.py b/dvc/scm/git.py index 3e93490cb..96d30dff8 100644 --- a/dvc/scm/git.py +++ b/dvc/scm/git.py @@ -39,6 +39,8 @@ class Git(Base): libpath = env.get("LD_LIBRARY_PATH", None) self.git.git.update_environment(LD_LIBRARY_PATH=libpath) + self.ignored_paths = [] + @staticmethod def is_repo(root_dir): return os.path.isdir(Git._get_git_dir(root_dir)) @@ -98,6 +100,8 @@ class Git(Base): if self.repo is not None: self.repo.files_to_git_add.append(os.path.relpath(gitignore)) + self.ignored_paths.append(path) + def ignore_remove(self, path): entry, gitignore = self._get_gitignore(path) @@ -173,3 +177,11 @@ class Git(Base): def install(self): self._install_hook("post-checkout", "checkout") self._install_hook("pre-commit", "status") + + def cleanup_ignores(self): + for path in self.ignored_paths: + self.ignore_remove(path) + self.reset_ignores() + + def reset_ignores(self): + self.ignored_paths = [] diff --git a/tests/test_add.py b/tests/test_add.py index 447b11d16..cfc9eabcd 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -21,7 +21,7 @@ from dvc.output.base import OutputAlreadyTrackedError from dvc.repo import Repo as DvcRepo from tests.basic_env import TestDvc -from tests.utils import spy +from tests.utils import spy, get_gitignore_content from tests.utils.logger import MockLoggerHandlers, ConsoleFontColorsRemover @@ -457,3 +457,23 @@ class TestAddFilename(TestDvc): self.assertEqual(0, ret) self.assertTrue(os.path.exists("bar.dvc")) self.assertFalse(os.path.exists("foo.dvc")) + + +class TestShouldCleanUpAfterFailedAdd(TestDvc): + def test(self): + ret = main(["add", self.FOO]) + self.assertEqual(0, ret) + + foo_stage_file = self.FOO + Stage.STAGE_FILE_SUFFIX + # corrupt stage file + with open(foo_stage_file, "a+") as file: + file.write("this will break yaml file structure") + + ret = main(["add", self.BAR]) + self.assertEqual(1, ret) + + bar_stage_file = self.BAR + Stage.STAGE_FILE_SUFFIX + self.assertFalse(os.path.exists(bar_stage_file)) + + gitignore_content = get_gitignore_content() + self.assertFalse(any(self.BAR in line for line in gitignore_content)) diff --git a/tests/test_scm.py b/tests/test_scm.py index 7c66cd596..fd2a2a622 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -5,6 +5,7 @@ from git import Repo from dvc.scm import SCM, Base, Git from tests.basic_env import TestDir, TestGit, TestGitSubmodule +from tests.utils import get_gitignore_content class TestSCM(TestDir): @@ -43,11 +44,9 @@ class TestSCMGitSubmodule(TestGitSubmodule): class TestIgnore(TestGit): def _count_gitignore(self): - with open(Git.GITIGNORE, "r") as fd: - lines = fd.readlines() - return len( - list(filter(lambda x: x.strip() == "/" + self.FOO, lines)) - ) + lines = get_gitignore_content() + + return len(list(filter(lambda x: x.strip() == "/" + self.FOO, lines))) def test(self): git = Git(self._root_dir) diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py index f452a1c31..5188d19a2 100644 --- a/tests/utils/__init__.py +++ b/tests/utils/__init__.py @@ -1,3 +1,4 @@ +import yaml import os from dvc.scm import Git @@ -21,6 +22,11 @@ def get_gitignore_content(): return gitignore.readlines() +def load_stage_file(path): + with open(path, "r") as fobj: + return yaml.safe_load(fobj) + + @contextmanager def cd(newdir): prevdir = os.getcwd()