From 8a76742d3b6a806f39d1ba9be0f562b37986111d Mon Sep 17 00:00:00 2001 From: Aljoscha Steffens Date: Thu, 6 Feb 2020 11:20:49 +0000 Subject: [PATCH 1/6] made '_ignored' check if the given file is already ignored by the root .gitignore. Added corresponding functional test --- dvc/scm/git/__init__.py | 14 ++++++++++++-- tests/func/test_scm.py | 8 ++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 79fbdb3606..8421ddb10f 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -133,8 +133,18 @@ def _get_gitignore(self, path): return entry, gitignore - @staticmethod - def _ignored(entry, gitignore_path): + def _ignored(self, entry, gitignore_path): + + # We want to check first if `entry` is already being ignored by the .gitignore file in the root dir. + entry_path = os.path.join(os.path.dirname(gitignore_path), entry) + from git.exc import GitCommandError + try: + self.repo.git.check_ignore(entry_path) + return True + except GitCommandError: + # If none of the paths passed to `check_ignore` are ignored, GitPython annoyingly raises an Exception + pass + if os.path.exists(gitignore_path): with open(gitignore_path, "r") as fobj: ignore_list = fobj.readlines() diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index 23d303e1e0..f01ff40058 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -83,6 +83,14 @@ def test_ignore(tmp_dir, scm): assert _count_gitignore_entries(target) == 0 +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("file1.jpg", fspath(tmp_dir / "dir1" / ".gitignore")) + assert not scm._ignored("file2.txt", fspath(tmp_dir / "dir1" / ".gitignore")) + + def test_get_gitignore(tmp_dir, scm): tmp_dir.gen({"file1": "contents", "dir": {}}) From 4d772fea9674fe81c112d53368f0c73dbe8feea4 Mon Sep 17 00:00:00 2001 From: Aljoscha Steffens Date: Thu, 6 Feb 2020 13:05:43 +0000 Subject: [PATCH 2/6] code formatting --- dvc/scm/git/__init__.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 8421ddb10f..cf06a82394 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -18,7 +18,6 @@ from dvc.utils import relpath from dvc.utils.fs import path_isin - logger = logging.getLogger(__name__) @@ -135,23 +134,22 @@ def _get_gitignore(self, path): def _ignored(self, entry, gitignore_path): - # We want to check first if `entry` is already being ignored by the .gitignore file in the root dir. + # We want to check first if `entry` is already being ignored + # by the .gitignore file in the root dir. + + entry = ( + entry[1:] if entry[0] == "/" else entry + ) # make sure that joining the paths works entry_path = os.path.join(os.path.dirname(gitignore_path), entry) from git.exc import GitCommandError + try: self.repo.git.check_ignore(entry_path) return True except GitCommandError: - # If none of the paths passed to `check_ignore` are ignored, GitPython annoyingly raises an Exception - pass - - 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 + # If none of the paths passed to `check_ignore` are ignored, + # GitPython annoyingly raises an Exception + return False def ignore(self, path): entry, gitignore = self._get_gitignore(path) From fd75bbdb47fc4225b98b3bdabcf604bc7201b91f Mon Sep 17 00:00:00 2001 From: Aljoscha Steffens Date: Thu, 6 Feb 2020 16:18:41 +0000 Subject: [PATCH 3/6] Changed method input, adjusted tests, formatted code --- dvc/scm/git/__init__.py | 11 +++-------- tests/func/test_scm.py | 4 ++-- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index cf06a82394..c95ed3b794 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -132,19 +132,14 @@ def _get_gitignore(self, path): return entry, gitignore - def _ignored(self, entry, gitignore_path): + def _ignored(self, path): # We want to check first if `entry` is already being ignored # by the .gitignore file in the root dir. - - entry = ( - entry[1:] if entry[0] == "/" else entry - ) # make sure that joining the paths works - entry_path = os.path.join(os.path.dirname(gitignore_path), entry) from git.exc import GitCommandError try: - self.repo.git.check_ignore(entry_path) + self.repo.git.check_ignore(path) return True except GitCommandError: # If none of the paths passed to `check_ignore` are ignored, @@ -154,7 +149,7 @@ def _ignored(self, entry, gitignore_path): def ignore(self, path): entry, gitignore = self._get_gitignore(path) - if self._ignored(entry, gitignore): + if self._ignored(path): return msg = "Adding '{}' to '{}'.".format(relpath(path), relpath(gitignore)) diff --git a/tests/func/test_scm.py b/tests/func/test_scm.py index f01ff40058..185c8b60cd 100644 --- a/tests/func/test_scm.py +++ b/tests/func/test_scm.py @@ -87,8 +87,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("file1.jpg", fspath(tmp_dir / "dir1" / ".gitignore")) - assert not scm._ignored("file2.txt", fspath(tmp_dir / "dir1" / ".gitignore")) + assert scm._ignored(fspath(tmp_dir / "dir1" / "file1.jpg")) + assert not scm._ignored(fspath(tmp_dir / "dir1" / "file2.txt")) def test_get_gitignore(tmp_dir, scm): From 72919eec1ec53ab2c771e914e76a3af9d4d78909 Mon Sep 17 00:00:00 2001 From: Aljoscha Steffens Date: Thu, 6 Feb 2020 16:20:17 +0000 Subject: [PATCH 4/6] typo --- dvc/scm/git/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index c95ed3b794..31a5f3da8b 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -134,7 +134,7 @@ def _get_gitignore(self, path): def _ignored(self, path): - # We want to check first if `entry` is already being ignored + # We want to check first if `path` is already being ignored # by the .gitignore file in the root dir. from git.exc import GitCommandError From ea61fa60fffd4960d4739b6e22447c14e65a6028 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Feb 2020 19:28:54 +0200 Subject: [PATCH 5/6] tests: use git logger fix in conftest See https://github.com/iterative/dvc/issues/3167 --- tests/dir_helpers.py | 6 ++++++ tests/func/test_get.py | 6 +----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/dir_helpers.py b/tests/dir_helpers.py index 4fc44e2110..19580bbf68 100644 --- a/tests/dir_helpers.py +++ b/tests/dir_helpers.py @@ -45,6 +45,7 @@ import os import pathlib +import logging from contextlib import contextmanager import pytest @@ -65,6 +66,11 @@ ] +# see https://github.com/iterative/dvc/issues/3167 +git_logger = logging.getLogger("git") +git_logger.setLevel(logging.CRITICAL) + + class TmpDir(pathlib.Path): def __new__(cls, *args, **kwargs): if cls is TmpDir: diff --git a/tests/func/test_get.py b/tests/func/test_get.py index 45f8393551..aa6f576425 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -163,11 +163,7 @@ def test_get_from_non_dvc_master(tmp_dir, git_dir, caplog): caplog.clear() - # removing `git` import in conftest resulted in unexpected logs from - # that package, see https://github.com/iterative/dvc/issues/3167 - with caplog.at_level(logging.INFO, logger="git"), caplog.at_level( - logging.INFO, logger="dvc" - ): + with caplog.at_level(logging.INFO, logger="dvc"): Repo.get(fspath(git_dir), "some_file", out="some_dst", rev="branch") assert caplog.text == "" From 468daec28dde94faa53d6451b46db43b07855004 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Feb 2020 19:30:11 +0200 Subject: [PATCH 6/6] scm: git: remove redundant comments --- dvc/scm/git/__init__.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 31a5f3da8b..ac63854ffa 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -133,17 +133,12 @@ def _get_gitignore(self, path): return entry, gitignore def _ignored(self, path): - - # We want to check first if `path` is already being ignored - # by the .gitignore file in the root dir. from git.exc import GitCommandError try: self.repo.git.check_ignore(path) return True except GitCommandError: - # If none of the paths passed to `check_ignore` are ignored, - # GitPython annoyingly raises an Exception return False def ignore(self, path):