From 227f475e1785aecb7792c0b6ba28aa2cc7020038 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 7 Nov 2024 19:19:56 -0800 Subject: [PATCH] Fix an error when implicitly upgrading the check state directory across filesystems (#931). --- NEWS | 2 ++ borgmatic/actions/check.py | 7 ++++--- borgmatic/actions/restore.py | 4 +++- tests/unit/actions/test_check.py | 20 ++++++++++---------- tests/unit/actions/test_restore.py | 4 ++-- 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/NEWS b/NEWS index 2214d6db..41998210 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.9.1.dev0 * #928: Fix the user runtime directory location on macOS (and possibly Cygwin). + * #931: Fix an error when implicitly upgrading the check state directory from ~/.borgmatic to + ~/.local/state/borgmatic across filesystems. 1.9.0 * #609: Fix the glob expansion of "source_directories" values to respect the "working_directory" diff --git a/borgmatic/actions/check.py b/borgmatic/actions/check.py index 946111bd..d5f7fbe2 100644 --- a/borgmatic/actions/check.py +++ b/borgmatic/actions/check.py @@ -6,6 +6,7 @@ import logging import os import pathlib import random +import shutil import borgmatic.borg.check import borgmatic.borg.create @@ -322,7 +323,7 @@ def upgrade_check_times(config, borg_repository_id): f'Upgrading archives check times directory from {borgmatic_source_checks_path} to {borgmatic_state_checks_path}' ) os.makedirs(borgmatic_state_path, mode=0o700, exist_ok=True) - os.rename(borgmatic_source_checks_path, borgmatic_state_checks_path) + shutil.move(borgmatic_source_checks_path, borgmatic_state_checks_path) for check_type in ('archives', 'data'): new_path = make_check_time_path(config, borg_repository_id, check_type, 'all') @@ -335,12 +336,12 @@ def upgrade_check_times(config, borg_repository_id): logger.debug(f'Upgrading archives check time file from {old_path} to {new_path}') try: - os.rename(old_path, temporary_path) + shutil.move(old_path, temporary_path) except FileNotFoundError: pass os.mkdir(old_path) - os.rename(temporary_path, new_path) + shutil.move(temporary_path, new_path) def collect_spot_check_source_paths( diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 3d8a873e..3441002c 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -91,7 +91,9 @@ def strip_path_prefix_from_extracted_dump_destination( if not databases_directory.endswith('_databases'): continue - os.rename(subdirectory_path, os.path.join(borgmatic_runtime_directory, databases_directory)) + shutil.move( + subdirectory_path, os.path.join(borgmatic_runtime_directory, databases_directory) + ) break diff --git a/tests/unit/actions/test_check.py b/tests/unit/actions/test_check.py index 05e9a6b0..f8987733 100644 --- a/tests/unit/actions/test_check.py +++ b/tests/unit/actions/test_check.py @@ -381,7 +381,7 @@ def test_upgrade_check_times_moves_checks_from_borgmatic_source_directory_to_sta '/home/user/.local/state/borgmatic/checks' ).and_return(False) flexmock(module.os).should_receive('makedirs') - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( '/home/user/.borgmatic/checks', '/home/user/.local/state/borgmatic/checks' ).once() @@ -408,13 +408,13 @@ def test_upgrade_check_times_with_checks_already_in_borgmatic_state_directory_do '/home/user/.local/state/borgmatic/checks' ).and_return(True) flexmock(module.os).should_receive('makedirs').never() - flexmock(module.os).should_receive('rename').never() + flexmock(module.shutil).should_receive('move').never() flexmock(module).should_receive('make_check_time_path').and_return( '/home/user/.local/state/borgmatic/checks/1234/archives/all' ) flexmock(module.os.path).should_receive('isfile').and_return(False) - flexmock(module.os).should_receive('rename').never() + flexmock(module.shutil).should_receive('move').never() flexmock(module.os).should_receive('mkdir').never() module.upgrade_check_times(flexmock(), flexmock()) @@ -448,11 +448,11 @@ def test_upgrade_check_times_renames_old_check_paths_to_all(): flexmock(module.os.path).should_receive('isfile').with_args( f'{base_path}/data.temp' ).and_return(False) - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/archives', f'{base_path}/archives.temp' ).once() flexmock(module.os).should_receive('mkdir').with_args(f'{base_path}/archives').once() - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/archives.temp', f'{base_path}/archives/all' ).once() @@ -484,11 +484,11 @@ def test_upgrade_check_times_renames_data_check_paths_when_archives_paths_are_al flexmock(module.os.path).should_receive('isfile').with_args(f'{base_path}/data').and_return( True ) - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/data', f'{base_path}/data.temp' ).once() flexmock(module.os).should_receive('mkdir').with_args(f'{base_path}/data').once() - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/data.temp', f'{base_path}/data/all' ).once() @@ -508,7 +508,7 @@ def test_upgrade_check_times_skips_already_upgraded_check_paths(): '/home/user/.local/state/borgmatic/checks/1234/archives/all' ) flexmock(module.os.path).should_receive('isfile').and_return(False) - flexmock(module.os).should_receive('rename').never() + flexmock(module.shutil).should_receive('move').never() flexmock(module.os).should_receive('mkdir').never() module.upgrade_check_times(flexmock(), flexmock()) @@ -542,11 +542,11 @@ def test_upgrade_check_times_renames_stale_temporary_check_path(): flexmock(module.os.path).should_receive('isfile').with_args( f'{base_path}/data.temp' ).and_return(False) - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/archives', f'{base_path}/archives.temp' ).and_raise(FileNotFoundError) flexmock(module.os).should_receive('mkdir').with_args(f'{base_path}/archives').once() - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( f'{base_path}/archives.temp', f'{base_path}/archives/all' ).once() diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 31393e0b..d9e3d9d6 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -75,10 +75,10 @@ def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matchin ] ) - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( '/foo/bar/postgresql_databases', '/run/user/0/borgmatic/postgresql_databases' ).once() - flexmock(module.os).should_receive('rename').with_args( + flexmock(module.shutil).should_receive('move').with_args( '/foo/bar/mariadb_databases', '/run/user/0/borgmatic/mariadb_databases' ).never()