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

src/borg/patterns.py

@@ -4,11 +4,15 @@
 import sys
 import unicodedata
 from collections import namedtuple
+import os
 from enum import Enum
 
 from .helpers import clean_lines, shellpattern
 from .helpers.argparsing import Action, ArgumentTypeError
 from .helpers.errors import Error
+from .logger import create_logger
+
+logger = create_logger()
 
 
 def parse_patternfile_line(line, roots, ie_commands, fallback):
@@ -89,15 +93,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 +152,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 +316,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 +377,18 @@ 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?
+        # Check if path is absolute
+        is_absolute = remainder_str.startswith("/")
+        # Check if path exists
+        path_exists = os.path.exists(remainder_str)
+
+        # Warn about relative paths
+        if not is_absolute:
+            logger.warning("Root path %r is relative, recommended to use absolute path", remainder_str)
+        # Warn about non-existent paths
+        if not path_exists:
+            logger.warning("Root path %r does not exist", remainder_str)
+
         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 logging
 import os.path
 import sys
 
 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,72 @@ 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(caplog):
+    import tempfile
+    import os as test_os
+
+    caplog.set_level(logging.WARNING, logger="borg.patterns")
+
+    # Test 1: Unix-style absolute path that doesn't exist
+    # On Unix systems, /absolute/paths don't trigger "relative" warning
+    # On Windows, such paths may not exist but still validate correctly
+    caplog.clear()
+    parse_inclexcl_command("R /absolute/unix/path")
+    # Should not have "relative" warning (starts with /)
+    assert "relative" not in caplog.text
+
+    # Test 2: absolute path that doesn't exist should warn about existence
+    caplog.clear()
+    parse_inclexcl_command("R /nonexistent/absolute/path/12345")
+    assert "does not exist" in caplog.text
+
+    # Test 3: relative path that exists should warn about relative
+    caplog.clear()
+    with tempfile.TemporaryDirectory() as tmpdir:
+        # Create temp subdir to be relative
+        old_cwd = test_os.getcwd()
+        try:
+            test_os.chdir(tmpdir)
+            # Create a subdir
+            test_os.makedirs("test_root_dir", exist_ok=True)
+            parse_inclexcl_command("R test_root_dir")
+            assert "relative" in caplog.text
+            assert "does not exist" not in caplog.text
+        finally:
+            test_os.chdir(old_cwd)
+
+    # Test 4: relative path that doesn't exist should warn about both
+    caplog.clear()
+    parse_inclexcl_command("R relative/nonexistent/path")
+    assert "relative" in caplog.text
+    assert "does not exist" in caplog.text
+
+
 @pytest.mark.parametrize(
     "pattern, regex",
     [