From f5d68c1ad48f3684011e0e86187240e50a2d6a16 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 11 Sep 2022 09:08:08 +0530 Subject: [PATCH 01/20] dvcignore: read ignore patterns from system level .dvcignore Read the patterns from system-level .dvcignore and include these as part of the default ignore pattern used while initiating the ignore trie Fixes #8261 --- dvc/ignore.py | 21 +++++++++++++++++---- tests/func/test_check_ignore.py | 23 +++++++++++++++++++++++ tests/func/test_ignore.py | 16 ++++++++++++++++ tests/unit/test_ignore.py | 18 ++++++++++++++++++ 4 files changed, 74 insertions(+), 4 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index b26d62ff12..10a2ef60bc 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -3,6 +3,7 @@ import re from collections import namedtuple from itertools import chain, groupby, takewhile +from pathlib import Path from pathspec.patterns import GitWildMatchPattern from pathspec.util import normalize_file @@ -25,10 +26,10 @@ def __call__(self, root, dirs, files): class DvcIgnorePatterns(DvcIgnore): def __init__(self, pattern_list, dirname, sep): if pattern_list: - if isinstance(pattern_list[0], str): - pattern_list = [ - PatternInfo(pattern, "") for pattern in pattern_list - ] + pattern_list = [ + PatternInfo(pattern, "") if isinstance(pattern, str) else pattern + for pattern in pattern_list + ] self.sep = sep self.pattern_list = pattern_list @@ -174,12 +175,24 @@ def __init__(self, fs, root_dir): self._ignores_trie_subrepos = Trie() key = self._get_key(root_dir) + + sys_root_dvcignore_path = self.fs.path.join(str(Path.home()), DvcIgnore.DVCIGNORE_FILE) + + if (self.fs.exists(sys_root_dvcignore_path)): + sys_root_ignore_patterns = DvcIgnorePatterns.from_file( + sys_root_dvcignore_path, + self.fs, + "sys_root" + ) + default_ignore_patterns.extend(sys_root_ignore_patterns.pattern_list) + self.ignores_trie_fs[key] = DvcIgnorePatterns( default_ignore_patterns, root_dir, fs.sep, ) self._ignores_trie_subrepos[key] = self.ignores_trie_fs[key] + self._update( self.root_dir, self._ignores_trie_subrepos, diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 6e377af494..5e9d4aec8d 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -1,4 +1,5 @@ import os +from pathlib import Path import pytest @@ -97,6 +98,28 @@ def test_check_ignore_sub_repo(tmp_dir, dvc, capsys): "in sub_repo:{}\t{}".format("dir", os.path.join("dir", "foo")) in out ) +def test_check_sys_root_ignore_file(tmp_dir, dvc, capsys): + tmp_dir.gen({ "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_sys_root": "ignored_in_sys_root" + }}) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_sys_root", + encoding="utf-8" + ) + + assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 + output, _ = capsys.readouterr() + assert ( + output == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\tignored_in_repo_root\n" + ) + + assert main(["check-ignore", "-d", "ignored_in_sys_root"]) == 0 + output, _ = capsys.readouterr() + assert ( + output == "sys_root:1:ignored_in_sys_root\tignored_in_sys_root\n" + ) def test_check_sub_dir_ignore_file(tmp_dir, dvc, capsys): tmp_dir.gen( diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index fd374ca639..b55b40b1cc 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -35,6 +35,22 @@ def test_ignore(tmp_dir, dvc, filename): (tmp_dir / "dir" / "other").fs_path, } +def test_ignore_from_sys_root_dvcignore(tmp_dir, dvc): + tmp_dir.gen({ "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_sys_root": "ignored_in_sys_root" + }}) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_sys_root", + encoding="utf-8" + ) + + result = set(walk_files(dvc, dvc.fs, tmp_dir)) + + for ignored_file in ["ignored_in_repo_root", "ignored_in_sys_root"]: + assert (tmp_dir / "dir" / ignored_file).fs_path in result + def test_rename_ignored_file(tmp_dir, dvc): tmp_dir.gen({"dir": {"ignored": "...", "other": "text"}}) diff --git a/tests/unit/test_ignore.py b/tests/unit/test_ignore.py index c5f4b5a3da..1124a7607d 100644 --- a/tests/unit/test_ignore.py +++ b/tests/unit/test_ignore.py @@ -3,6 +3,7 @@ import pytest +from dvc.pathspec_math import PatternInfo from dvc.ignore import DvcIgnorePatterns @@ -213,3 +214,20 @@ def test_should_ignore_dir(omit_dir, sub_dir): assert set(new_dirs) == {"dir1", "dir2"} assert set(new_files) == {"file1", "file2", omit_dir} + +def test_allow_string_pattern_info_mix_input(): + pattern1 = DvcIgnorePatterns([ + "pattern1_string", + PatternInfo("pattern1_info", "pattern1_info_pattern_info") + ], "root", os.sep) + + assert PatternInfo("pattern1_string", "") in pattern1.pattern_list + assert PatternInfo("pattern1_info", "pattern1_info_pattern_info") in pattern1.pattern_list + + pattern2 = DvcIgnorePatterns([ + PatternInfo("pattern2_info", "pattern2_info_pattern_info"), + "pattern2_string" + ], "root", os.sep) + + assert PatternInfo("pattern2_string", "") in pattern2.pattern_list + assert PatternInfo("pattern2_info", "pattern2_info_pattern_info") in pattern2.pattern_list From fb4bfff8b39c26e7131272baa10893563c0317ae Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 11 Sep 2022 03:49:06 +0000 Subject: [PATCH 02/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/ignore.py | 18 +++++++++------- tests/func/test_check_ignore.py | 24 ++++++++++++--------- tests/func/test_ignore.py | 16 ++++++++------ tests/unit/test_ignore.py | 37 +++++++++++++++++++++++---------- 4 files changed, 61 insertions(+), 34 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index 10a2ef60bc..fbb0dafa4a 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -27,7 +27,9 @@ class DvcIgnorePatterns(DvcIgnore): def __init__(self, pattern_list, dirname, sep): if pattern_list: pattern_list = [ - PatternInfo(pattern, "") if isinstance(pattern, str) else pattern + PatternInfo(pattern, "") + if isinstance(pattern, str) + else pattern for pattern in pattern_list ] @@ -176,15 +178,17 @@ def __init__(self, fs, root_dir): key = self._get_key(root_dir) - sys_root_dvcignore_path = self.fs.path.join(str(Path.home()), DvcIgnore.DVCIGNORE_FILE) + sys_root_dvcignore_path = self.fs.path.join( + str(Path.home()), DvcIgnore.DVCIGNORE_FILE + ) - if (self.fs.exists(sys_root_dvcignore_path)): + if self.fs.exists(sys_root_dvcignore_path): sys_root_ignore_patterns = DvcIgnorePatterns.from_file( - sys_root_dvcignore_path, - self.fs, - "sys_root" + sys_root_dvcignore_path, self.fs, "sys_root" + ) + default_ignore_patterns.extend( + sys_root_ignore_patterns.pattern_list ) - default_ignore_patterns.extend(sys_root_ignore_patterns.pattern_list) self.ignores_trie_fs[key] = DvcIgnorePatterns( default_ignore_patterns, diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 5e9d4aec8d..5dadbd0833 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -98,28 +98,32 @@ def test_check_ignore_sub_repo(tmp_dir, dvc, capsys): "in sub_repo:{}\t{}".format("dir", os.path.join("dir", "foo")) in out ) + def test_check_sys_root_ignore_file(tmp_dir, dvc, capsys): - tmp_dir.gen({ "dir": { - "ignored_in_repo_root": "ignored_in_repo_root", - "ignored_in_sys_root": "ignored_in_sys_root" - }}) + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_sys_root": "ignored_in_sys_root", + } + } + ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_sys_root", - encoding="utf-8" + "ignored_in_sys_root", encoding="utf-8" ) assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 output, _ = capsys.readouterr() assert ( - output == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\tignored_in_repo_root\n" + output + == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\tignored_in_repo_root\n" ) assert main(["check-ignore", "-d", "ignored_in_sys_root"]) == 0 output, _ = capsys.readouterr() - assert ( - output == "sys_root:1:ignored_in_sys_root\tignored_in_sys_root\n" - ) + assert output == "sys_root:1:ignored_in_sys_root\tignored_in_sys_root\n" + def test_check_sub_dir_ignore_file(tmp_dir, dvc, capsys): tmp_dir.gen( diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index b55b40b1cc..e26f3cf891 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -35,15 +35,19 @@ def test_ignore(tmp_dir, dvc, filename): (tmp_dir / "dir" / "other").fs_path, } + def test_ignore_from_sys_root_dvcignore(tmp_dir, dvc): - tmp_dir.gen({ "dir": { - "ignored_in_repo_root": "ignored_in_repo_root", - "ignored_in_sys_root": "ignored_in_sys_root" - }}) + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_sys_root": "ignored_in_sys_root", + } + } + ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_sys_root", - encoding="utf-8" + "ignored_in_sys_root", encoding="utf-8" ) result = set(walk_files(dvc, dvc.fs, tmp_dir)) diff --git a/tests/unit/test_ignore.py b/tests/unit/test_ignore.py index 1124a7607d..c3663f8d01 100644 --- a/tests/unit/test_ignore.py +++ b/tests/unit/test_ignore.py @@ -3,8 +3,8 @@ import pytest -from dvc.pathspec_math import PatternInfo from dvc.ignore import DvcIgnorePatterns +from dvc.pathspec_math import PatternInfo def mock_dvcignore(dvcignore_path, patterns): @@ -215,19 +215,34 @@ def test_should_ignore_dir(omit_dir, sub_dir): assert set(new_dirs) == {"dir1", "dir2"} assert set(new_files) == {"file1", "file2", omit_dir} + def test_allow_string_pattern_info_mix_input(): - pattern1 = DvcIgnorePatterns([ - "pattern1_string", - PatternInfo("pattern1_info", "pattern1_info_pattern_info") - ], "root", os.sep) + pattern1 = DvcIgnorePatterns( + [ + "pattern1_string", + PatternInfo("pattern1_info", "pattern1_info_pattern_info"), + ], + "root", + os.sep, + ) assert PatternInfo("pattern1_string", "") in pattern1.pattern_list - assert PatternInfo("pattern1_info", "pattern1_info_pattern_info") in pattern1.pattern_list + assert ( + PatternInfo("pattern1_info", "pattern1_info_pattern_info") + in pattern1.pattern_list + ) - pattern2 = DvcIgnorePatterns([ - PatternInfo("pattern2_info", "pattern2_info_pattern_info"), - "pattern2_string" - ], "root", os.sep) + pattern2 = DvcIgnorePatterns( + [ + PatternInfo("pattern2_info", "pattern2_info_pattern_info"), + "pattern2_string", + ], + "root", + os.sep, + ) assert PatternInfo("pattern2_string", "") in pattern2.pattern_list - assert PatternInfo("pattern2_info", "pattern2_info_pattern_info") in pattern2.pattern_list + assert ( + PatternInfo("pattern2_info", "pattern2_info_pattern_info") + in pattern2.pattern_list + ) From 730a75598807b45815346936b3e2e47ab99720ef Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 11 Sep 2022 10:47:23 +0530 Subject: [PATCH 03/20] fix lint --- tests/func/test_check_ignore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 5dadbd0833..d22d4ecc28 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -117,7 +117,8 @@ def test_check_sys_root_ignore_file(tmp_dir, dvc, capsys): output, _ = capsys.readouterr() assert ( output - == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\tignored_in_repo_root\n" + == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\t" + + "ignored_in_repo_root\n" ) assert main(["check-ignore", "-d", "ignored_in_sys_root"]) == 0 From 277c7bc2478aff26154600a3693e3f93c829cae3 Mon Sep 17 00:00:00 2001 From: meshde Date: Thu, 15 Sep 2022 15:06:56 +0530 Subject: [PATCH 04/20] dvcignore: allow reading patterns from core.excludesfile --- dvc/config_schema.py | 1 + dvc/ignore.py | 28 ++++++-- tests/func/test_check_ignore.py | 87 ++++++++++++++++++++++-- tests/func/test_ignore.py | 116 ++++++++++++++++++++++++++++++-- 4 files changed, 213 insertions(+), 19 deletions(-) diff --git a/dvc/config_schema.py b/dvc/config_schema.py index 13590cf97f..e87eee784d 100644 --- a/dvc/config_schema.py +++ b/dvc/config_schema.py @@ -123,6 +123,7 @@ class RelPath(str): Optional("experiments"): Bool, # obsoleted Optional("check_update", default=True): Bool, "machine": Lower, + "excludesfile": str, }, "cache": { "local": str, diff --git a/dvc/ignore.py b/dvc/ignore.py index fbb0dafa4a..a15a5cbf83 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -10,6 +10,7 @@ from pygtrie import Trie from dvc.fs import AnyFSPath, FileSystem, Schemes, localfs +from dvc.config import Config from dvc.pathspec_math import PatternInfo, merge_patterns from dvc.types import List, Optional @@ -175,21 +176,34 @@ def __init__(self, fs, root_dir): self.root_dir = root_dir self.ignores_trie_fs = Trie() self._ignores_trie_subrepos = Trie() + self.config = Config() key = self._get_key(root_dir) - sys_root_dvcignore_path = self.fs.path.join( - str(Path.home()), DvcIgnore.DVCIGNORE_FILE - ) + core_config = self.config.get("core", {}) + config_ignore_file = core_config.get("excludesfile", None) - if self.fs.exists(sys_root_dvcignore_path): - sys_root_ignore_patterns = DvcIgnorePatterns.from_file( - sys_root_dvcignore_path, self.fs, "sys_root" + def extend_default_ignore_patterns(ignore_file_path): + ignore_patterns = DvcIgnorePatterns.from_file( + ignore_file_path, self.fs, ignore_file_path ) default_ignore_patterns.extend( - sys_root_ignore_patterns.pattern_list + ignore_patterns.pattern_list ) + if config_ignore_file: + if self.fs.exists(config_ignore_file): + extend_default_ignore_patterns(config_ignore_file) + else: + for level in ["global", "system"]: + ignore_file_path_at_level = self.fs.path.join( + Config.get_dir(level), + DvcIgnore.DVCIGNORE_FILE + ) + if self.fs.exists(ignore_file_path_at_level): + extend_default_ignore_patterns(ignore_file_path_at_level) + break + self.ignores_trie_fs[key] = DvcIgnorePatterns( default_ignore_patterns, root_dir, diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index d22d4ecc28..8b95c49a39 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -7,6 +7,22 @@ from dvc.ignore import DvcIgnore +def _get_tmp_config_dir(tmp_path, level): + return tmp_path / ".." / (tmp_path.name + "_config_" + level) + +@pytest.fixture(autouse=True) +def tmp_config_dir(mocker, tmp_path): + """ + Fixture to prevent modifying/reading the actual global config + """ + + for level in ["global", "system"]: + os.makedirs(_get_tmp_config_dir(tmp_path, level)) + + def get_tmp_config_dir(level): + return str(_get_tmp_config_dir(tmp_path, level)) + mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) + @pytest.mark.parametrize( "file,ret,output", [("ignored", 0, True), ("not_ignored", 1, False)] ) @@ -99,18 +115,50 @@ def test_check_ignore_sub_repo(tmp_dir, dvc, capsys): ) -def test_check_sys_root_ignore_file(tmp_dir, dvc, capsys): +def test_check_excludesfile(tmp_dir, dvc, capsys): + excludesfile = Path.home() / DvcIgnore.DVCIGNORE_FILE + + with dvc.config.edit() as conf: + conf["core"]["excludesfile"] = str(excludesfile) + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_excludesfile": "ignored_in_excludesfile", + } + } + ) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + (excludesfile).write_text( + "ignored_in_excludesfile", encoding="utf-8" + ) + + assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 + output, _ = capsys.readouterr() + assert ( + output + == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\t" + + "ignored_in_repo_root\n" + ) + + assert main(["check-ignore", "-d", "ignored_in_excludesfile"]) == 0 + output, _ = capsys.readouterr() + assert output == f"{excludesfile}:1:ignored_in_excludesfile\tignored_in_excludesfile\n" + +def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen( { "dir": { "ignored_in_repo_root": "ignored_in_repo_root", - "ignored_in_sys_root": "ignored_in_sys_root", + "ignored_in_global": "ignored_in_global", } } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_sys_root", encoding="utf-8" + global_dvcignore = _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE + global_dvcignore.write_text( + "ignored_in_global", + encoding="utf-8" ) assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 @@ -121,10 +169,37 @@ def test_check_sys_root_ignore_file(tmp_dir, dvc, capsys): + "ignored_in_repo_root\n" ) - assert main(["check-ignore", "-d", "ignored_in_sys_root"]) == 0 + assert main(["check-ignore", "-d", "ignored_in_global"]) == 0 + output, _ = capsys.readouterr() + assert output == f"{global_dvcignore}:1:ignored_in_global\tignored_in_global\n" + +def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_system": "ignored_in_system", + } + } + ) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + system_dvcignore = _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE + system_dvcignore.write_text( + "ignored_in_system", + encoding="utf-8" + ) + + assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 output, _ = capsys.readouterr() - assert output == "sys_root:1:ignored_in_sys_root\tignored_in_sys_root\n" + assert ( + output + == f"{DvcIgnore.DVCIGNORE_FILE}:1:ignored_in_repo_root\t" + + "ignored_in_repo_root\n" + ) + assert main(["check-ignore", "-d", "ignored_in_system"]) == 0 + output, _ = capsys.readouterr() + assert output == f"{system_dvcignore}:1:ignored_in_system\tignored_in_system\n" def test_check_sub_dir_ignore_file(tmp_dir, dvc, capsys): tmp_dir.gen( diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index e26f3cf891..db7f8ad8e2 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -21,6 +21,22 @@ def _to_pattern_info_list(str_list: List): def walk_files(dvc, *args): yield from dvc.dvcignore.find(*args) +def _get_tmp_config_dir(tmp_path, level): + return tmp_path / ".." / (tmp_path.name + "_config_" + level) + +@pytest.fixture(autouse=True) +def tmp_config_dir(mocker, tmp_path): + """ + Fixture to prevent modifying/reading the actual global config + """ + + for level in ["global", "system"]: + os.makedirs(_get_tmp_config_dir(tmp_path, level)) + + def get_tmp_config_dir(level): + return str(_get_tmp_config_dir(tmp_path, level)) + mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) + @pytest.mark.parametrize("filename", ["ignored", "тест"]) def test_ignore(tmp_dir, dvc, filename): @@ -36,24 +52,112 @@ def test_ignore(tmp_dir, dvc, filename): } -def test_ignore_from_sys_root_dvcignore(tmp_dir, dvc): +@pytest.mark.parametrize("file_exists", [True, False]) +def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): + # NOTE(meshde): if core.excludesfile is defined in the config + # then the ignore patterns from the global or system .dvcignore + # should not be considered irrespective of whether or not the path + # given to code.excludesfile exists in the file system + excludesfile = Path.home() / DvcIgnore.DVCIGNORE_FILE + + with dvc.config.edit() as conf: + conf["core"]["excludesfile"] = str(excludesfile) + + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_excludesfile": "ignored_in_excludesfile", + "ignored_in_global": "ignored_in_global", + "ignored_in_system": "ignored_in_system", + } + } + ) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + if (file_exists): + (excludesfile).write_text( + "ignored_in_excludesfile", encoding="utf-8" + ) + (_get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_global", + encoding="utf-8" + ) + (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_system", + encoding="utf-8" + ) + dvc._reset() + + result = set(walk_files(dvc, dvc.fs, tmp_dir)) + + files_to_be_ignored = ["ignored_in_repo_root"] + files_not_to_be_ignored = ["ignored_in_global", "ignored_in_system"] + + if (file_exists): + files_to_be_ignored.append("ignored_in_excludesfile") + else: + files_not_to_be_ignored.append("ignored_in_excludesfile") + + for ignored_file in files_to_be_ignored: + assert (tmp_dir / "dir" / ignored_file).fs_path not in result + + for file in files_not_to_be_ignored: + assert (tmp_dir / "dir" / file).fs_path in result + +def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): + # NOTE(meshde): if core.excludesfile is not defined in the config + # and global .dvcignore exists then consider read patterns from this file + # and do not consider patterns from system .dvcignore tmp_dir.gen( { "dir": { "ignored_in_repo_root": "ignored_in_repo_root", - "ignored_in_sys_root": "ignored_in_sys_root", + "ignored_in_global": "ignored_in_global", + "ignored_in_system": "ignored_in_system", } } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - (Path.home() / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_sys_root", encoding="utf-8" + (_get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_global", + encoding="utf-8" + ) + (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_system", + encoding="utf-8" ) + dvc._reset() + + result = set(walk_files(dvc, dvc.fs, tmp_dir)) + + for ignored_file in ["ignored_in_repo_root", "ignored_in_global"]: + assert (tmp_dir / "dir" / ignored_file).fs_path not in result + + assert (tmp_dir / "dir" / "ignored_in_system").fs_path in result + +def test_ignore_from_system_dvcignore(tmp_path, tmp_dir, dvc): + # NOTE(meshde): if core.excludesfile is not defined in the config and + # global .dvcignore does not exist but system .dvcignore exists then + # consider ignore patterns from this file + tmp_dir.gen( + { + "dir": { + "ignored_in_repo_root": "ignored_in_repo_root", + "ignored_in_system": "ignored_in_system", + } + } + ) + tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( + "ignored_in_system", + encoding="utf-8" + ) + dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) - for ignored_file in ["ignored_in_repo_root", "ignored_in_sys_root"]: - assert (tmp_dir / "dir" / ignored_file).fs_path in result + for ignored_file in ["ignored_in_repo_root", "ignored_in_system"]: + assert (tmp_dir / "dir" / ignored_file).fs_path not in result def test_rename_ignored_file(tmp_dir, dvc): From 759d427b4dd8a63ba35000a5cae4b8905fdd8100 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 15 Sep 2022 09:38:28 +0000 Subject: [PATCH 05/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- dvc/ignore.py | 9 +++---- tests/func/test_check_ignore.py | 39 +++++++++++++++++---------- tests/func/test_ignore.py | 48 ++++++++++++++++----------------- 3 files changed, 51 insertions(+), 45 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index a15a5cbf83..7908fb9f97 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -9,8 +9,8 @@ from pathspec.util import normalize_file from pygtrie import Trie -from dvc.fs import AnyFSPath, FileSystem, Schemes, localfs from dvc.config import Config +from dvc.fs import AnyFSPath, FileSystem, Schemes, localfs from dvc.pathspec_math import PatternInfo, merge_patterns from dvc.types import List, Optional @@ -187,9 +187,7 @@ def extend_default_ignore_patterns(ignore_file_path): ignore_patterns = DvcIgnorePatterns.from_file( ignore_file_path, self.fs, ignore_file_path ) - default_ignore_patterns.extend( - ignore_patterns.pattern_list - ) + default_ignore_patterns.extend(ignore_patterns.pattern_list) if config_ignore_file: if self.fs.exists(config_ignore_file): @@ -197,8 +195,7 @@ def extend_default_ignore_patterns(ignore_file_path): else: for level in ["global", "system"]: ignore_file_path_at_level = self.fs.path.join( - Config.get_dir(level), - DvcIgnore.DVCIGNORE_FILE + Config.get_dir(level), DvcIgnore.DVCIGNORE_FILE ) if self.fs.exists(ignore_file_path_at_level): extend_default_ignore_patterns(ignore_file_path_at_level) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 8b95c49a39..ba8ac37f25 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -10,6 +10,7 @@ def _get_tmp_config_dir(tmp_path, level): return tmp_path / ".." / (tmp_path.name + "_config_" + level) + @pytest.fixture(autouse=True) def tmp_config_dir(mocker, tmp_path): """ @@ -21,8 +22,10 @@ def tmp_config_dir(mocker, tmp_path): def get_tmp_config_dir(level): return str(_get_tmp_config_dir(tmp_path, level)) + mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) + @pytest.mark.parametrize( "file,ret,output", [("ignored", 0, True), ("not_ignored", 1, False)] ) @@ -129,9 +132,7 @@ def test_check_excludesfile(tmp_dir, dvc, capsys): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - (excludesfile).write_text( - "ignored_in_excludesfile", encoding="utf-8" - ) + (excludesfile).write_text("ignored_in_excludesfile", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 output, _ = capsys.readouterr() @@ -143,7 +144,11 @@ def test_check_excludesfile(tmp_dir, dvc, capsys): assert main(["check-ignore", "-d", "ignored_in_excludesfile"]) == 0 output, _ = capsys.readouterr() - assert output == f"{excludesfile}:1:ignored_in_excludesfile\tignored_in_excludesfile\n" + assert ( + output + == f"{excludesfile}:1:ignored_in_excludesfile\tignored_in_excludesfile\n" + ) + def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen( @@ -155,11 +160,10 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - global_dvcignore = _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE - global_dvcignore.write_text( - "ignored_in_global", - encoding="utf-8" + global_dvcignore = ( + _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE ) + global_dvcignore.write_text("ignored_in_global", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 output, _ = capsys.readouterr() @@ -171,7 +175,11 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): assert main(["check-ignore", "-d", "ignored_in_global"]) == 0 output, _ = capsys.readouterr() - assert output == f"{global_dvcignore}:1:ignored_in_global\tignored_in_global\n" + assert ( + output + == f"{global_dvcignore}:1:ignored_in_global\tignored_in_global\n" + ) + def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen( @@ -183,11 +191,10 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - system_dvcignore = _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE - system_dvcignore.write_text( - "ignored_in_system", - encoding="utf-8" + system_dvcignore = ( + _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE ) + system_dvcignore.write_text("ignored_in_system", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 output, _ = capsys.readouterr() @@ -199,7 +206,11 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): assert main(["check-ignore", "-d", "ignored_in_system"]) == 0 output, _ = capsys.readouterr() - assert output == f"{system_dvcignore}:1:ignored_in_system\tignored_in_system\n" + assert ( + output + == f"{system_dvcignore}:1:ignored_in_system\tignored_in_system\n" + ) + def test_check_sub_dir_ignore_file(tmp_dir, dvc, capsys): tmp_dir.gen( diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index db7f8ad8e2..3eb2e0e4fa 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -21,9 +21,11 @@ def _to_pattern_info_list(str_list: List): def walk_files(dvc, *args): yield from dvc.dvcignore.find(*args) + def _get_tmp_config_dir(tmp_path, level): return tmp_path / ".." / (tmp_path.name + "_config_" + level) + @pytest.fixture(autouse=True) def tmp_config_dir(mocker, tmp_path): """ @@ -35,6 +37,7 @@ def tmp_config_dir(mocker, tmp_path): def get_tmp_config_dir(level): return str(_get_tmp_config_dir(tmp_path, level)) + mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) @@ -74,18 +77,14 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - if (file_exists): - (excludesfile).write_text( - "ignored_in_excludesfile", encoding="utf-8" - ) - (_get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_global", - encoding="utf-8" - ) - (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_system", - encoding="utf-8" - ) + if file_exists: + (excludesfile).write_text("ignored_in_excludesfile", encoding="utf-8") + ( + _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE + ).write_text("ignored_in_global", encoding="utf-8") + ( + _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE + ).write_text("ignored_in_system", encoding="utf-8") dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) @@ -93,7 +92,7 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): files_to_be_ignored = ["ignored_in_repo_root"] files_not_to_be_ignored = ["ignored_in_global", "ignored_in_system"] - if (file_exists): + if file_exists: files_to_be_ignored.append("ignored_in_excludesfile") else: files_not_to_be_ignored.append("ignored_in_excludesfile") @@ -104,6 +103,7 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): for file in files_not_to_be_ignored: assert (tmp_dir / "dir" / file).fs_path in result + def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): # NOTE(meshde): if core.excludesfile is not defined in the config # and global .dvcignore exists then consider read patterns from this file @@ -118,14 +118,12 @@ def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - (_get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_global", - encoding="utf-8" - ) - (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_system", - encoding="utf-8" - ) + ( + _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE + ).write_text("ignored_in_global", encoding="utf-8") + ( + _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE + ).write_text("ignored_in_system", encoding="utf-8") dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) @@ -135,6 +133,7 @@ def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): assert (tmp_dir / "dir" / "ignored_in_system").fs_path in result + def test_ignore_from_system_dvcignore(tmp_path, tmp_dir, dvc): # NOTE(meshde): if core.excludesfile is not defined in the config and # global .dvcignore does not exist but system .dvcignore exists then @@ -148,10 +147,9 @@ def test_ignore_from_system_dvcignore(tmp_path, tmp_dir, dvc): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - (_get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE).write_text( - "ignored_in_system", - encoding="utf-8" - ) + ( + _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE + ).write_text("ignored_in_system", encoding="utf-8") dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) From 7f5500e79e050daa9ec3dbb4db037a0ae304a707 Mon Sep 17 00:00:00 2001 From: meshde Date: Thu, 15 Sep 2022 15:25:03 +0530 Subject: [PATCH 06/20] fix lint --- dvc/ignore.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index 7908fb9f97..48c5937176 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -3,7 +3,6 @@ import re from collections import namedtuple from itertools import chain, groupby, takewhile -from pathlib import Path from pathspec.patterns import GitWildMatchPattern from pathspec.util import normalize_file From 5a073cb36adbce6e6914126e7c108967d6353937 Mon Sep 17 00:00:00 2001 From: meshde Date: Thu, 15 Sep 2022 15:46:51 +0530 Subject: [PATCH 07/20] fix lint --- tests/func/test_check_ignore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index ba8ac37f25..79809452b8 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -146,7 +146,8 @@ def test_check_excludesfile(tmp_dir, dvc, capsys): output, _ = capsys.readouterr() assert ( output - == f"{excludesfile}:1:ignored_in_excludesfile\tignored_in_excludesfile\n" + == f"{excludesfile}:1:ignored_in_excludesfile" + + "\tignored_in_excludesfile\n" ) From 3bc410d3c15c894a82d5b031d712b50da9ab9db1 Mon Sep 17 00:00:00 2001 From: meshde Date: Sat, 17 Sep 2022 16:37:07 +0530 Subject: [PATCH 08/20] tests: make isolate fixture isolate system-level config dir --- tests/conftest.py | 11 +++++++- tests/func/test_check_ignore.py | 27 ++++-------------- tests/func/test_ignore.py | 50 +++++++++++++-------------------- 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4c46ba448c..817aa0a335 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -181,11 +181,20 @@ def mocked_webbrowser_open(mocker): @pytest.fixture(autouse=True) -def isolate(tmp_path_factory, monkeypatch) -> None: +def isolate(tmp_path_factory, monkeypatch, mocker) -> None: path = tmp_path_factory.mktemp("mock") home_dir = path / "home" home_dir.mkdir() + # NOTE(meshde): appdirs.site_config_dir statically returns + # /Library/Application Support/ on macos leaving us no way to + # manipulate the response of this function using env variables + # + # Hence, resorting to mocking this function entirely + root_dir = path / "root" + root_dir.mkdir() + mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) + if sys.platform == "win32": home_drive, home_path = os.path.splitdrive(home_dir) monkeypatch.setenv("USERPROFILE", str(home_dir)) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 79809452b8..258aebe154 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -5,25 +5,7 @@ from dvc.cli import main from dvc.ignore import DvcIgnore - - -def _get_tmp_config_dir(tmp_path, level): - return tmp_path / ".." / (tmp_path.name + "_config_" + level) - - -@pytest.fixture(autouse=True) -def tmp_config_dir(mocker, tmp_path): - """ - Fixture to prevent modifying/reading the actual global config - """ - - for level in ["global", "system"]: - os.makedirs(_get_tmp_config_dir(tmp_path, level)) - - def get_tmp_config_dir(level): - return str(_get_tmp_config_dir(tmp_path, level)) - - mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) +from dvc.config import Config @pytest.mark.parametrize( @@ -161,8 +143,10 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + global_path = Path(Config.get_dir("global")) + os.makedirs(global_path) global_dvcignore = ( - _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE + global_path / DvcIgnore.DVCIGNORE_FILE ) global_dvcignore.write_text("ignored_in_global", encoding="utf-8") @@ -192,8 +176,9 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") + system_path = Path(Config.get_dir("system")) system_dvcignore = ( - _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE + system_path / DvcIgnore.DVCIGNORE_FILE ) system_dvcignore.write_text("ignored_in_system", encoding="utf-8") diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 3eb2e0e4fa..cb04e800b3 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -12,6 +12,7 @@ from dvc.types import List from dvc_data.hashfile.build import IgnoreInCollectedDirError from dvc_data.hashfile.utils import get_mtime_and_size +from dvc.config import Config def _to_pattern_info_list(str_list: List): @@ -22,23 +23,23 @@ def walk_files(dvc, *args): yield from dvc.dvcignore.find(*args) -def _get_tmp_config_dir(tmp_path, level): - return tmp_path / ".." / (tmp_path.name + "_config_" + level) - - -@pytest.fixture(autouse=True) -def tmp_config_dir(mocker, tmp_path): - """ - Fixture to prevent modifying/reading the actual global config - """ - - for level in ["global", "system"]: - os.makedirs(_get_tmp_config_dir(tmp_path, level)) +@pytest.fixture +def global_dvcignore(): + global_path = Path(Config.get_dir("global")) + os.makedirs(global_path) + global_dvcignore_path = ( + global_path / DvcIgnore.DVCIGNORE_FILE + ) + global_dvcignore_path.write_text("ignored_in_global", encoding="utf-8") - def get_tmp_config_dir(level): - return str(_get_tmp_config_dir(tmp_path, level)) - mocker.patch("dvc.config.Config.get_dir", side_effect=get_tmp_config_dir) +@pytest.fixture +def system_dvcignore(): + system_path = Path(Config.get_dir("system")) + system_dvcignore_path = ( + system_path / DvcIgnore.DVCIGNORE_FILE + ) + system_dvcignore_path.write_text("ignored_in_system", encoding="utf-8") @pytest.mark.parametrize("filename", ["ignored", "тест"]) @@ -56,6 +57,7 @@ def test_ignore(tmp_dir, dvc, filename): @pytest.mark.parametrize("file_exists", [True, False]) +@pytest.mark.usefixtures("global_dvcignore", "system_dvcignore") def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): # NOTE(meshde): if core.excludesfile is defined in the config # then the ignore patterns from the global or system .dvcignore @@ -79,12 +81,7 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") if file_exists: (excludesfile).write_text("ignored_in_excludesfile", encoding="utf-8") - ( - _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE - ).write_text("ignored_in_global", encoding="utf-8") - ( - _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE - ).write_text("ignored_in_system", encoding="utf-8") + dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) @@ -104,6 +101,7 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): assert (tmp_dir / "dir" / file).fs_path in result +@pytest.mark.usefixtures("global_dvcignore", "system_dvcignore") def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): # NOTE(meshde): if core.excludesfile is not defined in the config # and global .dvcignore exists then consider read patterns from this file @@ -118,12 +116,6 @@ def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - ( - _get_tmp_config_dir(tmp_path, "global") / DvcIgnore.DVCIGNORE_FILE - ).write_text("ignored_in_global", encoding="utf-8") - ( - _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE - ).write_text("ignored_in_system", encoding="utf-8") dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) @@ -134,6 +126,7 @@ def test_ignore_from_global_dvcignore(tmp_path, tmp_dir, dvc): assert (tmp_dir / "dir" / "ignored_in_system").fs_path in result +@pytest.mark.usefixtures("system_dvcignore") def test_ignore_from_system_dvcignore(tmp_path, tmp_dir, dvc): # NOTE(meshde): if core.excludesfile is not defined in the config and # global .dvcignore does not exist but system .dvcignore exists then @@ -147,9 +140,6 @@ def test_ignore_from_system_dvcignore(tmp_path, tmp_dir, dvc): } ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") - ( - _get_tmp_config_dir(tmp_path, "system") / DvcIgnore.DVCIGNORE_FILE - ).write_text("ignored_in_system", encoding="utf-8") dvc._reset() result = set(walk_files(dvc, dvc.fs, tmp_dir)) From 69a74b62f60085d228feb379ac3f4de70c244163 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 17 Sep 2022 11:08:46 +0000 Subject: [PATCH 09/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/func/test_check_ignore.py | 10 +++------- tests/func/test_ignore.py | 10 +++------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 258aebe154..36978fb978 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -4,8 +4,8 @@ import pytest from dvc.cli import main -from dvc.ignore import DvcIgnore from dvc.config import Config +from dvc.ignore import DvcIgnore @pytest.mark.parametrize( @@ -145,9 +145,7 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") global_path = Path(Config.get_dir("global")) os.makedirs(global_path) - global_dvcignore = ( - global_path / DvcIgnore.DVCIGNORE_FILE - ) + global_dvcignore = global_path / DvcIgnore.DVCIGNORE_FILE global_dvcignore.write_text("ignored_in_global", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 @@ -177,9 +175,7 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") system_path = Path(Config.get_dir("system")) - system_dvcignore = ( - system_path / DvcIgnore.DVCIGNORE_FILE - ) + system_dvcignore = system_path / DvcIgnore.DVCIGNORE_FILE system_dvcignore.write_text("ignored_in_system", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index cb04e800b3..b29197f893 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -4,6 +4,7 @@ import pytest +from dvc.config import Config from dvc.ignore import DvcIgnore, DvcIgnorePatterns from dvc.output import OutputIsIgnoredError from dvc.pathspec_math import PatternInfo, merge_patterns @@ -12,7 +13,6 @@ from dvc.types import List from dvc_data.hashfile.build import IgnoreInCollectedDirError from dvc_data.hashfile.utils import get_mtime_and_size -from dvc.config import Config def _to_pattern_info_list(str_list: List): @@ -27,18 +27,14 @@ def walk_files(dvc, *args): def global_dvcignore(): global_path = Path(Config.get_dir("global")) os.makedirs(global_path) - global_dvcignore_path = ( - global_path / DvcIgnore.DVCIGNORE_FILE - ) + global_dvcignore_path = global_path / DvcIgnore.DVCIGNORE_FILE global_dvcignore_path.write_text("ignored_in_global", encoding="utf-8") @pytest.fixture def system_dvcignore(): system_path = Path(Config.get_dir("system")) - system_dvcignore_path = ( - system_path / DvcIgnore.DVCIGNORE_FILE - ) + system_dvcignore_path = system_path / DvcIgnore.DVCIGNORE_FILE system_dvcignore_path.write_text("ignored_in_system", encoding="utf-8") From 6573f21848cac534d5e16d1c8de0a09ef2b7bf5f Mon Sep 17 00:00:00 2001 From: meshde Date: Sat, 17 Sep 2022 17:31:29 +0530 Subject: [PATCH 10/20] tests: do not fail if dir exists while creation --- tests/func/test_check_ignore.py | 3 ++- tests/func/test_ignore.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 36978fb978..4ce50a3c8b 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -144,7 +144,7 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") global_path = Path(Config.get_dir("global")) - os.makedirs(global_path) + global_path.mkdir(parents=True, exist_ok=True) global_dvcignore = global_path / DvcIgnore.DVCIGNORE_FILE global_dvcignore.write_text("ignored_in_global", encoding="utf-8") @@ -176,6 +176,7 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") system_path = Path(Config.get_dir("system")) system_dvcignore = system_path / DvcIgnore.DVCIGNORE_FILE + system_path.mkdir(parents=True, exist_ok=True) system_dvcignore.write_text("ignored_in_system", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index b29197f893..3e594dc51c 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -26,7 +26,7 @@ def walk_files(dvc, *args): @pytest.fixture def global_dvcignore(): global_path = Path(Config.get_dir("global")) - os.makedirs(global_path) + global_path.mkdir(parents=True, exist_ok=True) global_dvcignore_path = global_path / DvcIgnore.DVCIGNORE_FILE global_dvcignore_path.write_text("ignored_in_global", encoding="utf-8") @@ -34,6 +34,7 @@ def global_dvcignore(): @pytest.fixture def system_dvcignore(): system_path = Path(Config.get_dir("system")) + system_path.mkdir(parents=True, exist_ok=True) system_dvcignore_path = system_path / DvcIgnore.DVCIGNORE_FILE system_dvcignore_path.write_text("ignored_in_system", encoding="utf-8") From ddff2f0806f3d50ae6a9133bcc3b48ac0deb8cb8 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 07:16:52 +0530 Subject: [PATCH 11/20] tests: print vars to debug ci check failure --- tests/func/test_ignore.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 3e594dc51c..7ce4be5b59 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -216,6 +216,9 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname): sub_dir_path = os.path.dirname(os.fspath(ignore_file)) + print(dvcignore._get_trie_pattern(top_ignore_path).pattern_list) + print(dvcignore._get_trie_pattern(sub_dir_path).pattern_list) + assert ( DvcIgnorePatterns( *merge_patterns( From bfe9b85388f0e773f961c65299691a91f379a7c6 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 07:28:43 +0530 Subject: [PATCH 12/20] tests: print /Users/meshde to debug ci check --- tests/func/test_ignore.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 7ce4be5b59..c5c51562c7 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -218,6 +218,7 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname): print(dvcignore._get_trie_pattern(top_ignore_path).pattern_list) print(dvcignore._get_trie_pattern(sub_dir_path).pattern_list) + print(os.environ["HOME"]) assert ( DvcIgnorePatterns( From d02df22b6b92a560a33053ceb4de67cce4691d6b Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 10:55:27 +0530 Subject: [PATCH 13/20] tests: remove prints --- tests/func/test_ignore.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index c5c51562c7..3e594dc51c 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -216,10 +216,6 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname): sub_dir_path = os.path.dirname(os.fspath(ignore_file)) - print(dvcignore._get_trie_pattern(top_ignore_path).pattern_list) - print(dvcignore._get_trie_pattern(sub_dir_path).pattern_list) - print(os.environ["HOME"]) - assert ( DvcIgnorePatterns( *merge_patterns( From 4794cb5e678bf498d36ddfb56791a3c79e0d8e77 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 10:57:04 +0530 Subject: [PATCH 14/20] tests: set XDG_CONFIG_HOME in isolate fixture --- tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/conftest.py b/tests/conftest.py index 817aa0a335..417731f243 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -210,6 +210,7 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: monkeypatch.setenv(env_var, os.fspath(path)) else: monkeypatch.setenv("HOME", str(home_dir)) + monkeypatch.setenv("XDG_CONFIG_HOME", str(home_dir)) monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") contents = b""" From 22cd0c22d7e816aa4f51e6456d867169f5e88332 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 11:34:32 +0000 Subject: [PATCH 15/20] tests: support global and system dvc config isolation --- tests/conftest.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 417731f243..c21321f423 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -185,15 +185,8 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: path = tmp_path_factory.mktemp("mock") home_dir = path / "home" home_dir.mkdir() - - # NOTE(meshde): appdirs.site_config_dir statically returns - # /Library/Application Support/ on macos leaving us no way to - # manipulate the response of this function using env variables - # - # Hence, resorting to mocking this function entirely root_dir = path / "root" root_dir.mkdir() - mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) if sys.platform == "win32": home_drive, home_path = os.path.splitdrive(home_dir) @@ -208,9 +201,28 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: path = home_dir / "AppData" / sub_path path.mkdir(parents=True) monkeypatch.setenv(env_var, os.fspath(path)) + + # NOTE(meshde): The env vars set above don't seem to affect the + # response of appdirs.site_config_dir or appdirs.user_config_dir + # on Windows and these continue to return the actual respective + # config dirs on Windows machines + # + # Hence, resorting to mocking these functions entirely + mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) + mocker.patch("appdirs.user_config_dir", return_value=str(home_dir)) + elif sys.platform == "darwin": + monkeypatch.setenv("HOME", str(home_dir)) + + # NOTE(meshde): appdirs.site_config_dir statically returns + # /Library/Application Support/ on macos leaving us no way to + # manipulate the response of this function using env variables + # + # Hence, resorting to mocking this function entirely + mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) else: monkeypatch.setenv("HOME", str(home_dir)) monkeypatch.setenv("XDG_CONFIG_HOME", str(home_dir)) + monkeypatch.setenv("XDG_CONFIG_DIRS", str(root_dir)) monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") contents = b""" From 27904553ab338822f14370f616bb985867fe4c11 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 18:41:08 +0530 Subject: [PATCH 16/20] tests: use a custom file name for excludesfile --- tests/func/test_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 3e594dc51c..db00fb2943 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -60,7 +60,7 @@ def test_ignore_from_excludesfile(tmp_path, tmp_dir, dvc, file_exists): # then the ignore patterns from the global or system .dvcignore # should not be considered irrespective of whether or not the path # given to code.excludesfile exists in the file system - excludesfile = Path.home() / DvcIgnore.DVCIGNORE_FILE + excludesfile = Path.home() / (DvcIgnore.DVCIGNORE_FILE + "_custom") with dvc.config.edit() as conf: conf["core"]["excludesfile"] = str(excludesfile) From d0cbaf7af9b9cb65840fe3f61e410b00ae36a597 Mon Sep 17 00:00:00 2001 From: meshde Date: Sun, 18 Sep 2022 20:17:58 +0530 Subject: [PATCH 17/20] tests: allow config dirs to be called for different apps --- tests/conftest.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index c21321f423..0647605329 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -188,6 +188,9 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: root_dir = path / "root" root_dir.mkdir() + def config_dir_generator(dirname): + return lambda x, y: str(dirname / x / (y or "")) + if sys.platform == "win32": home_drive, home_path = os.path.splitdrive(home_dir) monkeypatch.setenv("USERPROFILE", str(home_dir)) @@ -208,8 +211,14 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: # config dirs on Windows machines # # Hence, resorting to mocking these functions entirely - mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) - mocker.patch("appdirs.user_config_dir", return_value=str(home_dir)) + mocker.patch( + "appdirs.site_config_dir", + side_effect=config_dir_generator(root_dir) + ) + mocker.patch( + "appdirs.user_config_dir", + side_effect=config_dir_generator(home_dir) + ) elif sys.platform == "darwin": monkeypatch.setenv("HOME", str(home_dir)) @@ -218,7 +227,10 @@ def isolate(tmp_path_factory, monkeypatch, mocker) -> None: # manipulate the response of this function using env variables # # Hence, resorting to mocking this function entirely - mocker.patch("appdirs.site_config_dir", return_value=str(root_dir)) + mocker.patch( + "appdirs.site_config_dir", + side_effect=config_dir_generator(root_dir) + ) else: monkeypatch.setenv("HOME", str(home_dir)) monkeypatch.setenv("XDG_CONFIG_HOME", str(home_dir)) From d01f182ace5a2ef6fe716ffb4655f9047000866a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 18 Sep 2022 14:50:20 +0000 Subject: [PATCH 18/20] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/conftest.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0647605329..7b28562288 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -213,11 +213,11 @@ def config_dir_generator(dirname): # Hence, resorting to mocking these functions entirely mocker.patch( "appdirs.site_config_dir", - side_effect=config_dir_generator(root_dir) + side_effect=config_dir_generator(root_dir), ) mocker.patch( "appdirs.user_config_dir", - side_effect=config_dir_generator(home_dir) + side_effect=config_dir_generator(home_dir), ) elif sys.platform == "darwin": monkeypatch.setenv("HOME", str(home_dir)) @@ -229,7 +229,7 @@ def config_dir_generator(dirname): # Hence, resorting to mocking this function entirely mocker.patch( "appdirs.site_config_dir", - side_effect=config_dir_generator(root_dir) + side_effect=config_dir_generator(root_dir), ) else: monkeypatch.setenv("HOME", str(home_dir)) From aa1c11dcb18cbd5422cb8d2857bc3061018e270b Mon Sep 17 00:00:00 2001 From: meshde Date: Fri, 30 Sep 2022 02:22:58 +0000 Subject: [PATCH 19/20] dvcignore: refactor to have class method that returns global ignore patterns --- dvc/ignore.py | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index 48c5937176..e129952da8 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -179,26 +179,7 @@ def __init__(self, fs, root_dir): key = self._get_key(root_dir) - core_config = self.config.get("core", {}) - config_ignore_file = core_config.get("excludesfile", None) - - def extend_default_ignore_patterns(ignore_file_path): - ignore_patterns = DvcIgnorePatterns.from_file( - ignore_file_path, self.fs, ignore_file_path - ) - default_ignore_patterns.extend(ignore_patterns.pattern_list) - - if config_ignore_file: - if self.fs.exists(config_ignore_file): - extend_default_ignore_patterns(config_ignore_file) - else: - for level in ["global", "system"]: - ignore_file_path_at_level = self.fs.path.join( - Config.get_dir(level), DvcIgnore.DVCIGNORE_FILE - ) - if self.fs.exists(ignore_file_path_at_level): - extend_default_ignore_patterns(ignore_file_path_at_level) - break + default_ignore_patterns.extend(self._get_global_ignore_patterns()) self.ignores_trie_fs[key] = DvcIgnorePatterns( default_ignore_patterns, @@ -220,6 +201,32 @@ def extend_default_ignore_patterns(ignore_file_path): ignore_subrepos=True, ) + def _get_global_ignore_file(self): + core_config = self.config.get("core", {}) + config_ignore_file = core_config.get("excludesfile", None) + + if config_ignore_file: + return config_ignore_file + + for level in ["global", "system"]: + ignore_file_path_at_level = self.fs.path.join( + Config.get_dir(level), DvcIgnore.DVCIGNORE_FILE + ) + if self.fs.exists(ignore_file_path_at_level): + return ignore_file_path_at_level + + return None + + def _get_global_ignore_patterns(self): + global_ignore_file = self._get_global_ignore_file() + + if global_ignore_file and self.fs.exists(global_ignore_file): + return DvcIgnorePatterns.from_file( + global_ignore_file, self.fs, global_ignore_file + ).pattern_list + + return [] + def _get_key(self, path): parts = self.fs.path.relparts(path, self.root_dir) if parts == (".",): From eae144f448fa7de5378d1697669ae08d38a4a516 Mon Sep 17 00:00:00 2001 From: meshde Date: Fri, 30 Sep 2022 08:22:25 +0530 Subject: [PATCH 20/20] dvcignore: use 'ignore' as the global/system ignore file name --- dvc/ignore.py | 3 ++- tests/func/test_check_ignore.py | 4 ++-- tests/func/test_ignore.py | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dvc/ignore.py b/dvc/ignore.py index e129952da8..2b4f05ca92 100644 --- a/dvc/ignore.py +++ b/dvc/ignore.py @@ -18,6 +18,7 @@ class DvcIgnore: DVCIGNORE_FILE = ".dvcignore" + GLOBAL_IGNORE_FILE = "ignore" def __call__(self, root, dirs, files): raise NotImplementedError @@ -210,7 +211,7 @@ def _get_global_ignore_file(self): for level in ["global", "system"]: ignore_file_path_at_level = self.fs.path.join( - Config.get_dir(level), DvcIgnore.DVCIGNORE_FILE + Config.get_dir(level), DvcIgnore.GLOBAL_IGNORE_FILE ) if self.fs.exists(ignore_file_path_at_level): return ignore_file_path_at_level diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 4ce50a3c8b..2cdf814596 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -145,7 +145,7 @@ def test_check_global_dvcignore(tmp_path, tmp_dir, dvc, capsys): tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") global_path = Path(Config.get_dir("global")) global_path.mkdir(parents=True, exist_ok=True) - global_dvcignore = global_path / DvcIgnore.DVCIGNORE_FILE + global_dvcignore = global_path / DvcIgnore.GLOBAL_IGNORE_FILE global_dvcignore.write_text("ignored_in_global", encoding="utf-8") assert main(["check-ignore", "-d", "ignored_in_repo_root"]) == 0 @@ -175,7 +175,7 @@ def test_check_system_dvcignore(tmp_path, tmp_dir, dvc, capsys): ) tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored_in_repo_root") system_path = Path(Config.get_dir("system")) - system_dvcignore = system_path / DvcIgnore.DVCIGNORE_FILE + system_dvcignore = system_path / DvcIgnore.GLOBAL_IGNORE_FILE system_path.mkdir(parents=True, exist_ok=True) system_dvcignore.write_text("ignored_in_system", encoding="utf-8") diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py index 19c3200024..26004efac6 100644 --- a/tests/func/test_ignore.py +++ b/tests/func/test_ignore.py @@ -27,7 +27,7 @@ def walk_files(dvc, *args): def global_dvcignore(): global_path = Path(Config.get_dir("global")) global_path.mkdir(parents=True, exist_ok=True) - global_dvcignore_path = global_path / DvcIgnore.DVCIGNORE_FILE + global_dvcignore_path = global_path / DvcIgnore.GLOBAL_IGNORE_FILE global_dvcignore_path.write_text("ignored_in_global", encoding="utf-8") @@ -35,7 +35,7 @@ def global_dvcignore(): def system_dvcignore(): system_path = Path(Config.get_dir("system")) system_path.mkdir(parents=True, exist_ok=True) - system_dvcignore_path = system_path / DvcIgnore.DVCIGNORE_FILE + system_dvcignore_path = system_path / DvcIgnore.GLOBAL_IGNORE_FILE system_dvcignore_path.write_text("ignored_in_system", encoding="utf-8")