borgfs include/exclude pattern matching broken

[nachtgeist]

Version: 1.1.4 (Debian/stretch-backports, amd64)

The attached test script creates a fresh repo, one archive and then tries to mount the repo with three different command-lines.

Case #1:

• Mounting works
• Accessing the archive list within the repo mount point works
• Acessing individual archives leads to an exception, see 1-*.log

Cases ##2, 3:
Mounting immediately leads to an exception, both with shell-style globbing ('*') and a simple regex ('.*') - see the 2nd and 3rd log.

borgfsfailure.sh.txt
1-borgfs-nopattern.log
1-ls.log
2-borgfs-asterisk.log
3-borgfs-dot-asterisk.log

[nachtgeist]

A quick workaround is

# diff -Naur /usr/lib/python3/dist-packages/borg/patterns.py{.orig,}
--- /usr/lib/python3/dist-packages/borg/patterns.py.orig        2018-01-17 17:58:41.352862857 +0100
+++ /usr/lib/python3/dist-packages/borg/patterns.py     2018-01-17 17:58:49.120876127 +0100
@@ -133,6 +133,8 @@
     def add_inclexcl(self, patterns):
         """Add list of patterns (of type CmdTuple) to internal list.
         """
+        if patterns is None:
+               return
         for pattern, cmd in patterns:
             self._add(pattern, cmd)

which allows to at least access all the archives. Actual usage of --exclude and/or --pattern still has no (obvious) effect. I'm not sure though whether this is to attribute to an implementation error or to insufficient documentation about PATTERN usage.

[ThomasWaldmann]

Is there a special reason why you use borgfs ... and not borg mount ...?

[nachtgeist]

I have my backup repos integrated in /etc/fstab so I can access them via a simple mount. I've gotten used to that so much that I did not in the least think that borg mount might make a difference.

I just tried again and can confirm that borg mount actually works without patching patterns.py.

So it seems that argument handling somewhat differs between borg and borgfs invocations...

[nachtgeist]

So it seems that argument handling somewhat differs between borg and borgfs invocations...

Found the culprit: the argument parser taking effect when sys.argv[0] is borgfs, lacks a default value for patterns which leads to the reported exception.

If one considers pattern.py implemented by-contract, another possible fix is

--- archiver.py 2018-01-22 01:01:19.894241848 +0100
+++ archiver.py2        2018-01-22 01:01:17.782230054 +0100
@@ -2505,6 +2505,7 @@
 
         common_parser = argparse.ArgumentParser(add_help=False, prog=self.prog)
         # some empty defaults for all subparsers
+        parser.set_defaults(patterns=[])
         common_parser.set_defaults(paths=[], patterns=[])
         parser.common_options.add_common_group(common_parser, '_subcommand')

[ThomasWaldmann]

Hmm, shouldn't it also default paths=[]? And maybe only set these defaults for borgfs?

[nachtgeist]

The latter would require quite some re-structuring of the argparser objects, I guess. And I'm not sure about the first one. You probably should ask the one who implemented the include/exclude cmdline-options and associated code.

Personally, I very much prefer patching patterns.py.

[ThomasWaldmann]

in the calls seen in the traceback, it processes both paths and patterns and it tries to iterate over both.
guess this is why it defaults both to [], just one line below the set_defaults call you added.

and we need to be consistent here: either we always go the set_defaults way, or we kill set_defaults and adapt patterns.py code to cope with getting None.

[ThomasWaldmann]

working on this.

about case # 1: valid, fix via set_defaults for both path and patterns. i also tried to fix via patterns.py and remove the set_defaults, but other code also relies on the empty list default for these.

about cases # 2 and 3: invalid pattern, patterns must start with + / - / !.
the error msg it gave was not really helpful, but I already fixed that recently so that it is more clear now what's going wrong.