From a31deebd1885fc8c18e29ce87b402712a6b4955a Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Tue, 7 Jul 2026 22:37:18 -0700 Subject: [PATCH 01/15] Create function to measure size of files in folder --- lib/filesystem.py | 19 +++++++++++++++++++ testing/test.py | 30 +++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/filesystem.py b/lib/filesystem.py index 5638f50e..44e19f20 100644 --- a/lib/filesystem.py +++ b/lib/filesystem.py @@ -295,3 +295,22 @@ def classify_path(path: Path) -> str: else "Folder" if path.is_dir() else "File" if path.is_file() else "Unknown") + + +def folder_size(directory: Path) -> int: + """ + Calculate the total size of all files in a directory tree. + + Arguments: + directory: A folder whose contents will be measured for size + + Returns: + int: The total size of all files in the directory tree. + """ + total_size = 0 + for folder, _, file_names in directory.walk(): + for file_name in file_names: + path = folder/file_name + total_size += path.stat(follow_symlinks=False).st_size + + return total_size diff --git a/testing/test.py b/testing/test.py index 64e93f36..b2980aa9 100644 --- a/testing/test.py +++ b/testing/test.py @@ -1966,13 +1966,7 @@ def used_space(self, path: Path | str) -> int: if not path.is_relative_to(self.base_path): raise ValueError(f"{path} is not a subdirectory of {self.base_path}") - total_used = 0 - for directory, _, file_names in self.base_path.walk(): - for file_name in file_names: - file_path = directory/file_name - total_used += file_path.stat(follow_symlinks=False).st_size - - return total_used + return fs.folder_size(self.base_path) def total_size(self) -> int: """Returns the total size of this mock drive.""" @@ -6621,3 +6615,25 @@ def error_chmod(*_: Any, **__: Any) -> Never: # noqa: ANN401 self.assertTrue(folder_path.is_dir()) self.assertEqual(error.exception.args, (error_message,)) os.chmod(folder_path, stat.S_IWRITE, follow_symlinks=False) # noqa: PTH101 + + def test_size_of_empty_folder_is_zero(self) -> None: + """Test that an empty folder has zero size.""" + self.assertEqual(fs.folder_size(self.user_path), 0) + + def test_size_of_folder_with_one_file_is_size_of_file(self) -> None: + """Test that the size of a folder with one file is the size of that file.""" + file_size = 10_000_000 + create_large_files(self.user_path, file_size) + self.assertEqual(fs.folder_size(self.user_path), file_size) + + def test_size_of_folder_with_subfolders_and_files_is_size_of_all_files_in_tree(self) -> None: + """Test that all files in a directory tree get summed to find total size.""" + for num in range(3): + (self.user_path/str(num)).mkdir() + + file_size = 5_000_000 + create_large_files(self.user_path, file_size) + base_file = self.user_path/"base_file.txt" + base_file.write_text(file_size*"B") + total_size = 4*file_size + self.assertEqual(fs.folder_size(self.user_path), total_size) From a049d95fc3f2c072b20d1be8b900da96d4607550 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Tue, 7 Jul 2026 23:14:02 -0700 Subject: [PATCH 02/15] Add function to find size of unique files Unique files are those that are not hardlinked to any other file. --- lib/filesystem.py | 20 ++++++++++++++++++++ testing/test.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/lib/filesystem.py b/lib/filesystem.py index 44e19f20..a4972265 100644 --- a/lib/filesystem.py +++ b/lib/filesystem.py @@ -314,3 +314,23 @@ def folder_size(directory: Path) -> int: total_size += path.stat(follow_symlinks=False).st_size return total_size + + +def unique_folder_size(directory: Path) -> int: + """ + Calculate the total size of all non-hardlinked files in a directory tree. + + Arguments: + directory: A folder whose contents will be measured for size + + Returns: + int: The total size of all files in the directory tree that are not hardlinks. + """ + total_size = 0 + for folder, _, file_names in directory.walk(): + for file_name in file_names: + path = folder/file_name + if path.stat().st_nlink == 1: + total_size += path.stat(follow_symlinks=False).st_size + + return total_size diff --git a/testing/test.py b/testing/test.py index b2980aa9..31d0d300 100644 --- a/testing/test.py +++ b/testing/test.py @@ -6637,3 +6637,46 @@ def test_size_of_folder_with_subfolders_and_files_is_size_of_all_files_in_tree(s base_file.write_text(file_size*"B") total_size = 4*file_size self.assertEqual(fs.folder_size(self.user_path), total_size) + + def test_unique_size_of_empty_folder_is_zero(self) -> None: + """Test that an empty folder has zero size.""" + self.assertEqual(fs.unique_folder_size(self.user_path), 0) + + def test_unique_size_of_folder_with_one_file_is_size_of_file(self) -> None: + """Test that the size of a folder with one file is the size of that file.""" + file_size = 10_000_000 + create_large_files(self.user_path, file_size) + self.assertEqual(fs.unique_folder_size(self.user_path), file_size) + + def test_unique_size_of_folder_with_subfolders_is_size_of_all_files_in_tree(self) -> None: + """Test that all files in a directory tree get summed to find total size.""" + for num in range(3): + (self.user_path/str(num)).mkdir() + + file_size = 5_000_000 + create_large_files(self.user_path, file_size) + base_file = self.user_path/"base_file.txt" + base_file.write_text(file_size*"B") + total_size = 4*file_size + self.assertEqual(fs.unique_folder_size(self.user_path), total_size) + + def test_unique_size_of_folder_with_all_files_hardlinked_is_zero(self) -> None: + """Test that the unique size of a folder with all files hardlinked is zero.""" + create_user_data(self.user_path) + for _ in range(2): + default_backup(self.user_path, self.backup_path) + + backups = util.all_backups(self.backup_path) + for backup in backups: + self.assertEqual(fs.unique_folder_size(backup), 0) + + def test_unique_size_of_folder_with_nearly_all_files_hardlinked_is_zero(self) -> None: + """Test that the unique size of a folder one unique file is the size of the unique file.""" + create_user_data(self.user_path) + for _ in range(2): + default_backup(self.user_path, self.backup_path) + + backups = util.all_backups(self.backup_path) + file_size = 50 + (backups[0]/"unique.txt").write_text(file_size*"L") + self.assertEqual(fs.unique_folder_size(backups[0]), file_size) From 8a2c4a7a2da4a2505781f4037db449595ae77c82 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Tue, 7 Jul 2026 23:16:54 -0700 Subject: [PATCH 03/15] Implement --free-up auto --- lib/argument_parser.py | 5 ++++- lib/main.py | 17 ++++++++++++++--- testing/test.py | 42 ++++++++++++++++++++++++++++++++++++++++++ wiki/delete.md | 10 +++++++++- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/lib/argument_parser.py b/lib/argument_parser.py index a34cac0d..b8e15c69 100644 --- a/lib/argument_parser.py +++ b/lib/argument_parser.py @@ -367,7 +367,10 @@ def argument_parser() -> argparse.ArgumentParser: indicate a unit in bytes. The number will be interpreted as a number of bytes. Case does not matter, so all of the following specify 15 megabytes: 15MB, 15Mb, 15mB, 15mb, 15M, and 15m. Old backups -will be deleted until at least that much space is free.""")) +will be deleted until at least that much space is free. + +Alternatively, this argument can be "auto". This will cause Vintage Backup to delete old backups +only when creating a new backup fails due to the backup media to running out of space.""")) deletion_group.add_argument("--delete-after", metavar="TIME", help=format_help( """After a successful backup, delete backups if they are older than the time span in the argument. diff --git a/lib/main.py b/lib/main.py index 54355d45..1d2542d6 100644 --- a/lib/main.py +++ b/lib/main.py @@ -6,14 +6,14 @@ from lib.argument_parser import parse_command_line, print_help, print_usage, toggle_is_set from lib.automation import generate_windows_scripts -from lib.backup import start_backup, print_backup_storage_stats +from lib.backup import start_backup, print_backup_storage_stats, backup_staging_folder from lib.backup_deletion import delete_old_backups from lib.backup_set import preview_filter from lib.backup_utilities import all_backups from lib.configuration import generate_config from lib.console import print_run_title import lib.exceptions as exc -from lib.filesystem import absolute_path, parse_storage_space +from lib.filesystem import absolute_path, parse_storage_space, unique_folder_size from lib.logs import setup_initial_null_logger, setup_log_file from lib.move_backups import start_move_backups from lib.purge import choose_purge_target_from_backups, start_backup_purge @@ -53,6 +53,10 @@ def backup_cycle(args: argparse.Namespace) -> None: CommandLineError: If the backup storage media runs out of space and --free-up cannot delete enough old backups to make room """ + auto_free_up = (args.free_up == "auto") + if auto_free_up: + args.free_up = "" + while True: try: delete_old_backups(args) @@ -63,9 +67,16 @@ def backup_cycle(args: argparse.Namespace) -> None: raise logger.warning("Could not complete backup. %s", error) - free_up_space = parse_storage_space(args.free_up) backup_location = absolute_path(args.backup_folder) free_space = shutil.disk_usage(backup_location).free + + if auto_free_up: + staging_space = unique_folder_size(backup_staging_folder(backup_location)) + if not staging_space: + staging_space = free_space or 1000 + args.free_up = str(free_space + staging_space) + + free_up_space = parse_storage_space(args.free_up) if free_up_space < free_space: raise exc.CommandLineError( "Cannot free up enough space to complete backup. " diff --git a/testing/test.py b/testing/test.py index 31d0d300..a16e236a 100644 --- a/testing/test.py +++ b/testing/test.py @@ -2498,6 +2498,48 @@ def test_error_raised_when_no_free_up_and_and_no_space(self) -> None: self.assertRaises(OutOfSpaceError)): main.default_action(args) + def test_free_up_auto_does_nothing_when_enough_space_for_backup(self) -> None: + """Test that --free-up auto does nothing when there is sufficient space for a backup.""" + create_user_data(self.user_path) + data_size = fs.folder_size(self.user_path) + backup_count = 4 + mock_storage = DiskUsageMock(self.backup_path, (backup_count + 1)*data_size) + for _ in range(backup_count): + with (patch("lib.backup.shutil.disk_usage", mock_storage), + patch("lib.backup_deletion.shutil.disk_usage", mock_storage), + patch("lib.backup.datetime", Now_Mock())): + main_assert_no_error_log([ + "-u", str(self.user_path), + "-b", str(self.backup_path), + "--force-copy", + "--free-up", "auto"], + self) + + all_backups = util.all_backups(self.backup_path) + self.assertEqual(len(all_backups), backup_count) + + def test_free_up_auto_deletes_old_backups_when_not_enough_space_for_backup(self) -> None: + """Test that --free-up auto deletes old backups to make room for new ones.""" + create_user_data(self.user_path) + data_size = fs.folder_size(self.user_path) + backup_count = 5 + backup_storage_count = 3 + storage_space = int((backup_storage_count + 0.5)*data_size) + mock_storage = DiskUsageMock(self.backup_path, storage_space) + for _ in range(backup_count): + with (patch("lib.backup.shutil.disk_usage", mock_storage), + patch("lib.backup_deletion.shutil.disk_usage", mock_storage), + patch("lib.backup.shutil.copy2", MockCopy2()), + patch("lib.backup.datetime", Now_Mock())): + main_no_log([ + "-u", str(self.user_path), + "-b", str(self.backup_path), + "--force-copy", + "--free-up", "auto"]) + + all_backups = util.all_backups(self.backup_path) + self.assertEqual(len(all_backups), backup_storage_count) + class MoveBackupsTests(TestCaseWithTemporaryFilesAndFolders): """Test moving backup sets to a different location.""" diff --git a/wiki/delete.md b/wiki/delete.md index 0ca96b92..3d32976c 100644 --- a/wiki/delete.md +++ b/wiki/delete.md @@ -13,7 +13,7 @@ Backup deletions after a backup ensure that most of the time the next backup can Specify how much disk space should be kept free at the backup location. If there is less space before or after a backup, old backups will be deleted until this amount of space is free. -This parameter can be just a number or a number with a byte unit. +This parameter can be just a number, a number with a byte unit, or the word `auto`. For example, `--free-up "10 GB"` @@ -27,6 +27,14 @@ If there is space between the number and unit like `10 GB`, then the whole param This size of this parameter should be an overestimate of the space needed for each backup. This depends on how much new data is added between backups and how often files are copied instead of hard-linked (see the [`--hard-link-count`](backup.md#--hard-link-count) and [`--copy-probability`](backup.md#--copy-probability) parameters). +If the parameter is + +`--free-up auto` + +then old backups are only deleted when the backup location cannot complete a backup due to running out of space. +This option maximizes the use of space at the backup location. +However, this can increase the amount of time a backup takes to complete since multiple attempts may be required to create a backup if a lot of new data has been added to the user's folder. + If the backup storage media runs out of space during a backup and this parameter is used, then 1. The backup process will abort, 2. Old backups will be deleted according to `--free-up`, and From 3950de821dc1c75bb29ef24363bd10ff125b7a09 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Wed, 8 Jul 2026 20:53:52 -0700 Subject: [PATCH 04/15] Delete oldest backup with --free-up auto Instead of complicated calculations, just delete the oldest backup when using --free-up auto. --- lib/backup_deletion.py | 22 +++++++++++++++++++++ lib/filesystem.py | 20 -------------------- lib/main.py | 14 +++++++------- testing/test.py | 43 ------------------------------------------ 4 files changed, 29 insertions(+), 70 deletions(-) diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index eaa305c8..bf72ec86 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -134,6 +134,28 @@ def delete_single_backup(backup: Path, verify_checksum_result_folder: Path | Non logger.info("Free space: %s", fs.byte_units(shutil.disk_usage(backup.parent.parent).free)) +def delete_oldest_backup(backup_location: Path, verify_checksum_result_folder: Path | None) -> None: + """ + Delete the oldest backup at the specified location. + + Arguments: + backup_location: The base directory holding all dated backups. + verify_checksum_result_folder: If the checksum of the backup is being verified prior to + deletion, put the verification result files in this folder. + + Raises: + CommandLineError: If there are no backups to delete or one remaining backup. + """ + backups = util.all_backups(backup_location) + if not backups: + raise CommandLineError("No backups to delete.") + + if len(backups) == 1: + raise CommandLineError("Last remaining backup will not be deleted.") + + delete_single_backup(backups[0], verify_checksum_result_folder) + + def delete_backups( backup_folder: Path, min_backups_remaining: int, diff --git a/lib/filesystem.py b/lib/filesystem.py index a4972265..44e19f20 100644 --- a/lib/filesystem.py +++ b/lib/filesystem.py @@ -314,23 +314,3 @@ def folder_size(directory: Path) -> int: total_size += path.stat(follow_symlinks=False).st_size return total_size - - -def unique_folder_size(directory: Path) -> int: - """ - Calculate the total size of all non-hardlinked files in a directory tree. - - Arguments: - directory: A folder whose contents will be measured for size - - Returns: - int: The total size of all files in the directory tree that are not hardlinks. - """ - total_size = 0 - for folder, _, file_names in directory.walk(): - for file_name in file_names: - path = folder/file_name - if path.stat().st_nlink == 1: - total_size += path.stat(follow_symlinks=False).st_size - - return total_size diff --git a/lib/main.py b/lib/main.py index 1d2542d6..9e3368cb 100644 --- a/lib/main.py +++ b/lib/main.py @@ -6,14 +6,14 @@ from lib.argument_parser import parse_command_line, print_help, print_usage, toggle_is_set from lib.automation import generate_windows_scripts -from lib.backup import start_backup, print_backup_storage_stats, backup_staging_folder -from lib.backup_deletion import delete_old_backups +from lib.backup import start_backup, print_backup_storage_stats +from lib.backup_deletion import delete_old_backups, delete_oldest_backup from lib.backup_set import preview_filter from lib.backup_utilities import all_backups from lib.configuration import generate_config from lib.console import print_run_title import lib.exceptions as exc -from lib.filesystem import absolute_path, parse_storage_space, unique_folder_size +from lib.filesystem import absolute_path, parse_storage_space, path_or_none from lib.logs import setup_initial_null_logger, setup_log_file from lib.move_backups import start_move_backups from lib.purge import choose_purge_target_from_backups, start_backup_purge @@ -71,10 +71,10 @@ def backup_cycle(args: argparse.Namespace) -> None: free_space = shutil.disk_usage(backup_location).free if auto_free_up: - staging_space = unique_folder_size(backup_staging_folder(backup_location)) - if not staging_space: - staging_space = free_space or 1000 - args.free_up = str(free_space + staging_space) + backup_folder = absolute_path(args.backup_folder) + verify_checksum_result_folder = path_or_none(args.verify_checksum_before_deletion) + delete_oldest_backup(backup_folder, verify_checksum_result_folder) + continue free_up_space = parse_storage_space(args.free_up) if free_up_space < free_space: diff --git a/testing/test.py b/testing/test.py index a16e236a..70366213 100644 --- a/testing/test.py +++ b/testing/test.py @@ -6679,46 +6679,3 @@ def test_size_of_folder_with_subfolders_and_files_is_size_of_all_files_in_tree(s base_file.write_text(file_size*"B") total_size = 4*file_size self.assertEqual(fs.folder_size(self.user_path), total_size) - - def test_unique_size_of_empty_folder_is_zero(self) -> None: - """Test that an empty folder has zero size.""" - self.assertEqual(fs.unique_folder_size(self.user_path), 0) - - def test_unique_size_of_folder_with_one_file_is_size_of_file(self) -> None: - """Test that the size of a folder with one file is the size of that file.""" - file_size = 10_000_000 - create_large_files(self.user_path, file_size) - self.assertEqual(fs.unique_folder_size(self.user_path), file_size) - - def test_unique_size_of_folder_with_subfolders_is_size_of_all_files_in_tree(self) -> None: - """Test that all files in a directory tree get summed to find total size.""" - for num in range(3): - (self.user_path/str(num)).mkdir() - - file_size = 5_000_000 - create_large_files(self.user_path, file_size) - base_file = self.user_path/"base_file.txt" - base_file.write_text(file_size*"B") - total_size = 4*file_size - self.assertEqual(fs.unique_folder_size(self.user_path), total_size) - - def test_unique_size_of_folder_with_all_files_hardlinked_is_zero(self) -> None: - """Test that the unique size of a folder with all files hardlinked is zero.""" - create_user_data(self.user_path) - for _ in range(2): - default_backup(self.user_path, self.backup_path) - - backups = util.all_backups(self.backup_path) - for backup in backups: - self.assertEqual(fs.unique_folder_size(backup), 0) - - def test_unique_size_of_folder_with_nearly_all_files_hardlinked_is_zero(self) -> None: - """Test that the unique size of a folder one unique file is the size of the unique file.""" - create_user_data(self.user_path) - for _ in range(2): - default_backup(self.user_path, self.backup_path) - - backups = util.all_backups(self.backup_path) - file_size = 50 - (backups[0]/"unique.txt").write_text(file_size*"L") - self.assertEqual(fs.unique_folder_size(backups[0]), file_size) From a99ce6a6a263fe6aeac9a993605320ecfc09ce5e Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Wed, 8 Jul 2026 21:02:52 -0700 Subject: [PATCH 05/15] Assert no errors with --free-up auto Found program bug where no new backups would be made after running out of space. This was due to a logic error in the except branch that prevented the --free-up auto logic from running. --- lib/main.py | 13 ++++++------- testing/test.py | 4 +++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/main.py b/lib/main.py index 9e3368cb..215716f9 100644 --- a/lib/main.py +++ b/lib/main.py @@ -63,19 +63,18 @@ def backup_cycle(args: argparse.Namespace) -> None: start_backup(args) break except exc.OutOfSpaceError as error: - if not args.free_up: - raise - - logger.warning("Could not complete backup. %s", error) - backup_location = absolute_path(args.backup_folder) - free_space = shutil.disk_usage(backup_location).free - if auto_free_up: backup_folder = absolute_path(args.backup_folder) verify_checksum_result_folder = path_or_none(args.verify_checksum_before_deletion) delete_oldest_backup(backup_folder, verify_checksum_result_folder) continue + if not args.free_up: + raise + + logger.warning("Could not complete backup. %s", error) + backup_location = absolute_path(args.backup_folder) + free_space = shutil.disk_usage(backup_location).free free_up_space = parse_storage_space(args.free_up) if free_up_space < free_space: raise exc.CommandLineError( diff --git a/testing/test.py b/testing/test.py index 70366213..8c58539b 100644 --- a/testing/test.py +++ b/testing/test.py @@ -2531,12 +2531,14 @@ def test_free_up_auto_deletes_old_backups_when_not_enough_space_for_backup(self) patch("lib.backup_deletion.shutil.disk_usage", mock_storage), patch("lib.backup.shutil.copy2", MockCopy2()), patch("lib.backup.datetime", Now_Mock())): - main_no_log([ + exit_code = main_no_log([ "-u", str(self.user_path), "-b", str(self.backup_path), "--force-copy", "--free-up", "auto"]) + self.assertEqual(exit_code, 0) + all_backups = util.all_backups(self.backup_path) self.assertEqual(len(all_backups), backup_storage_count) From c304f11610c932ab3a3b190fddfcded8767124d1 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Wed, 8 Jul 2026 21:05:06 -0700 Subject: [PATCH 06/15] Put line back in original location --- lib/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/main.py b/lib/main.py index 215716f9..4b0dae51 100644 --- a/lib/main.py +++ b/lib/main.py @@ -73,9 +73,9 @@ def backup_cycle(args: argparse.Namespace) -> None: raise logger.warning("Could not complete backup. %s", error) + free_up_space = parse_storage_space(args.free_up) backup_location = absolute_path(args.backup_folder) free_space = shutil.disk_usage(backup_location).free - free_up_space = parse_storage_space(args.free_up) if free_up_space < free_space: raise exc.CommandLineError( "Cannot free up enough space to complete backup. " From 3a915cf3156074fa6d12d6f73025fdb93d01d09b Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Wed, 8 Jul 2026 21:24:49 -0700 Subject: [PATCH 07/15] folder_size() doesn't overcount hardlinked files Add tests to confirm. --- lib/filesystem.py | 7 ++++--- testing/test.py | 12 ++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/lib/filesystem.py b/lib/filesystem.py index 44e19f20..8332d40c 100644 --- a/lib/filesystem.py +++ b/lib/filesystem.py @@ -307,10 +307,11 @@ def folder_size(directory: Path) -> int: Returns: int: The total size of all files in the directory tree. """ - total_size = 0 + inode_sizes: dict[int, int] = {} # inode --> size of file for folder, _, file_names in directory.walk(): for file_name in file_names: path = folder/file_name - total_size += path.stat(follow_symlinks=False).st_size + stat = path.stat(follow_symlinks=False) + inode_sizes[stat.st_ino] = stat.st_size - return total_size + return sum(inode_sizes.values()) diff --git a/testing/test.py b/testing/test.py index 8c58539b..e34d0648 100644 --- a/testing/test.py +++ b/testing/test.py @@ -6681,3 +6681,15 @@ def test_size_of_folder_with_subfolders_and_files_is_size_of_all_files_in_tree(s base_file.write_text(file_size*"B") total_size = 4*file_size self.assertEqual(fs.folder_size(self.user_path), total_size) + + def test_size_of_folder_tree_with_hard_links_does_not_double_count(self) -> None: + """Test that the sizes of files that are hardlinked together are not double-counted.""" + create_user_data(self.user_path) + default_backup(self.user_path, self.backup_path) + single_backup_size = fs.folder_size(self.backup_path) + + # All new files at backup location are hardlinked to first backup. + default_backup(self.user_path, self.backup_path) + all_backups_size = fs.folder_size(self.backup_path) + + self.assertEqual(single_backup_size, all_backups_size) From 7c6572842a85f31c07b42e9a454eea9e51f0dcf7 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Thu, 9 Jul 2026 19:20:01 -0700 Subject: [PATCH 08/15] delete_oldest_backup() respects --max-deletions --- lib/backup_deletion.py | 10 +++++++++- lib/main.py | 12 ++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index bf72ec86..9d1d0b6c 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -292,12 +292,14 @@ def check_time_span_parameters(args: argparse.Namespace) -> None: last_time_span_str = time_span_str -def delete_old_backups(args: argparse.Namespace) -> None: +def delete_old_backups(args: argparse.Namespace, *, delete_oldest: bool = False) -> None: """ Delete the oldest backups by various criteria in the command line options. Arguments: args: Parsed command line + delete_oldest: Whether to delete the oldest backup first. The argument --max-deletions is + still respected. Note: The argument `args.max_deletions` is reduced by the number of deletions to make sure that option is respected if multiple rounds of backup deletions are required. @@ -318,10 +320,16 @@ def delete_old_backups(args: argparse.Namespace) -> None: verify_checksum_result_folder = fs.path_or_none(args.verify_checksum_before_deletion) max_deletions = int(args.max_deletions or backup_count) min_backups_remaining = max(backup_count - max_deletions, 1) + + if delete_oldest and backup_count > min_backups_remaining: + delete_oldest_backup(backup_folder, verify_checksum_result_folder) + delete_too_frequent_backups( backup_folder, args, min_backups_remaining, verify_checksum_result_folder) + delete_oldest_backups_for_space( backup_folder, args.free_up, verify_checksum_result_folder, min_backups_remaining) + delete_backups_older_than( backup_folder, args.delete_after, verify_checksum_result_folder, min_backups_remaining) diff --git a/lib/main.py b/lib/main.py index 4b0dae51..5c303eec 100644 --- a/lib/main.py +++ b/lib/main.py @@ -7,13 +7,13 @@ from lib.argument_parser import parse_command_line, print_help, print_usage, toggle_is_set from lib.automation import generate_windows_scripts from lib.backup import start_backup, print_backup_storage_stats -from lib.backup_deletion import delete_old_backups, delete_oldest_backup +from lib.backup_deletion import delete_old_backups from lib.backup_set import preview_filter from lib.backup_utilities import all_backups from lib.configuration import generate_config from lib.console import print_run_title import lib.exceptions as exc -from lib.filesystem import absolute_path, parse_storage_space, path_or_none +from lib.filesystem import absolute_path, parse_storage_space from lib.logs import setup_initial_null_logger, setup_log_file from lib.move_backups import start_move_backups from lib.purge import choose_purge_target_from_backups, start_backup_purge @@ -56,17 +56,17 @@ def backup_cycle(args: argparse.Namespace) -> None: auto_free_up = (args.free_up == "auto") if auto_free_up: args.free_up = "" + delete_oldest = False while True: try: - delete_old_backups(args) + delete_old_backups(args, delete_oldest=delete_oldest) + delete_oldest = False start_backup(args) break except exc.OutOfSpaceError as error: if auto_free_up: - backup_folder = absolute_path(args.backup_folder) - verify_checksum_result_folder = path_or_none(args.verify_checksum_before_deletion) - delete_oldest_backup(backup_folder, verify_checksum_result_folder) + delete_oldest = True continue if not args.free_up: From f0244a6ad5d29d41e0ed982bd5ea8c07d93db1aa Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Thu, 9 Jul 2026 19:31:54 -0700 Subject: [PATCH 09/15] Error if --free-up auto fails to delete a backup --- lib/backup_deletion.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index 9d1d0b6c..0211329f 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -134,12 +134,17 @@ def delete_single_backup(backup: Path, verify_checksum_result_folder: Path | Non logger.info("Free space: %s", fs.byte_units(shutil.disk_usage(backup.parent.parent).free)) -def delete_oldest_backup(backup_location: Path, verify_checksum_result_folder: Path | None) -> None: +def delete_oldest_backup( + backup_location: Path, + min_backups_remaining: int, + verify_checksum_result_folder: Path | None) -> None: """ Delete the oldest backup at the specified location. Arguments: backup_location: The base directory holding all dated backups. + min_backups_remaining: The minimum number of backups that should remain after deletion + operations. verify_checksum_result_folder: If the checksum of the backup is being verified prior to deletion, put the verification result files in this folder. @@ -153,6 +158,9 @@ def delete_oldest_backup(backup_location: Path, verify_checksum_result_folder: P if len(backups) == 1: raise CommandLineError("Last remaining backup will not be deleted.") + if len(backups) <= min_backups_remaining: + raise CommandLineError("Reached maximum number of backup deletions this session.") + delete_single_backup(backups[0], verify_checksum_result_folder) @@ -321,8 +329,9 @@ def delete_old_backups(args: argparse.Namespace, *, delete_oldest: bool = False) max_deletions = int(args.max_deletions or backup_count) min_backups_remaining = max(backup_count - max_deletions, 1) - if delete_oldest and backup_count > min_backups_remaining: - delete_oldest_backup(backup_folder, verify_checksum_result_folder) + if delete_oldest: + delete_oldest_backup( + backup_folder, min_backups_remaining, verify_checksum_result_folder) delete_too_frequent_backups( backup_folder, args, min_backups_remaining, verify_checksum_result_folder) From c161ee91993844a34cd396737d997ba689510025 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Thu, 9 Jul 2026 19:51:46 -0700 Subject: [PATCH 10/15] Add test for --free-up auto error state --- testing/test.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/testing/test.py b/testing/test.py index e34d0648..117ba938 100644 --- a/testing/test.py +++ b/testing/test.py @@ -2542,6 +2542,25 @@ def test_free_up_auto_deletes_old_backups_when_not_enough_space_for_backup(self) all_backups = util.all_backups(self.backup_path) self.assertEqual(len(all_backups), backup_storage_count) + def test_free_up_auto_raises_error_if_backup_cannot_be_deleted(self) -> None: + """Test that an exception is raised if --free-up auto fails to delete a backup.""" + create_user_data(self.user_path) + data_size = fs.folder_size(self.user_path) + mock_storage = DiskUsageMock(self.backup_path, int(1.5*data_size)) + default_backup(self.user_path, self.backup_path) + with (patch("lib.backup.shutil.disk_usage", mock_storage), + patch("lib.backup.shutil.copy2", MockCopy2()), + patch("lib.backup_deletion.shutil.disk_usage", mock_storage), + self.assertLogs(level=logging.ERROR) as logs): + exit_code = main_no_log([ + "-u", str(self.user_path), + "-b", str(self.backup_path), + "--force-copy", + "--free-up", "auto"]) + + self.assertEqual(exit_code, 1) + self.assertEqual(logs.output, ["ERROR:root:Last remaining backup will not be deleted."]) + class MoveBackupsTests(TestCaseWithTemporaryFilesAndFolders): """Test moving backup sets to a different location.""" From 0488c4f4d92856fd8ae116d71e359264de4b9a6a Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Thu, 9 Jul 2026 21:03:05 -0700 Subject: [PATCH 11/15] Simplify handling of --free-up auto --- lib/backup.py | 2 ++ lib/backup_deletion.py | 2 +- lib/main.py | 5 +---- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/backup.py b/lib/backup.py index 44ed94c2..517bccb5 100644 --- a/lib/backup.py +++ b/lib/backup.py @@ -713,6 +713,8 @@ def log_backup_size(free_up_parameter: str | None, backup_space_taken: int) -> N free_up_parameter: The value given to the --free-up command line option backup_space_taken: The space taken by the most recent backup """ + if free_up_parameter == "auto": + free_up_parameter = "" free_up = fs.parse_storage_space(free_up_parameter or "0") free_up_percent = math.ceil(100*backup_space_taken/free_up) if free_up else 0 free_up_text = f" ({free_up_percent}% of --free-up)" if free_up else "" diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index 0211329f..bb605a37 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -41,7 +41,7 @@ def delete_oldest_backups_for_space( Raises: CommandLineError: If the --free-up parameter is larger than the entire backup storage media. """ - if not space_requirement: + if not space_requirement or space_requirement == "auto": return total_storage = shutil.disk_usage(backup_location).total diff --git a/lib/main.py b/lib/main.py index 5c303eec..c1e393b5 100644 --- a/lib/main.py +++ b/lib/main.py @@ -53,9 +53,6 @@ def backup_cycle(args: argparse.Namespace) -> None: CommandLineError: If the backup storage media runs out of space and --free-up cannot delete enough old backups to make room """ - auto_free_up = (args.free_up == "auto") - if auto_free_up: - args.free_up = "" delete_oldest = False while True: @@ -65,7 +62,7 @@ def backup_cycle(args: argparse.Namespace) -> None: start_backup(args) break except exc.OutOfSpaceError as error: - if auto_free_up: + if args.free_up == "auto": delete_oldest = True continue From 6e6088553a02afc5cffca738787dfb002ebfb400 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Thu, 9 Jul 2026 22:11:07 -0700 Subject: [PATCH 12/15] Log backup deletion --- lib/backup_deletion.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index bb605a37..1c27ac51 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -161,7 +161,9 @@ def delete_oldest_backup( if len(backups) <= min_backups_remaining: raise CommandLineError("Reached maximum number of backup deletions this session.") - delete_single_backup(backups[0], verify_checksum_result_folder) + oldest_backup = backups[0] + logger.info("Deleting oldest backup: %s", oldest_backup) + delete_single_backup(oldest_backup, verify_checksum_result_folder) def delete_backups( From c0db6715281a66f87c7b94ed4e20a889d8e33ec1 Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Sat, 11 Jul 2026 16:40:16 -0700 Subject: [PATCH 13/15] Add tests for delete_oldest_backup() --- testing/test.py | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/testing/test.py b/testing/test.py index 117ba938..53108d20 100644 --- a/testing/test.py +++ b/testing/test.py @@ -2561,6 +2561,64 @@ def test_free_up_auto_raises_error_if_backup_cannot_be_deleted(self) -> None: self.assertEqual(exit_code, 1) self.assertEqual(logs.output, ["ERROR:root:Last remaining backup will not be deleted."]) + def test_delete_oldest_backup_deletes_oldest_backup(self) -> None: + """Test that delete_oldest_backup() deletes only the oldest backup.""" + create_old_daily_backups(self.backup_path, 10) + backups = util.all_backups(self.backup_path) + expected_backups = backups[1:] + deletion.delete_oldest_backup(self.backup_path, 0, None) + remaining_backups = util.all_backups(self.backup_path) + self.assertEqual(expected_backups, remaining_backups) + + def test_delete_oldest_backup_raises_exception_if_no_backups(self) -> None: + """Test delete_oldest_backup() raises an exception if there are no backups to delete.""" + with self.assertRaises(CommandLineError) as error: + deletion.delete_oldest_backup(self.backup_path, 0, None) + self.assertEqual(error.exception.args, ("No backups to delete.",)) + + def test_delete_oldest_backup_raises_exception_if_only_one_backup(self) -> None: + """Test delete_oldest_backup() raises an exception if there is one backup left.""" + create_old_monthly_backups(self.backup_path, 1) + with self.assertRaises(CommandLineError) as error: + deletion.delete_oldest_backup(self.backup_path, 0, None) + self.assertEqual(error.exception.args, ("Last remaining backup will not be deleted.",)) + + def test_delete_oldest_backup_raises_exception_if_max_deletions_reached(self) -> None: + """Test delete_oldest_backup() raises an exception if there is one backup left.""" + backup_count = 10 + create_old_monthly_backups(self.backup_path, backup_count) + with self.assertRaises(CommandLineError) as error: + deletion.delete_oldest_backup(self.backup_path, backup_count, None) + self.assertEqual( + error.exception.args, + ("Reached maximum number of backup deletions this session.",)) + + def test_delete_oldest_backup_verifies_checksum(self) -> None: + """Test that delete_oldest_backup() verifies a checksum file if one exists.""" + create_user_data(self.user_path) + with patch("lib.backup.datetime", Now_Mock()): + exit_code = main_no_log([ + "-u", str(self.user_path), + "-b", str(self.backup_path), + "--checksum"]) + self.assertEqual(exit_code, 0) + + oldest_backup, = util.all_backups(self.backup_path) + checksum_file = oldest_backup/verify.checksum_file_name + self.assertTrue(checksum_file.exists()) + + changed_file = oldest_backup/"root_file.txt" + self.assertTrue(changed_file.exists()) + changed_file.write_text("corrupted", encoding="utf8") + + default_backup(self.user_path, self.backup_path) + deletion.delete_oldest_backup(self.backup_path, 0, self.user_path) + + checksum_verify_file = self.user_path/verify.verify_checksum_file_name + self.assertTrue(checksum_verify_file.exists()) + _, line, _ = checksum_verify_file.read_text(encoding="utf8").split("\n") + self.assertTrue(line.startswith(str(changed_file.relative_to(oldest_backup)))) + class MoveBackupsTests(TestCaseWithTemporaryFilesAndFolders): """Test moving backup sets to a different location.""" From 05adac88a2283c5bad2cd3f0dd8ec18fb2dc922d Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Sat, 11 Jul 2026 17:24:17 -0700 Subject: [PATCH 14/15] Make --free-up auto action more clear in wiki --- wiki/delete.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wiki/delete.md b/wiki/delete.md index 3d32976c..0c1e8d55 100644 --- a/wiki/delete.md +++ b/wiki/delete.md @@ -14,6 +14,7 @@ Backup deletions after a backup ensure that most of the time the next backup can Specify how much disk space should be kept free at the backup location. If there is less space before or after a backup, old backups will be deleted until this amount of space is free. This parameter can be just a number, a number with a byte unit, or the word `auto`. + For example, `--free-up "10 GB"` @@ -33,7 +34,7 @@ If the parameter is then old backups are only deleted when the backup location cannot complete a backup due to running out of space. This option maximizes the use of space at the backup location. -However, this can increase the amount of time a backup takes to complete since multiple attempts may be required to create a backup if a lot of new data has been added to the user's folder. +However, this can increase the amount of time a backup takes to complete since multiple deletions may be required to create enough space to make a new a backup. If the backup storage media runs out of space during a backup and this parameter is used, then 1. The backup process will abort, From bf39a71c1f692684694a8e9b8657c4a1f42a461a Mon Sep 17 00:00:00 2001 From: Mark Harrison Date: Sun, 12 Jul 2026 13:42:49 -0700 Subject: [PATCH 15/15] More logging messages - Restarting backup after running out of space - Free space after deleting staging folder - Blank lines between operations --- lib/backup.py | 1 + lib/backup_deletion.py | 3 ++- lib/filesystem.py | 5 +++++ lib/main.py | 4 ++++ 4 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/backup.py b/lib/backup.py index 517bccb5..a95bdb97 100644 --- a/lib/backup.py +++ b/lib/backup.py @@ -413,6 +413,7 @@ def create_new_backup( logger.info("There is a staging folder leftover from previous incomplete backup.") logger.info("Deleting %s ...", staging_backup_path) fs.delete_directory_tree(staging_backup_path) + fs.log_free_space(backup_location) backup_info.confirm_user_location_is_unchanged(user_data_location, backup_location) backup_info.record_user_location(user_data_location, backup_location) diff --git a/lib/backup_deletion.py b/lib/backup_deletion.py index 1c27ac51..fa6e85fd 100644 --- a/lib/backup_deletion.py +++ b/lib/backup_deletion.py @@ -131,7 +131,7 @@ def delete_single_backup(backup: Path, verify_checksum_result_folder: Path | Non except OSError: pass - logger.info("Free space: %s", fs.byte_units(shutil.disk_usage(backup.parent.parent).free)) + fs.log_free_space(backup.parent.parent) def delete_oldest_backup( @@ -162,6 +162,7 @@ def delete_oldest_backup( raise CommandLineError("Reached maximum number of backup deletions this session.") oldest_backup = backups[0] + logger.info("") logger.info("Deleting oldest backup: %s", oldest_backup) delete_single_backup(oldest_backup, verify_checksum_result_folder) diff --git a/lib/filesystem.py b/lib/filesystem.py index 8332d40c..3705c9c5 100644 --- a/lib/filesystem.py +++ b/lib/filesystem.py @@ -221,6 +221,11 @@ def parse_storage_space(space_requirement: str) -> float: raise CommandLineError(f"Invalid storage space value: {space_requirement}") from None +def log_free_space(path: Path) -> None: + """Log the amount of free space at a location.""" + logger.info("Free space: %s", byte_units(shutil.disk_usage(path).free)) + + def write_directory(output: TextIO, directory: Path, file_names: list[str]) -> None: """ Write the full path of a directory followed by a list of files it contains. diff --git a/lib/main.py b/lib/main.py index c1e393b5..355e4fed 100644 --- a/lib/main.py +++ b/lib/main.py @@ -58,11 +58,15 @@ def backup_cycle(args: argparse.Namespace) -> None: while True: try: delete_old_backups(args, delete_oldest=delete_oldest) + if delete_oldest: + logger.info("") + logger.info("Restarting backup") delete_oldest = False start_backup(args) break except exc.OutOfSpaceError as error: if args.free_up == "auto": + logger.info("Not enough space to complete backup.") delete_oldest = True continue