diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index dd3e1f314..62dfa5dbb 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -104,6 +104,16 @@ class Git(Base): return entry, gitignore + @staticmethod + def _ignored(entry, gitignore_path): + if os.path.exists(gitignore_path): + with open(gitignore_path, "r") as fobj: + ignore_list = fobj.readlines() + return any( + filter(lambda x: x.strip() == entry.strip(), ignore_list) + ) + return False + def ignore(self, path, in_curr_dir=False): base_dir = ( os.path.realpath(os.curdir) @@ -112,31 +122,32 @@ class Git(Base): ) entry, gitignore = self._get_gitignore(path, base_dir) - ignore_list = [] - if os.path.exists(gitignore): - with open(gitignore, "r") as f: - ignore_list = f.readlines() - if any(filter(lambda x: x.strip() == entry.strip(), ignore_list)): - return + if self._ignored(entry, gitignore): + return msg = "Adding '{}' to '{}'.".format( os.path.relpath(path), os.path.relpath(gitignore) ) logger.info(msg) - self._add_entry_to_gitignore(entry, gitignore, ignore_list) + self._add_entry_to_gitignore(entry, gitignore) self.track_file(os.path.relpath(gitignore)) self.ignored_paths.append(path) @staticmethod - def _add_entry_to_gitignore(entry, gitignore, ignore_list): - content = entry - if ignore_list: - content = "\n" + content - with open(gitignore, "a", encoding="utf-8") as fobj: - fobj.write(content) + def _add_entry_to_gitignore(entry, gitignore): + with open(gitignore, "a+", encoding="utf-8") as fobj: + fobj.seek(0, os.SEEK_END) + if fobj.tell() == 0: + # Empty file + prefix = "" + else: + fobj.seek(fobj.tell() - 1, os.SEEK_SET) + last = fobj.read(1) + prefix = "" if last == "\n" else "\n" + fobj.write("{}{}\n".format(prefix, entry)) def ignore_remove(self, path): entry, gitignore = self._get_gitignore(path) diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index 885b9c613..8135f6606 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals import os +import pytest from git import Repo from dvc.utils.compat import str # noqa: F401 @@ -61,80 +62,133 @@ class TestSCMGitSubmodule(TestGitSubmodule): self.assertTrue("foo" in self.git.git.ls_files()) -class TestIgnore(TestGit): - def _count_gitignore(self): +class TestIgnore(object): + @staticmethod + def _count_gitignore_entries(string): lines = get_gitignore_content() - return len([i for i in lines if i == "/" + self.FOO]) + return len([i for i in lines if i == string]) - def test_ignore(self): - git = Git(self._root_dir) - foo = os.path.join(self._root_dir, self.FOO) + def test_ignore(self, git, repo_dir): + git = Git(repo_dir._root_dir) + foo = os.path.join(repo_dir._root_dir, repo_dir.FOO) + + target = "/" + repo_dir.FOO git.ignore(foo) - self.assertTrue(os.path.isfile(Git.GITIGNORE)) - self.assertEqual(self._count_gitignore(), 1) + assert os.path.isfile(Git.GITIGNORE) + assert self._count_gitignore_entries(target) == 1 git.ignore(foo) - self.assertEqual(self._count_gitignore(), 1) + assert os.path.isfile(Git.GITIGNORE) + assert self._count_gitignore_entries(target) == 1 git.ignore_remove(foo) - self.assertEqual(self._count_gitignore(), 0) + assert self._count_gitignore_entries(target) == 0 - def test_get_gitignore(self): - data_dir = os.path.join(self._root_dir, "file1") - entry, gitignore = Git(self._root_dir)._get_gitignore(data_dir) - self.assertEqual(entry, "/file1") - self.assertEqual( - gitignore, os.path.join(self._root_dir, Git.GITIGNORE) + def test_get_gitignore(self, git, repo_dir): + data_dir = os.path.join(repo_dir._root_dir, "file1") + entry, gitignore = Git(repo_dir._root_dir)._get_gitignore(data_dir) + assert entry == "/file1" + assert gitignore == os.path.join(repo_dir._root_dir, Git.GITIGNORE) + + data_dir = os.path.join(repo_dir._root_dir, "dir") + entry, gitignore = Git(repo_dir._root_dir)._get_gitignore(data_dir) + + assert entry == "/dir" + assert gitignore == os.path.join(repo_dir._root_dir, Git.GITIGNORE) + + def test_get_gitignore_subdir(self, git, repo_dir): + data_dir = os.path.join( + repo_dir._root_dir, os.path.join("dir1", "file1") + ) + entry, gitignore = Git(repo_dir._root_dir)._get_gitignore(data_dir) + assert entry == "/file1" + assert gitignore == os.path.join( + repo_dir._root_dir, "dir1", Git.GITIGNORE ) - data_dir = os.path.join(self._root_dir, "dir") - entry, gitignore = Git(self._root_dir)._get_gitignore(data_dir) - self.assertEqual(entry, "/dir") - self.assertEqual( - gitignore, os.path.join(self._root_dir, Git.GITIGNORE) + data_dir = os.path.join( + repo_dir._root_dir, os.path.join("dir1", "dir2") + ) + entry, gitignore = Git(repo_dir._root_dir)._get_gitignore(data_dir) + assert entry == "/dir2" + assert gitignore == os.path.join( + repo_dir._root_dir, "dir1", Git.GITIGNORE ) - def test_get_gitignore_subdir(self): - data_dir = os.path.join(self._root_dir, os.path.join("dir1", "file1")) - entry, gitignore = Git(self._root_dir)._get_gitignore(data_dir) - self.assertEqual(entry, "/file1") - self.assertEqual( - gitignore, os.path.join(self._root_dir, "dir1", Git.GITIGNORE) - ) - - data_dir = os.path.join(self._root_dir, os.path.join("dir1", "dir2")) - entry, gitignore = Git(self._root_dir)._get_gitignore(data_dir) - self.assertEqual(entry, "/dir2") - self.assertEqual( - gitignore, os.path.join(self._root_dir, "dir1", Git.GITIGNORE) - ) - - def test_get_gitignore_ignorefile_dir(self): - git = Git(self._root_dir) + def test_get_gitignore_ignorefile_dir(self, git, repo_dir): + git = Git(repo_dir._root_dir) file_double_dir = os.path.join("dir1", "dir2", "file1") - data_dir1 = os.path.join(self._root_dir, file_double_dir) + data_dir1 = os.path.join(repo_dir._root_dir, file_double_dir) dir1_real1 = os.path.realpath("dir1") entry, gitignore = git._get_gitignore(data_dir1, dir1_real1) - self.assertEqual(entry, "/dir2/file1") - gitignore1 = os.path.join(self._root_dir, "dir1", Git.GITIGNORE) - self.assertEqual(gitignore, gitignore1) + assert entry == "/dir2/file1" + gitignore1 = os.path.join(repo_dir._root_dir, "dir1", Git.GITIGNORE) + assert gitignore == gitignore1 triple_dir = os.path.join("dir1", "dir2", "dir3") - data_dir2 = os.path.join(self._root_dir, triple_dir) + data_dir2 = os.path.join(repo_dir._root_dir, triple_dir) dir1_real2 = os.path.realpath("dir1") entry, gitignore = git._get_gitignore(data_dir2, dir1_real2) - self.assertEqual(entry, "/dir2/dir3") - gitignore2 = os.path.join(self._root_dir, "dir1", Git.GITIGNORE) - self.assertEqual(gitignore, gitignore2) + assert entry == "/dir2/dir3" + gitignore2 = os.path.join(repo_dir._root_dir, "dir1", Git.GITIGNORE) + assert gitignore == gitignore2 - def test_get_gitignore_ignorefile_dir_upper_level(self): - git = Git(self._root_dir) + def test_get_gitignore_ignorefile_dir_upper_level(self, git, repo_dir): + git = Git(repo_dir._root_dir) file_double_dir = os.path.join("dir1", "dir2", "file1") - data_dir1 = os.path.join(self._root_dir, file_double_dir) + data_dir1 = os.path.join(repo_dir._root_dir, file_double_dir) ignore_file_dir = os.path.realpath(os.path.join("aa", "bb")) - with self.assertRaises(FileNotInTargetSubdirError): + with pytest.raises(FileNotInTargetSubdirError): git._get_gitignore(data_dir1, ignore_file_dir) + + def test_gitignore_should_end_with_newline(self, git, repo_dir): + git = Git(repo_dir._root_dir) + + foo = os.path.join(repo_dir._root_dir, repo_dir.FOO) + bar = os.path.join(repo_dir._root_dir, repo_dir.BAR) + gitignore = os.path.join(repo_dir._root_dir, Git.GITIGNORE) + + git.ignore(foo) + + with open(gitignore, "r") as fobj: + last = fobj.readlines()[-1] + + assert last.endswith("\n") + + git.ignore(bar) + + with open(gitignore, "r") as fobj: + last = fobj.readlines()[-1] + + assert last.endswith("\n") + + def test_gitignore_should_append_newline_to_gitignore(self, git, repo_dir): + git = Git(repo_dir._root_dir) + + foo_ignore_pattern = "/foo" + bar_ignore_pattern = "/bar" + bar_path = os.path.join(repo_dir._root_dir, repo_dir.BAR) + gitignore = os.path.join(repo_dir._root_dir, Git.GITIGNORE) + + with open(gitignore, "w") as fobj: + fobj.write(foo_ignore_pattern) + + with open(gitignore, "r") as fobj: + last = fobj.readlines()[-1] + assert not last.endswith("\n") + + git.ignore(bar_path) + + with open(gitignore, "r") as fobj: + lines = list(fobj.readlines()) + + assert len(lines) == 2 + for l in lines: + assert l.endswith("\n") + + assert lines[0].strip() == foo_ignore_pattern + assert lines[1].strip() == bar_ignore_pattern