Repository: borgbackup/borg
Pull request: Fixes the man pages creation
The issue https://github.com/borgbackup/borg/issues/4471 solution
created many commands for borgfs which shouldn't exist in addition to
creating issue https://github.com/borgbackup/borg/issues/4752.

Fixes https://github.com/borgbackup/borg/issues/4752 .

Problem statement:
master: python setup.py build_man is broken
somehow it doesn't process `borgfs` correctly:

```
$ python setup.py build_man
Detected OpenSSL [via pkg-config]
Using bundled BLAKE2
Detected and preferring liblz4 [via pkg-config]
Using bundled ZSTD
Using bundled xxhash
running build_man
building man pages (in docs/man)
building man page borg-benchmark(1)
building man page borg-benchmark-crud(1)
building man page borgfs(1)
building man page borg-borgfs-benchmark(1)
building man page borg-borgfs-benchmark-crud(1)
error: [Errno 2] No such file or directory: 'docs/usage/borgfs_benchmark_crud.rst'
```

Hints from issue discussion:
Because of that, the related man pages will be missing in 1.2.0a7.
Hum,
I'm interested by this one.
However I'm not sure if it is just a matter of creating the good files for the borgfs commands, or some python code in the setup_docs.py file (which reads the files) in order to create them automatically.
The goal is to fix `python setup.py build_man` (or the `argparse` related code in `borg.archiver`?), so that build_man works without an issue.

`borg` is the usual cli command to invoke borgbackup, `borgfs` is a special cli command used by `fstab` / `mount` only in context of mounting a borg repo / archive.

`borg benchmark crud` exists as a command, but stuff like `borgfs benchmark crud` doesn't make sense nor does it need docs to be generated.
Ok right,
I'll have a look at it then!
Thanks for your answer!
BTW, this is likely a regression compared to older borg releases - I only noticed this brokenness recently.
So maybe checking older releases whether they work ok and comparing new to older code could give hints about what is broken.
Sure!
I'll have a look at the former releases!

Commit message: Fixes the man pages creation

The issue https://github.com/borgbackup/borg/issues/4471 solution
created many commands for borgfs which shouldn't exist in addition to
creating issue https://github.com/borgbackup/borg/issues/4752.

Fixes https://github.com/borgbackup/borg/issues/4752 .

Patch to review:
diff --git a/src/borg/archiver.py b/src/borg/archiver.py
index 8650a55301..c7e6118a01 100644
--- a/src/borg/archiver.py
+++ b/src/borg/archiver.py
@@ -2604,6 +2604,22 @@ def define_archive_filters_group(subparser, *, sort_by=True, first_last=True):
                 group.add_argument('--last', metavar='N', dest='last', default=0, type=positive_int_validator,
                                    help='consider last N archives after other filters were applied')
 
+        def define_borg_mount(subparser):
+            subparser.set_defaults(func=self.do_mount)
+            subparser.add_argument('location', metavar='REPOSITORY_OR_ARCHIVE', type=location_validator(),
+                                help='repository or archive to mount')
+            subparser.add_argument('mountpoint', metavar='MOUNTPOINT', type=str,
+                                help='where to mount filesystem')
+            subparser.add_argument('-f', '--foreground', dest='foreground',
+                                action='store_true',
+                                help='stay in foreground, do not daemonize')
+            subparser.add_argument('-o', dest='options', type=str,
+                                help='Extra mount options')
+            define_archive_filters_group(subparser)
+            subparser.add_argument('paths', metavar='PATH', nargs='*', type=str,
+                                   help='paths to extract; patterns are supported')
+            define_exclusion_group(subparser, strip_components=True)
+
         parser = argparse.ArgumentParser(prog=self.prog, description='Borg - Deduplicated Backups',
                                          add_help=False)
         # paths and patterns must have an empty list as default everywhere
@@ -2623,6 +2639,55 @@ def define_archive_filters_group(subparser, *, sort_by=True, first_last=True):
         mid_common_parser.set_defaults(paths=[], patterns=[])
         parser.common_options.add_common_group(mid_common_parser, '_midcommand')
 
+        # borg mount
+        mount_epilog = process_epilog("""
+        This command mounts an archive as a FUSE filesystem. This can be useful for
+        browsing an archive or restoring individual files. Unless the ``--foreground``
+        option is given the command will run in the background until the filesystem
+        is ``umounted``.
+
+        The command ``borgfs`` provides a wrapper for ``borg mount``. This can also be
+        used in fstab entries:
+        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto 0 0``
+
+        To allow a regular user to use fstab entries, add the ``user`` option:
+        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto,user 0 0``
+
+        For FUSE configuration and mount options, see the mount.fuse(8) manual page.
+
+        Additional mount options supported by borg:
+
+        - versions: when used with a repository mount, this gives a merged, versioned
+          view of the files in the archives. EXPERIMENTAL, layout may change in future.
+        - allow_damaged_files: by default damaged files (where missing chunks were
+          replaced with runs of zeros by borg check ``--repair``) are not readable and
+          return EIO (I/O error). Set this option to read such files.
+        - ignore_permissions: for security reasons the "default_permissions" mount
+          option is internally enforced by borg. "ignore_permissions" can be given to
+          not enforce "default_permissions".
+
+        The BORG_MOUNT_DATA_CACHE_ENTRIES environment variable is meant for advanced users
+        to tweak the performance. It sets the number of cached data chunks; additional
+        memory usage can be up to ~8 MiB times this number. The default is the number
+        of CPU cores.
+
+        When the daemonized process receives a signal or crashes, it does not unmount.
+        Unmounting in these cases could cause an active rsync or similar process
+        to unintentionally delete data.
+
+        When running in the foreground ^C/SIGINT unmounts cleanly, but other
+        signals or crashes do not.
+        """)
+
+        if parser.prog == 'borgfs':
+            parser.description = self.do_mount.__doc__
+            parser.epilog = mount_epilog
+            parser.formatter_class = argparse.RawDescriptionHelpFormatter
+            parser.help = 'mount repository'
+            subparser = parser
+            define_borg_mount(subparser)
+            return parser
+
         subparsers = parser.add_subparsers(title='required arguments', metavar='<command>')
 
         # borg benchmark
@@ -3771,74 +3836,12 @@ def define_archive_filters_group(subparser, *, sort_by=True, first_last=True):
         define_archive_filters_group(subparser)
         define_exclusion_group(subparser)
 
-        # borg mount
-        mount_epilog = process_epilog("""
-        This command mounts an archive as a FUSE filesystem. This can be useful for
-        browsing an archive or restoring individual files. Unless the ``--foreground``
-        option is given the command will run in the background until the filesystem
-        is ``umounted``.
-
-        The command ``borgfs`` provides a wrapper for ``borg mount``. This can also be
-        used in fstab entries:
-        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto 0 0``
-
-        To allow a regular user to use fstab entries, add the ``user`` option:
-        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto,user 0 0``
-
-        For FUSE configuration and mount options, see the mount.fuse(8) manual page.
-
-        Additional mount options supported by borg:
-
-        - versions: when used with a repository mount, this gives a merged, versioned
-          view of the files in the archives. EXPERIMENTAL, layout may change in future.
-        - allow_damaged_files: by default damaged files (where missing chunks were
-          replaced with runs of zeros by borg check ``--repair``) are not readable and
-          return EIO (I/O error). Set this option to read such files.
-        - ignore_permissions: for security reasons the "default_permissions" mount
-          option is internally enforced by borg. "ignore_permissions" can be given to
-          not enforce "default_permissions".
-
-        The BORG_MOUNT_DATA_CACHE_ENTRIES environment variable is meant for advanced users
-        to tweak the performance. It sets the number of cached data chunks; additional
-        memory usage can be up to ~8 MiB times this number. The default is the number
-        of CPU cores.
-
-        When the daemonized process receives a signal or crashes, it does not unmount.
-        Unmounting in these cases could cause an active rsync or similar process
-        to unintentionally delete data.
-
-        When running in the foreground ^C/SIGINT unmounts cleanly, but other
-        signals or crashes do not.
-        """)
-
-        if parser.prog == 'borgfs':
-            parser.description = self.do_mount.__doc__
-            parser.epilog = mount_epilog
-            parser.formatter_class = argparse.RawDescriptionHelpFormatter
-            parser.help = 'mount repository'
-            subparser = parser
-        else:
-            subparser = subparsers.add_parser('mount', parents=[common_parser], add_help=False,
-                                            description=self.do_mount.__doc__,
-                                            epilog=mount_epilog,
-                                            formatter_class=argparse.RawDescriptionHelpFormatter,
-                                            help='mount repository')
-        subparser.set_defaults(func=self.do_mount)
-        subparser.add_argument('location', metavar='REPOSITORY_OR_ARCHIVE', type=location_validator(),
-                            help='repository or archive to mount')
-        subparser.add_argument('mountpoint', metavar='MOUNTPOINT', type=str,
-                            help='where to mount filesystem')
-        subparser.add_argument('-f', '--foreground', dest='foreground',
-                            action='store_true',
-                            help='stay in foreground, do not daemonize')
-        subparser.add_argument('-o', dest='options', type=str,
-                            help='Extra mount options')
-        define_archive_filters_group(subparser)
-        subparser.add_argument('paths', metavar='PATH', nargs='*', type=str,
-                               help='paths to extract; patterns are supported')
-        define_exclusion_group(subparser, strip_components=True)
-        if parser.prog == 'borgfs':
-            return parser
+        subparser = subparsers.add_parser('mount', parents=[common_parser], add_help=False,
+                                        description=self.do_mount.__doc__,
+                                        epilog=mount_epilog,
+                                        formatter_class=argparse.RawDescriptionHelpFormatter,
+                                        help='mount repository')
+        define_borg_mount(subparser)
 
         # borg prune
         prune_epilog = process_epilog("""


Code review comments:
diff_hunk: @@ -2604,6 +2604,22 @@ def define_archive_filters_group(subparser, *, sort_by=True, first_last=True):
                 group.add_argument('--last', metavar='N', dest='last', default=0, type=positive_int_validator,
                                    help='consider last N archives after other filters were applied')
 
+        def define_borg_mount(subparser):
path: src/borg/archiver.py
text: @user1:
guess this should be `parser`.

that the parser can be in fact a subparser sometimes is a detail not needed here (and also is not true for borgfs).

diff_hunk: @@ -2623,6 +2639,55 @@ def define_archive_filters_group(subparser, *, sort_by=True, first_last=True):
         mid_common_parser.set_defaults(paths=[], patterns=[])
         parser.common_options.add_common_group(mid_common_parser, '_midcommand')
 
+        # borg mount
+        mount_epilog = process_epilog("""
+        This command mounts an archive as a FUSE filesystem. This can be useful for
+        browsing an archive or restoring individual files. Unless the ``--foreground``
+        option is given the command will run in the background until the filesystem
+        is ``umounted``.
+
+        The command ``borgfs`` provides a wrapper for ``borg mount``. This can also be
+        used in fstab entries:
+        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto 0 0``
+
+        To allow a regular user to use fstab entries, add the ``user`` option:
+        ``/path/to/repo /mnt/point fuse.borgfs defaults,noauto,user 0 0``
+
+        For FUSE configuration and mount options, see the mount.fuse(8) manual page.
+
+        Additional mount options supported by borg:
+
+        - versions: when used with a repository mount, this gives a merged, versioned
+          view of the files in the archives. EXPERIMENTAL, layout may change in future.
+        - allow_damaged_files: by default damaged files (where missing chunks were
+          replaced with runs of zeros by borg check ``--repair``) are not readable and
+          return EIO (I/O error). Set this option to read such files.
+        - ignore_permissions: for security reasons the "default_permissions" mount
+          option is internally enforced by borg. "ignore_permissions" can be given to
+          not enforce "default_permissions".
+
+        The BORG_MOUNT_DATA_CACHE_ENTRIES environment variable is meant for advanced users
+        to tweak the performance. It sets the number of cached data chunks; additional
+        memory usage can be up to ~8 MiB times this number. The default is the number
+        of CPU cores.
+
+        When the daemonized process receives a signal or crashes, it does not unmount.
+        Unmounting in these cases could cause an active rsync or similar process
+        to unintentionally delete data.
+
+        When running in the foreground ^C/SIGINT unmounts cleanly, but other
+        signals or crashes do not.
+        """)
+
+        if parser.prog == 'borgfs':
+            parser.description = self.do_mount.__doc__
+            parser.epilog = mount_epilog
+            parser.formatter_class = argparse.RawDescriptionHelpFormatter
+            parser.help = 'mount repository'
+            subparser = parser
+            define_borg_mount(subparser)
path: src/borg/archiver.py
text: @user1:
replace these 2 lines by:
```
define_borg_mount(parser)
```

The introduction of the `subparser` name is a relict from the old code structure - but as you have it as a function paramenter now, it is not needed any more.