patterns: clean up TODOs, move is_include_cmd to IECommand property, fixes #9442#9542
patterns: clean up TODOs, move is_include_cmd to IECommand property, fixes #9442#9542mr-raj12 wants to merge 2 commits intoborgbackup:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9542 +/- ##
==========================================
- Coverage 83.39% 83.38% -0.02%
==========================================
Files 87 87
Lines 15469 15484 +15
Branches 2321 2325 +4
==========================================
+ Hits 12901 12911 +10
- Misses 1818 1820 +2
- Partials 750 753 +3 ☔ View full report in Codecov by Sentry. |
src/borg/patterns.py
Outdated
|
|
||
| if cmd is IECommand.RootPath: | ||
| # TODO: validate string? | ||
| if not remainder_str.startswith("/"): |
There was a problem hiding this comment.
the roots in a pattern file are somehow equivalent to the PATHs given as cli arguments for borg create.
we do not require the PATHs to be absolute paths, so guess we should not require the roots to be abs paths either.
in both cases it might be clearer and less risky to use abs paths, but the user can use rel paths on their own risk.
validation: check if the path exists and if not, output a warning.
mr-raj12
force-pushed
the
cleanup-patterns-todos-master
branch
from
6d42b24 to
13c7df2
Compare
| if not remainder_str.startswith("/"): | ||
| logger.warning("Root path %r does not look like an absolute path", remainder_str) | ||
| # Check if path is absolute | ||
| is_absolute = remainder_str.startswith("/") |
There was a problem hiding this comment.
that does not work on win32.
Path(p).is_absolute() or so.
|
|
||
| # Warn about relative paths | ||
| if not is_absolute: | ||
| logger.warning("Root path %r is relative, recommended to use absolute path", remainder_str) |
There was a problem hiding this comment.
..., it is recommended to use an ...
2 participants
Description
Address the TODO items in
src/borg/patterns.pyas discussed in #9442:recurse_dir_default: Replaced the misleading TODO with a comment explaining why it must stayTrueas include patterns inside excluded directories (+inside-) depend on it.is_include_cmd: Moved from a dict onPatternMatcherto anis_includeproperty onIECommandand now the matcher callscmd.is_includeinstead of doing a dict lookup.RootPathvalidation: Added a warning when the root path doesn't start with/, so backups don't fail due to a typo.command_recurses_dir: Replacedcmd not in [IECommand.ExcludeNoRecurse]with explicitif/raisebranches usingiscomparisons. Now raisesValueErrorfor unexpected commands likeRootPathorPatternStyle.Tests updated to use real
IECommandvalues and cover the new code paths.Checklist
master