dvcignore: use fs.path instead of os.path

dvc/fs/local.py

@@ -1,6 +1,8 @@
 import logging
 import os
 
+from funcy import cached_property
+
 from dvc.scheme import Schemes
 from dvc.system import System
 from dvc.utils import tmp_fname
@@ -26,6 +28,12 @@ class LocalFileSystem(FileSystem):
         super().__init__(**config)
         self.fs = LocalFS()
 
+    @cached_property
+    def path(self):
+        from .path import Path
+
+        return Path(self.sep, os.getcwd)
+
     def open(self, path, mode="r", encoding=None, **kwargs):
         return open(path, mode=mode, encoding=encoding)
 

dvc/fs/path.py

@@ -3,7 +3,11 @@ import posixpath
 
 
 class Path:
-    def __init__(self, sep):
+    def __init__(self, sep, getcwd=None):
+        def _getcwd():
+            return ""
+
+        self.getcwd = getcwd or _getcwd
         if sep == posixpath.sep:
             self.flavour = posixpath
         elif sep == ntpath.sep:
@@ -14,6 +18,23 @@ class Path:
     def join(self, *parts):
         return self.flavour.join(*parts)
 
+    def split(self, path):
+        return self.flavour.split(path)
+
+    def normpath(self, path):
+        return self.flavour.normpath(path)
+
+    def isabs(self, path):
+        return self.flavour.isabs(path)
+
+    def abspath(self, path):
+        if not self.isabs(path):
+            path = self.join(self.getcwd(), path)
+        return self.normpath(path)
+
+    def commonprefix(self, path):
+        return self.flavour.commonprefix(path)
+
     def parts(self, path):
         drive, path = self.flavour.splitdrive(path.rstrip(self.flavour.sep))
 
@@ -40,6 +61,9 @@ class Path:
     def parent(self, path):
         return self.flavour.dirname(path)
 
+    def dirname(self, path):
+        return self.parent(path)
+
     def parents(self, path):
         parts = self.parts(path)
         return tuple(

dvc/ignore.py

@@ -12,7 +12,6 @@ from dvc.fs.local import localfs
 from dvc.pathspec_math import PatternInfo, merge_patterns
 from dvc.scheme import Schemes
 from dvc.types import AnyPath, List, Optional
-from dvc.utils import relpath
 from dvc.utils.collections import PathStringTrie
 
 logger = logging.getLogger(__name__)
@@ -26,16 +25,17 @@ class DvcIgnore:
 
 
 class DvcIgnorePatterns(DvcIgnore):
-    def __init__(self, pattern_list, dirname):
+    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
                 ]
 
+        self.sep = sep
         self.pattern_list = pattern_list
         self.dirname = dirname
-        self.prefix = self.dirname + os.sep
+        self.prefix = self.dirname + self.sep
 
         self.regex_pattern_list = [
             GitWildMatchPattern.pattern_to_regex(pattern_info.patterns)
@@ -52,8 +52,8 @@ class DvcIgnorePatterns(DvcIgnore):
 
     @classmethod
     def from_file(cls, path, fs, name):
-        assert os.path.isabs(path)
-        dirname = os.path.normpath(os.path.dirname(path))
+        assert fs.path.isabs(path)
+        dirname = fs.path.normpath(fs.path.dirname(path))
         with fs.open(path, encoding="utf-8") as fobj:
             path_spec_lines = [
                 PatternInfo(line, f"{name}:{line_no + 1}:{line}")
@@ -63,7 +63,7 @@ class DvcIgnorePatterns(DvcIgnore):
                 if line and not (line.strip().startswith("#"))
             ]
 
-        return cls(path_spec_lines, dirname)
+        return cls(path_spec_lines, dirname, fs.sep)
 
     def __call__(self, root: List[str], dirs: List[str], files: List[str]):
         files = [f for f in files if not self.matches(root, f)]
@@ -79,7 +79,7 @@ class DvcIgnorePatterns(DvcIgnore):
         elif dirname.startswith(self.prefix):
             rel = dirname[len(self.prefix) :]
             # NOTE: `os.path.join` is ~x5.5 slower
-            path = f"{rel}{os.sep}{basename}"
+            path = f"{rel}{self.sep}{basename}"
         else:
             return False
 
@@ -173,7 +173,9 @@ class DvcIgnoreFilter:
         self.ignores_trie_fs = PathStringTrie()
         self._ignores_trie_subrepos = PathStringTrie()
         self.ignores_trie_fs[root_dir] = DvcIgnorePatterns(
-            default_ignore_patterns, root_dir
+            default_ignore_patterns,
+            root_dir,
+            fs.sep,
         )
         self._ignores_trie_subrepos[root_dir] = self.ignores_trie_fs[root_dir]
         self._update(
@@ -193,19 +195,18 @@ class DvcIgnoreFilter:
         old_pattern = trie.longest_prefix(dirname).value
         matches = old_pattern.matches(dirname, DvcIgnore.DVCIGNORE_FILE, False)
 
-        path = os.path.join(dirname, DvcIgnore.DVCIGNORE_FILE)
+        path = self.fs.path.join(dirname, DvcIgnore.DVCIGNORE_FILE)
         if not matches and self.fs.exists(path):
-            name = os.path.relpath(path, self.root_dir)
+            name = self.fs.path.relpath(path, self.root_dir)
             new_pattern = DvcIgnorePatterns.from_file(path, self.fs, name)
             if old_pattern:
-                trie[dirname] = DvcIgnorePatterns(
-                    *merge_patterns(
-                        old_pattern.pattern_list,
-                        old_pattern.dirname,
-                        new_pattern.pattern_list,
-                        new_pattern.dirname,
-                    )
+                plist, prefix = merge_patterns(
+                    old_pattern.pattern_list,
+                    old_pattern.dirname,
+                    new_pattern.pattern_list,
+                    new_pattern.dirname,
                 )
+                trie[dirname] = DvcIgnorePatterns(plist, prefix, self.fs.sep)
             else:
                 trie[dirname] = new_pattern
         elif old_pattern:
@@ -229,7 +230,7 @@ class DvcIgnoreFilter:
 
             for dname in dnames:
                 self._update_sub_repo(
-                    os.path.join(dirname, dname), ignore_trie
+                    self.fs.path.join(dirname, dname), ignore_trie
                 )
 
     def _update_sub_repo(self, path, ignore_trie: PathStringTrie):
@@ -238,28 +239,27 @@ class DvcIgnoreFilter:
         if path == self.root_dir:
             return
 
-        dvc_dir = os.path.join(path, Repo.DVC_DIR)
-        if not os.path.exists(dvc_dir):
+        dvc_dir = self.fs.path.join(path, Repo.DVC_DIR)
+        if not self.fs.exists(dvc_dir):
             return
 
-        root, dname = os.path.split(path)
+        root, dname = self.fs.path.split(path)
         pattern_info = PatternInfo(f"/{dname}/", f"in sub_repo:{dname}")
-        new_pattern = DvcIgnorePatterns([pattern_info], root)
+        new_pattern = DvcIgnorePatterns([pattern_info], root, self.fs.sep)
         old_pattern = ignore_trie.longest_prefix(root).value
         if old_pattern:
-            ignore_trie[root] = DvcIgnorePatterns(
-                *merge_patterns(
-                    old_pattern.pattern_list,
-                    old_pattern.dirname,
-                    new_pattern.pattern_list,
-                    new_pattern.dirname,
-                )
+            plist, prefix = merge_patterns(
+                old_pattern.pattern_list,
+                old_pattern.dirname,
+                new_pattern.pattern_list,
+                new_pattern.dirname,
             )
+            ignore_trie[root] = DvcIgnorePatterns(plist, prefix, self.fs.sep)
         else:
             ignore_trie[root] = new_pattern
 
     def __call__(self, root, dirs, files, ignore_subrepos=True):
-        abs_root = os.path.abspath(root)
+        abs_root = self.fs.path.abspath(root)
         ignore_pattern = self._get_trie_pattern(
             abs_root, dnames=dirs, ignore_subrepos=ignore_subrepos
         )
@@ -343,7 +343,7 @@ class DvcIgnoreFilter:
     ):
         if self._outside_repo(path):
             return False
-        dirname, basename = os.path.split(os.path.normpath(path))
+        dirname, basename = self.fs.path.split(self.fs.path.normpath(path))
         ignore_pattern = self._get_trie_pattern(dirname, None, ignore_subrepos)
         if ignore_pattern:
             return ignore_pattern.matches(dirname, basename, is_dir)
@@ -351,7 +351,7 @@ class DvcIgnoreFilter:
 
     def is_ignored_dir(self, path: str, ignore_subrepos: bool = True) -> bool:
         "Only used in LocalFileSystem"
-        path = os.path.abspath(path)
+        path = self.fs.path.abspath(path)
         if path == self.root_dir:
             return False
 
@@ -359,31 +359,24 @@ class DvcIgnoreFilter:
 
     def is_ignored_file(self, path: str, ignore_subrepos: bool = True) -> bool:
         "Only used in LocalFileSystem"
-        path = os.path.abspath(path)
+        path = self.fs.path.abspath(path)
         return self._is_ignored(path, False, ignore_subrepos=ignore_subrepos)
 
     def _outside_repo(self, path):
-        # paths outside of the repo should be ignored
-        path = relpath(path, self.root_dir)
-        if path.startswith("..") or (
-            os.name == "nt"
-            and not os.path.commonprefix(
-                [os.path.abspath(path), self.root_dir]
-            )
-        ):
-            return True
-        return False
+        return not self.fs.path.isin_or_eq(path, self.root_dir)
 
     def check_ignore(self, target):
         # NOTE: can only be used in `dvc check-ignore`, see
         # https://github.com/iterative/dvc/issues/5046
-        full_target = os.path.abspath(target)
+        full_target = self.fs.path.abspath(target)
         if not self._outside_repo(full_target):
-            dirname, basename = os.path.split(os.path.normpath(full_target))
+            dirname, basename = self.fs.path.split(
+                self.fs.path.normpath(full_target)
+            )
             pattern = self._get_trie_pattern(dirname)
             if pattern:
                 matches = pattern.matches(
-                    dirname, basename, os.path.isdir(full_target), True
+                    dirname, basename, self.fs.isdir(full_target), True
                 )
 
                 if matches:

tests/func/test_ignore.py

@@ -115,7 +115,8 @@ def test_ignore_collecting_dvcignores(tmp_dir, dvc, dname):
                 os.fspath(tmp_dir),
                 _to_pattern_info_list([os.path.basename(dname)]),
                 top_ignore_path,
-            )
+            ),
+            os.sep,
         )
         == dvcignore._get_trie_pattern(top_ignore_path)
         == dvcignore._get_trie_pattern(sub_dir_path)
@@ -140,7 +141,7 @@ def test_ignore_on_branch(tmp_dir, scm, dvc):
     }
 
     dvc.fs = GitFileSystem(scm=scm, rev="branch")
-    assert dvc.dvcignore.is_ignored_file(tmp_dir / "foo")
+    assert dvc.dvcignore.is_ignored_file((tmp_dir / "foo").fs_path)
 
 
 def test_match_nested(tmp_dir, dvc):
@@ -372,15 +373,15 @@ def test_pattern_trie_fs(tmp_dir, dvc):
         os.fspath(tmp_dir / "other"),
     )
 
-    assert DvcIgnorePatterns(*base_pattern) == ignore_pattern_top
-    assert DvcIgnorePatterns(*other_pattern) == ignore_pattern_other
+    assert DvcIgnorePatterns(*base_pattern, os.sep) == ignore_pattern_top
+    assert DvcIgnorePatterns(*other_pattern, os.sep) == ignore_pattern_other
     assert (
-        DvcIgnorePatterns(*first_pattern)
+        DvcIgnorePatterns(*first_pattern, os.sep)
         == ignore_pattern_first
         == ignore_pattern_middle
     )
     assert (
-        DvcIgnorePatterns(*second_pattern)
+        DvcIgnorePatterns(*second_pattern, os.sep)
         == ignore_pattern_second
         == ignore_pattern_bottom
     )

tests/unit/test_ignore.py

@@ -7,7 +7,10 @@ from dvc.ignore import DvcIgnorePatterns
 
 
 def mock_dvcignore(dvcignore_path, patterns):
+    from dvc.fs.local import localfs
+
     fs = MagicMock()
+    fs.path = localfs.path
     with patch.object(fs, "open", mock_open(read_data="\n".join(patterns))):
         ignore_patterns = DvcIgnorePatterns.from_file(
             dvcignore_path, fs, "mocked"
@@ -195,7 +198,7 @@ def test_match_ignore_from_file(
 @pytest.mark.parametrize("omit_dir", [".git", ".hg", ".dvc"])
 def test_should_ignore_dir(omit_dir, sub_dir):
     root = os.path.join(os.path.sep, "walk", "dir", "root")
-    ignore = DvcIgnorePatterns([".git/", ".hg/", ".dvc/"], root)
+    ignore = DvcIgnorePatterns([".git/", ".hg/", ".dvc/"], root, os.sep)
 
     dirs = [omit_dir, "dir1", "dir2"]
     files = [omit_dir, "file1", "file2"]