From b94999bba4bfe06bce4ce56f184cabcd004591df Mon Sep 17 00:00:00 2001
From: Dan Helfman <witten@torsion.org>
Date: Sat, 7 Dec 2019 21:36:51 -0800
Subject: [PATCH] Fix "borgmatic umount" so it only runs Borg once instead of
 once per repository / configuration file.

---
 NEWS                                  |  2 ++
 borgmatic/commands/borgmatic.py       | 24 ++++++++++++++++-------
 tests/unit/commands/test_borgmatic.py | 28 +++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/NEWS b/NEWS
index bc4a687..5739784 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 1.4.18
  * Fix "--repository" flag to accept relative paths.
+ * Fix "borgmatic umount" so it only runs Borg once instead of once per repository / configuration
+   file.
  * #253: Mount whole repositories via "borgmatic mount" without any "--archive" flag.
 
 1.4.17
diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py
index 5e285ae..a623db7 100644
--- a/borgmatic/commands/borgmatic.py
+++ b/borgmatic/commands/borgmatic.py
@@ -274,13 +274,6 @@ def run_actions(
                 local_path=local_path,
                 remote_path=remote_path,
             )
-    if 'umount' in arguments:
-        logger.info(
-            '{}: Unmounting mount point {}'.format(repository, arguments['umount'].mount_point)
-        )
-        borg_umount.unmount_archive(
-            mount_point=arguments['umount'].mount_point, local_path=local_path
-        )
     if 'restore' in arguments:
         if arguments['restore'].repository is None or validate.repositories_match(
             repository, arguments['restore'].repository
@@ -447,6 +440,14 @@ def make_error_log_records(message, error=None):
         pass
 
 
+def get_local_path(configs):
+    '''
+    Arbitrarily return the local path from the first configuration dict. Default to "borg" if not
+    set.
+    '''
+    return next(iter(configs.values())).get('location', {}).get('local_path', 'borg')
+
+
 def collect_configuration_run_summary_logs(configs, arguments):
     '''
     Given a dict of configuration filename to corresponding parsed configuration, and parsed
@@ -517,6 +518,15 @@ def collect_configuration_run_summary_logs(configs, arguments):
             if results:
                 json_results.extend(results)
 
+    if 'umount' in arguments:
+        logger.info('Unmounting mount point {}'.format(arguments['umount'].mount_point))
+        try:
+            borg_umount.unmount_archive(
+                mount_point=arguments['umount'].mount_point, local_path=get_local_path(configs)
+            )
+        except (CalledProcessError, OSError) as error:
+            yield from make_error_log_records('Error unmounting mount point', error)
+
     if json_results:
         sys.stdout.write(json.dumps(json_results))
 
diff --git a/tests/unit/commands/test_borgmatic.py b/tests/unit/commands/test_borgmatic.py
index b0a887b..1bdb321 100644
--- a/tests/unit/commands/test_borgmatic.py
+++ b/tests/unit/commands/test_borgmatic.py
@@ -169,6 +169,18 @@ def test_make_error_log_records_generates_nothing_for_other_error():
     assert logs == ()
 
 
+def test_get_local_path_uses_configuration_value():
+    assert module.get_local_path({'test.yaml': {'location': {'local_path': 'borg1'}}}) == 'borg1'
+
+
+def test_get_local_path_without_location_defaults_to_borg():
+    assert module.get_local_path({'test.yaml': {}}) == 'borg'
+
+
+def test_get_local_path_without_local_path_defaults_to_borg():
+    assert module.get_local_path({'test.yaml': {'location': {}}}) == 'borg'
+
+
 def test_collect_configuration_run_summary_logs_info_for_success():
     flexmock(module.command).should_receive('execute_hook').never()
     flexmock(module).should_receive('run_configuration').and_return([])
@@ -324,6 +336,22 @@ def test_collect_configuration_run_summary_logs_run_configuration_error():
     assert {log.levelno for log in logs} == {logging.CRITICAL}
 
 
+def test_collect_configuration_run_summary_logs_run_umount_error():
+    flexmock(module.validate).should_receive('guard_configuration_contains_repository')
+    flexmock(module).should_receive('run_configuration').and_return([])
+    flexmock(module.borg_umount).should_receive('unmount_archive').and_raise(OSError)
+    flexmock(module).should_receive('make_error_log_records').and_return(
+        [logging.makeLogRecord(dict(levelno=logging.CRITICAL, levelname='CRITICAL', msg='Error'))]
+    )
+    arguments = {'umount': flexmock(mount_point='/mnt')}
+
+    logs = tuple(
+        module.collect_configuration_run_summary_logs({'test.yaml': {}}, arguments=arguments)
+    )
+
+    assert {log.levelno for log in logs} == {logging.INFO, logging.CRITICAL}
+
+
 def test_collect_configuration_run_summary_logs_outputs_merged_json_results():
     flexmock(module).should_receive('run_configuration').and_return(['foo', 'bar']).and_return(
         ['baz']