Просмотр исходного кода

Fix an error when running the "spot" check or "extract" action with the "progress" option or "--progress" flag (#1210).

Dan Helfman 1 день назад
Родитель
Сommit
802ca5aef3

+ 2 - 0
NEWS

@@ -1,4 +1,6 @@
 2.0.14.dev0
 2.0.14.dev0
+ * #1210: Fix an error when running the "spot" check or "extract" action with the "progress" option
+   or "--progress" flag.
  * #1212: Fix an error when restoring multiple directory-format database dumps at once.
  * #1212: Fix an error when restoring multiple directory-format database dumps at once.
 
 
 2.0.13
 2.0.13

+ 1 - 4
borgmatic/actions/check.py

@@ -380,10 +380,7 @@ def collect_spot_check_source_paths(
         dry_run=True,
         dry_run=True,
         repository_path=repository['path'],
         repository_path=repository['path'],
         # Omit "progress" because it interferes with "list_details".
         # Omit "progress" because it interferes with "list_details".
-        config=dict(
-            {option: value for option, value in config.items() if option != 'progress'},
-            list_details=True,
-        ),
+        config=dict(config, progress=False, list_details=True),
         patterns=borgmatic.actions.pattern.process_patterns(
         patterns=borgmatic.actions.pattern.process_patterns(
             borgmatic.actions.pattern.collect_patterns(config)
             borgmatic.actions.pattern.collect_patterns(config)
             + tuple(
             + tuple(

+ 0 - 2
borgmatic/actions/config/bootstrap.py

@@ -127,8 +127,6 @@ def run_bootstrap(bootstrap_arguments, global_arguments, local_borg_version):
         bootstrap_arguments.repository,
         bootstrap_arguments.repository,
         archive_name,
         archive_name,
         [config_path.lstrip(os.path.sep) for config_path in manifest_config_paths],
         [config_path.lstrip(os.path.sep) for config_path in manifest_config_paths],
-        # Only add progress here and not the extract_archive() call above, because progress
-        # conflicts with extract_to_stdout.
         dict(config, progress=bootstrap_arguments.progress or False),
         dict(config, progress=bootstrap_arguments.progress or False),
         local_borg_version,
         local_borg_version,
         global_arguments,
         global_arguments,

+ 1 - 1
borgmatic/actions/restore.py

@@ -496,7 +496,7 @@ def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored):
     if any requested dumps to restore weren't restored, indicating that they were missing from the
     if any requested dumps to restore weren't restored, indicating that they were missing from the
     configuration.
     configuration.
     '''
     '''
-    if not dumps_actually_restored:
+    if not dumps_to_restore:
         raise ValueError('No data source dumps were found to restore')
         raise ValueError('No data source dumps were found to restore')
 
 
     missing_dumps = sorted(
     missing_dumps = sorted(

+ 9 - 12
borgmatic/borg/extract.py

@@ -97,9 +97,6 @@ def extract_archive(
     lock_wait = config.get('lock_wait', None)
     lock_wait = config.get('lock_wait', None)
     extra_borg_options = config.get('extra_borg_options', {}).get('extract', '')
     extra_borg_options = config.get('extra_borg_options', {}).get('extract', '')
 
 
-    if config.get('progress') and extract_to_stdout:
-        raise ValueError('progress and extract to stdout cannot both be set')
-
     if feature.available(feature.Feature.NUMERIC_IDS, local_borg_version):
     if feature.available(feature.Feature.NUMERIC_IDS, local_borg_version):
         numeric_ids_flags = ('--numeric-ids',) if config.get('numeric_ids') else ()
         numeric_ids_flags = ('--numeric-ids',) if config.get('numeric_ids') else ()
     else:
     else:
@@ -133,7 +130,7 @@ def extract_archive(
         + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
         + (('--debug', '--list', '--show-rc') if logger.isEnabledFor(logging.DEBUG) else ())
         + (('--dry-run',) if dry_run else ())
         + (('--dry-run',) if dry_run else ())
         + (('--strip-components', str(strip_components)) if strip_components else ())
         + (('--strip-components', str(strip_components)) if strip_components else ())
-        + (('--progress',) if config.get('progress') else ())
+        + (('--progress',) if config.get('progress') and not extract_to_stdout else ())
         + (('--stdout',) if extract_to_stdout else ())
         + (('--stdout',) if extract_to_stdout else ())
         + (tuple(shlex.split(extra_borg_options)) if extra_borg_options else ())
         + (tuple(shlex.split(extra_borg_options)) if extra_borg_options else ())
         + flags.make_repository_archive_flags(
         + flags.make_repository_archive_flags(
@@ -152,29 +149,29 @@ def extract_archive(
         os.path.join(working_directory or '', destination_path) if destination_path else None
         os.path.join(working_directory or '', destination_path) if destination_path else None
     )
     )
 
 
-    # The progress output isn't compatible with captured and logged output, as progress messes with
-    # the terminal directly.
-    if config.get('progress'):
+    if extract_to_stdout:
         return execute_command(
         return execute_command(
             full_command,
             full_command,
-            output_file=DO_NOT_CAPTURE,
+            output_file=subprocess.PIPE,
+            run_to_completion=False,
             environment=environment.make_environment(config),
             environment=environment.make_environment(config),
             working_directory=full_destination_path,
             working_directory=full_destination_path,
             borg_local_path=local_path,
             borg_local_path=local_path,
             borg_exit_codes=borg_exit_codes,
             borg_exit_codes=borg_exit_codes,
         )
         )
-        return None
 
 
-    if extract_to_stdout:
+    # The progress output isn't compatible with captured and logged output, as progress messes with
+    # the terminal directly.
+    if config.get('progress'):
         return execute_command(
         return execute_command(
             full_command,
             full_command,
-            output_file=subprocess.PIPE,
-            run_to_completion=False,
+            output_file=DO_NOT_CAPTURE,
             environment=environment.make_environment(config),
             environment=environment.make_environment(config),
             working_directory=full_destination_path,
             working_directory=full_destination_path,
             borg_local_path=local_path,
             borg_local_path=local_path,
             borg_exit_codes=borg_exit_codes,
             borg_exit_codes=borg_exit_codes,
         )
         )
+        return None
 
 
     # Don't give Borg local path so as to error on warnings, as "borg extract" only gives a warning
     # Don't give Borg local path so as to error on warnings, as "borg extract" only gives a warning
     # if the restore paths don't exist in the archive.
     # if the restore paths don't exist in the archive.

+ 2 - 2
tests/unit/actions/test_check.py

@@ -651,7 +651,7 @@ def test_collect_spot_check_source_paths_omits_progress_from_create_dry_run_comm
     flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args(
     flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args(
         dry_run=True,
         dry_run=True,
         repository_path='repo',
         repository_path='repo',
-        config={'working_directory': '/', 'list_details': True},
+        config={'working_directory': '/', 'progress': False, 'list_details': True},
         patterns=[Pattern('foo'), Pattern('bar')],
         patterns=[Pattern('foo'), Pattern('bar')],
         local_borg_version=object,
         local_borg_version=object,
         global_arguments=object,
         global_arguments=object,
@@ -941,7 +941,7 @@ def test_collect_spot_check_source_paths_uses_working_directory():
     flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args(
     flexmock(module.borgmatic.borg.create).should_receive('make_base_create_command').with_args(
         dry_run=True,
         dry_run=True,
         repository_path='repo',
         repository_path='repo',
-        config={'working_directory': '/working/dir', 'list_details': True},
+        config={'working_directory': '/working/dir', 'progress': False, 'list_details': True},
         patterns=[Pattern('foo'), Pattern('bar')],
         patterns=[Pattern('foo'), Pattern('bar')],
         local_borg_version=object,
         local_borg_version=object,
         global_arguments=object,
         global_arguments=object,

+ 2 - 2
tests/unit/actions/test_restore.py

@@ -1144,8 +1144,8 @@ def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise(
 def test_ensure_requested_dumps_restored_with_no_dumps_raises():
 def test_ensure_requested_dumps_restored_with_no_dumps_raises():
     with pytest.raises(ValueError):
     with pytest.raises(ValueError):
         module.ensure_requested_dumps_restored(
         module.ensure_requested_dumps_restored(
-            dumps_to_restore={},
-            dumps_actually_restored={},
+            dumps_to_restore=set(),
+            dumps_actually_restored=set(),
         )
         )
 
 
 
 

+ 28 - 6
tests/unit/borg/test_extract.py

@@ -654,23 +654,45 @@ def test_extract_archive_calls_borg_with_progress_flag():
     )
     )
 
 
 
 
-def test_extract_archive_with_progress_and_extract_to_stdout_raises():
-    flexmock(module).should_receive('execute_command').never()
+def test_extract_archive_calls_borg_with_extract_to_stdout_returns_process():
+    flexmock(module.os.path).should_receive('abspath').and_return('repo')
+    process = flexmock()
+    flexmock(module.environment).should_receive('make_environment')
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module).should_receive('execute_command').with_args(
+        ('borg', 'extract', '--stdout', 'repo::archive'),
+        output_file=module.subprocess.PIPE,
+        run_to_completion=False,
+        environment=None,
+        working_directory=None,
+        borg_local_path='borg',
+        borg_exit_codes=None,
+    ).and_return(process).once()
+    flexmock(module.feature).should_receive('available').and_return(True)
+    flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return(None)
+    flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
+        ('repo::archive',),
+    )
+    flexmock(module.borgmatic.config.validate).should_receive(
+        'normalize_repository_path',
+    ).and_return('repo')
 
 
-    with pytest.raises(ValueError):
+    assert (
         module.extract_archive(
         module.extract_archive(
             dry_run=False,
             dry_run=False,
             repository='repo',
             repository='repo',
             archive='archive',
             archive='archive',
             paths=None,
             paths=None,
-            config={'progress': True},
+            config={},
             local_borg_version='1.2.3',
             local_borg_version='1.2.3',
             global_arguments=flexmock(),
             global_arguments=flexmock(),
             extract_to_stdout=True,
             extract_to_stdout=True,
         )
         )
+        == process
+    )
 
 
 
 
-def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process():
+def test_extract_archive_with_progress_and_extract_to_stdout_ignores_progress():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     process = flexmock()
     process = flexmock()
     flexmock(module.environment).should_receive('make_environment')
     flexmock(module.environment).should_receive('make_environment')
@@ -699,7 +721,7 @@ def test_extract_archive_calls_borg_with_stdout_parameter_and_returns_process():
             repository='repo',
             repository='repo',
             archive='archive',
             archive='archive',
             paths=None,
             paths=None,
-            config={},
+            config={'progress': True},
             local_borg_version='1.2.3',
             local_borg_version='1.2.3',
             global_arguments=flexmock(),
             global_arguments=flexmock(),
             extract_to_stdout=True,
             extract_to_stdout=True,