Merge pull request #9542 from mr-raj12/cleanup-patterns-todos-master

patterns: clean up TODOs, move is_include_cmd to IECommand property, fixes #9442

Author: ThomasWaldmann

src/borg/patterns.py

@@ -3,8 +3,10 @@
 import re
 import sys
 import unicodedata
+import warnings
 from collections import namedtuple
 from enum import Enum
+from pathlib import Path
 
 from .helpers import clean_lines, shellpattern
 from .helpers.argparsing import Action, ArgumentTypeError
@@ -89,15 +91,13 @@ def __init__(self, fallback=None):
         # False when calling match().
         self.recurse_dir = None
 
-        # whether to recurse into directories when no match is found
-        # TODO: allow modification as a config option?
+        # Whether to recurse into directories when no match is found.
+        # This must be True so that include patterns inside excluded directories
+        # work correctly (e.g. "+ /excluded_dir/important" inside "- /excluded_dir").
         self.recurse_dir_default = True
 
         self.include_patterns = []
 
-        # TODO: move this info to parse_inclexcl_command and store in PatternBase subclass?
-        self.is_include_cmd = {IECommand.Exclude: False, IECommand.ExcludeNoRecurse: False, IECommand.Include: True}
-
     def empty(self):
         return not len(self._items) and not len(self._path_full_patterns)
 
@@ -150,13 +150,13 @@ def match(self, path):
         if value is not non_existent:
             # we have a full path match!
             self.recurse_dir = command_recurses_dir(value)
-            return self.is_include_cmd[value]
+            return value.is_include
 
         # this is the slow way, if we have many patterns in self._items:
         for pattern, cmd in self._items:
             if pattern.match(path, normalize=False):
                 self.recurse_dir = pattern.recurse_dir
-                return self.is_include_cmd[cmd]
+                return cmd.is_include
 
         # by default we will recurse if there is no match
         self.recurse_dir = self.recurse_dir_default
@@ -314,10 +314,17 @@ class IECommand(Enum):
     Exclude = 4
     ExcludeNoRecurse = 5
 
+    @property
+    def is_include(self):
+        return self is IECommand.Include
+
 
 def command_recurses_dir(cmd):
-    # TODO?: raise error or return None if *cmd* is RootPath or PatternStyle
-    return cmd not in [IECommand.ExcludeNoRecurse]
+    if cmd is IECommand.ExcludeNoRecurse:
+        return False
+    if cmd is IECommand.Include or cmd is IECommand.Exclude:
+        return True
+    raise ValueError(f"command_recurses_dir: unexpected command: {cmd!r}")
 
 
 def get_pattern_class(prefix):
@@ -368,7 +375,14 @@ def parse_inclexcl_command(cmd_line_str, fallback=ShellPattern):
         raise ArgumentTypeError("A pattern/command must have a value part.")
 
     if cmd is IECommand.RootPath:
-        # TODO: validate string?
+        if not Path(remainder_str).is_absolute():
+            warnings.warn(
+                f"Root path {remainder_str!r} is not absolute, it is recommended to use an absolute path",
+                UserWarning,
+                stacklevel=2,
+            )
+        if not Path(remainder_str).exists():
+            warnings.warn(f"Root path {remainder_str!r} does not exist", UserWarning, stacklevel=2)
         val = remainder_str
     elif cmd is IECommand.PatternStyle:
         # then remainder_str is something like 're' or 'sh'

src/borg/testsuite/patterns_test.py

@@ -1,13 +1,14 @@
 import io
 import os.path
 import sys
+import warnings
 
 import pytest
 
 from ..helpers.argparsing import ArgumentTypeError
 from ..patterns import PathFullPattern, PathPrefixPattern, FnmatchPattern, ShellPattern, RegexPattern
-from ..patterns import load_exclude_file, load_pattern_file
-from ..patterns import parse_pattern, PatternMatcher
+from ..patterns import IECommand, load_exclude_file, load_pattern_file
+from ..patterns import command_recurses_dir, parse_inclexcl_command, parse_pattern, PatternMatcher
 from ..patterns import get_regex_from_pattern
 
 
@@ -605,25 +606,53 @@ def test_pattern_matcher():
     for i in ["", "foo", "bar"]:
         assert pm.match(i) is None
 
-    # add extra entries to aid in testing
-    for target in ["A", "B", "Empty", "FileNotFound"]:
-        pm.is_include_cmd[target] = target
+    pm.add([RegexPattern("^a")], IECommand.Include)
+    pm.add([RegexPattern("^b"), RegexPattern("^z")], IECommand.Exclude)
+    pm.add([RegexPattern("^$")], IECommand.ExcludeNoRecurse)
+    pm.fallback = False
 
-    pm.add([RegexPattern("^a")], "A")
-    pm.add([RegexPattern("^b"), RegexPattern("^z")], "B")
-    pm.add([RegexPattern("^$")], "Empty")
-    pm.fallback = "FileNotFound"
-
-    assert pm.match("") == "Empty"
-    assert pm.match("aaa") == "A"
-    assert pm.match("bbb") == "B"
-    assert pm.match("ccc") == "FileNotFound"
-    assert pm.match("xyz") == "FileNotFound"
-    assert pm.match("z") == "B"
+    assert pm.match("") is False  # ExcludeNoRecurse -> not include
+    assert pm.match("aaa") is True  # Include
+    assert pm.match("bbb") is False  # Exclude
+    assert pm.match("ccc") is False  # fallback
+    assert pm.match("xyz") is False  # fallback
+    assert pm.match("z") is False  # Exclude (matches ^z)
 
     assert PatternMatcher(fallback="hey!").fallback == "hey!"
 
 
+def test_command_recurses_dir():
+    assert command_recurses_dir(IECommand.Include) is True
+    assert command_recurses_dir(IECommand.Exclude) is True
+    assert command_recurses_dir(IECommand.ExcludeNoRecurse) is False
+    with pytest.raises(ValueError, match="unexpected command"):
+        command_recurses_dir(IECommand.RootPath)
+    with pytest.raises(ValueError, match="unexpected command"):
+        command_recurses_dir(IECommand.PatternStyle)
+
+
+def test_root_path_validation(tmp_path):
+    # absolute path that exists: no warnings
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        parse_inclexcl_command(f"R {tmp_path}")
+
+    # absolute path that doesn't exist: only "does not exist" warning
+    nonexistent = str(tmp_path / "nonexistent_subdir_12345")
+    with pytest.warns(UserWarning) as warning_list:
+        parse_inclexcl_command(f"R {nonexistent}")
+    messages = [str(w.message) for w in warning_list]
+    assert any("does not exist" in m for m in messages)
+    assert not any("absolute" in m for m in messages)
+
+    # relative path that doesn't exist: warns about both
+    with pytest.warns(UserWarning) as warning_list:
+        parse_inclexcl_command("R relative/nonexistent/path/xyz123")
+    messages = [str(w.message) for w in warning_list]
+    assert any("absolute" in m for m in messages)
+    assert any("does not exist" in m for m in messages)
+
+
 @pytest.mark.parametrize(
     "pattern, regex",
     [