From 28fa9c443b3f68f8a085e1155e537d12685ea8d3 Mon Sep 17 00:00:00 2001 From: Michael Deyaso Date: Fri, 10 Feb 2023 09:56:41 +0300 Subject: [PATCH 1/4] Modified Item.pyx to include diffs in ctime and mtime. Fixes #7248 --- - Modified Item.pyx to return ctime and mtime attributes on diff - Modified diff_cmd to sort its JSON output alphabetically --- src/borg/archive.py | 3 ++- src/borg/archiver/diff_cmd.py | 10 ++++++-- src/borg/helpers/parseformat.py | 2 +- src/borg/item.pyx | 19 +++++++++++--- src/borg/testsuite/archiver/__init__.py | 5 ++++ src/borg/testsuite/archiver/diff_cmd.py | 33 ++++++++++++++++++++++--- 6 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/borg/archive.py b/src/borg/archive.py index 12b2c847a1..ecbbea5a13 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -1096,7 +1096,7 @@ def chunk_decref(id, stats): logger.warning("borg check --repair is required to free all space.") @staticmethod - def compare_archives_iter(archive1, archive2, matcher=None, can_compare_chunk_ids=False): + def compare_archives_iter(archive1, archive2, matcher=None, can_compare_chunk_ids=False, content_only=False): """ Yields tuples with a path and an ItemDiff instance describing changes/indicating equality. @@ -1111,6 +1111,7 @@ def compare_items(item1, item2): archive1.pipeline.fetch_many([c.id for c in item1.get("chunks", [])]), archive2.pipeline.fetch_many([c.id for c in item2.get("chunks", [])]), can_compare_chunk_ids=can_compare_chunk_ids, + content_only=content_only, ) orphans_archive1 = OrderedDict() diff --git a/src/borg/archiver/diff_cmd.py b/src/borg/archiver/diff_cmd.py index cf0499411e..40b2f5f489 100644 --- a/src/borg/archiver/diff_cmd.py +++ b/src/borg/archiver/diff_cmd.py @@ -6,6 +6,7 @@ from ..constants import * # NOQA from ..helpers import archivename_validator from ..manifest import Manifest +from ..helpers.parseformat import BorgJsonEncoder from ..logger import create_logger @@ -19,7 +20,7 @@ def do_diff(self, args, repository, manifest, archive): """Diff contents of two archives""" def print_json_output(diff, path): - print(json.dumps({"path": path, "changes": [j for j, str in diff]})) + print(json.dumps({"path": path, "changes": [j for j, str in diff]}, sort_keys=True, cls=BorgJsonEncoder)) def print_text_output(diff, path): print("{:<19} {}".format(" ".join([str for j, str in diff]), path)) @@ -42,7 +43,9 @@ def print_text_output(diff, path): matcher = build_matcher(args.patterns, args.paths) - diffs = Archive.compare_archives_iter(archive1, archive2, matcher, can_compare_chunk_ids=can_compare_chunk_ids) + diffs = Archive.compare_archives_iter( + archive1, archive2, matcher, can_compare_chunk_ids=can_compare_chunk_ids, content_only=args.content_only + ) # Conversion to string and filtering for diff.equal to save memory if sorting diffs = ((path, diff.changes()) for path, diff in diffs if not diff.equal) @@ -105,6 +108,9 @@ def build_parser_diff(self, subparsers, common_parser, mid_common_parser): ) subparser.add_argument("--sort", dest="sort", action="store_true", help="Sort the output lines by file path.") subparser.add_argument("--json-lines", action="store_true", help="Format output as JSON Lines. ") + subparser.add_argument( + "--content-only", action="store_true", help="Only compare differences in content (Exclude time differences)" + ) subparser.add_argument("name", metavar="ARCHIVE1", type=archivename_validator, help="ARCHIVE1 name") subparser.add_argument("other_name", metavar="ARCHIVE2", type=archivename_validator, help="ARCHIVE2 name") subparser.add_argument( diff --git a/src/borg/helpers/parseformat.py b/src/borg/helpers/parseformat.py index ca16f25096..412669ce38 100644 --- a/src/borg/helpers/parseformat.py +++ b/src/borg/helpers/parseformat.py @@ -876,7 +876,7 @@ def __init__(self, archive, format, *, json_lines=False): self.used_call_keys = set(self.call_keys) & self.format_keys def format_item_json(self, item): - return json.dumps(self.get_item_data(item), cls=BorgJsonEncoder) + "\n" + return json.dumps(self.get_item_data(item), cls=BorgJsonEncoder, sort_keys=True) + "\n" def get_item_data(self, item): item_data = {} diff --git a/src/borg/item.pyx b/src/borg/item.pyx index 757362a6a6..670cc332fc 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -8,7 +8,7 @@ from .constants import ITEM_KEYS, ARCHIVE_KEYS from .helpers import StableDict from .helpers import format_file_size from .helpers.msgpack import timestamp_to_int, int_to_timestamp, Timestamp - +from .helpers.time import OutputTimestamp, safe_timestamp cdef extern from "_item.c": object _object_to_optr(object obj) @@ -626,7 +626,7 @@ class ItemDiff: It does not include extended or time attributes in the comparison. """ - def __init__(self, item1, item2, chunk_iterator1, chunk_iterator2, numeric_ids=False, can_compare_chunk_ids=False): + def __init__(self, item1, item2, chunk_iterator1, chunk_iterator2, numeric_ids=False, can_compare_chunk_ids=False, content_only=False): self._item1 = item1 self._item2 = item2 self._numeric_ids = numeric_ids @@ -656,6 +656,9 @@ class ItemDiff: changes.append(self._owner_diff()) changes.append(self._mode_diff()) + if not content_only: + changes.extend(self._time_diffs()) + # filter out empty changes self._changes = [ch for ch in changes if ch] @@ -672,7 +675,7 @@ class ItemDiff: if self._item1.get('deleted') and self._item2.get('deleted'): return True - attr_list = ['deleted', 'mode', 'target'] + attr_list = ['deleted', 'mode', 'target', 'ctime', 'mtime'] attr_list += ['uid', 'gid'] if self._numeric_ids else ['user', 'group'] for attr in attr_list: if self._item1.get(attr) != self._item2.get(attr): @@ -736,6 +739,16 @@ class ItemDiff: mode2 = stat.filemode(self._item2.mode) return ({"type": "mode", "old_mode": mode1, "new_mode": mode2}, '[{} -> {}]'.format(mode1, mode2)) + def _time_diffs(self): + changes = [] + attrs = ["ctime", "mtime"] + for attr in attrs: + if attr in self._item1 and attr in self._item2 and self._item1.get(attr) != self._item2.get(attr): + timestamp1 = OutputTimestamp(safe_timestamp(self._item1.get(attr))) + timestamp2 = OutputTimestamp(safe_timestamp(self._item2.get(attr))) + changes.append(({"type": attr, f"old_{attr}": timestamp1, f"new_{attr}": timestamp2}, '[{}: {} -> {}]'.format(attr, timestamp1, timestamp2))) + return changes + def _content_equal(self, chunk_iterator1, chunk_iterator2): if self._can_compare_chunk_ids: return self._item1.chunks == self._item2.chunks diff --git a/src/borg/testsuite/archiver/__init__.py b/src/borg/testsuite/archiver/__init__.py index b10cee76f7..97f9354737 100644 --- a/src/borg/testsuite/archiver/__init__.py +++ b/src/borg/testsuite/archiver/__init__.py @@ -195,6 +195,11 @@ def create_regular_file(self, name, size=0, contents=None): filename = os.path.join(self.input_path, name) if not os.path.exists(os.path.dirname(filename)): os.makedirs(os.path.dirname(filename)) + self.write_to_file(filename, size=size, append_path=False, contents=contents) + + def write_to_file(self, filename, size=0, append_path=True, contents=None): + if append_path: + filename = os.path.join(self.input_path, filename) with open(filename, "wb") as fd: if contents is None: contents = b"X" * size diff --git a/src/borg/testsuite/archiver/diff_cmd.py b/src/borg/testsuite/archiver/diff_cmd.py index 91c35a351d..dccefb393e 100644 --- a/src/borg/testsuite/archiver/diff_cmd.py +++ b/src/borg/testsuite/archiver/diff_cmd.py @@ -7,6 +7,7 @@ from .. import are_symlinks_supported, are_hardlinks_supported from ..platform import is_win32 from . import ArchiverTestCaseBase, RemoteArchiverTestCaseBase, ArchiverTestCaseBinaryBase, RK_ENCRYPTION, BORG_EXES +from time import sleep class ArchiverTestCase(ArchiverTestCaseBase): @@ -218,10 +219,34 @@ def get_changes(filename, data): if are_hardlinks_supported(): assert not any(get_changes("input/hardlink_target_replaced", joutput)) - do_asserts(self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a"), True) + output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a", "--content-only") + do_asserts(output, True) # We expect exit_code=1 due to the chunker params warning - do_asserts(self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1b", exit_code=1), False) - do_json_asserts(self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a", "--json-lines"), True) + output = self.cmd( + f"--repo={self.repository_location}", "diff", "test0", "test1b", "--content-only", exit_code=1 + ) + do_asserts(output, False) + output = self.cmd( + f"--repo={self.repository_location}", "diff", "test0", "test1a", "--json-lines", "--content-only" + ) + do_json_asserts(output, True) + + def test_time_diffs(self): + self.create_regular_file("test_file", size=10) + self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION) + self.cmd(f"--repo={self.repository_location}", "create", "archive1", "input") + sleep(1) + os.unlink("input/test_file") + self.create_regular_file("test_file", size=10) + self.cmd(f"--repo={self.repository_location}", "create", "archive2", "input", exit_code=0) + output = self.cmd(f"--repo={self.repository_location}", "diff", "archive1", "archive2") + self.assert_in("mtime", output) + self.assert_in("ctime", output) + os.chmod("input/test_file", 777) + self.cmd(f"--repo={self.repository_location}", "create", "archive3", "input", exit_code=0) + output = self.cmd(f"--repo={self.repository_location}", "diff", "archive2", "archive3") + self.assert_not_in("mtime", output) + self.assert_in("ctime", output) def test_sort_option(self): self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION) @@ -242,7 +267,7 @@ def test_sort_option(self): self.create_regular_file("d_file_added", size=256) self.cmd(f"--repo={self.repository_location}", "create", "test1", "input") - output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1", "--sort") + output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1", "--sort", "--content-only") expected = [ "a_file_removed", "b_file_added", From 86884ee04d0e19b8f857920e2d98272edec98aff Mon Sep 17 00:00:00 2001 From: Michael Deyaso Date: Thu, 23 Feb 2023 15:15:34 +0300 Subject: [PATCH 2/4] Seperated diff datapoints using content-only flag. Fixes #7248 --- - Moved mode and owner diff datapoints to be displayed under content-only flag - Added assert_line_exists helper function to testsuite - Modified diff test cases to conform to new output format - Added platform check to test case to accomodate for ctime diff --- src/borg/archiver/diff_cmd.py | 4 +- src/borg/item.pyx | 24 +++--- src/borg/testsuite/__init__.py | 4 + src/borg/testsuite/archiver/__init__.py | 5 -- src/borg/testsuite/archiver/diff_cmd.py | 103 ++++++++++++++---------- 5 files changed, 80 insertions(+), 60 deletions(-) diff --git a/src/borg/archiver/diff_cmd.py b/src/borg/archiver/diff_cmd.py index 40b2f5f489..583f425bcd 100644 --- a/src/borg/archiver/diff_cmd.py +++ b/src/borg/archiver/diff_cmd.py @@ -109,7 +109,9 @@ def build_parser_diff(self, subparsers, common_parser, mid_common_parser): subparser.add_argument("--sort", dest="sort", action="store_true", help="Sort the output lines by file path.") subparser.add_argument("--json-lines", action="store_true", help="Format output as JSON Lines. ") subparser.add_argument( - "--content-only", action="store_true", help="Only compare differences in content (Exclude time differences)" + "--content-only", + action="store_true", + help="Only compare differences in content (exclude metadata differences)", ) subparser.add_argument("name", metavar="ARCHIVE1", type=archivename_validator, help="ARCHIVE1 name") subparser.add_argument("other_name", metavar="ARCHIVE2", type=archivename_validator, help="ARCHIVE2 name") diff --git a/src/borg/item.pyx b/src/borg/item.pyx index 670cc332fc..679807698e 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -629,6 +629,7 @@ class ItemDiff: def __init__(self, item1, item2, chunk_iterator1, chunk_iterator2, numeric_ids=False, can_compare_chunk_ids=False, content_only=False): self._item1 = item1 self._item2 = item2 + self._content_only = content_only self._numeric_ids = numeric_ids self._can_compare_chunk_ids = can_compare_chunk_ids self.equal = self._equal(chunk_iterator1, chunk_iterator2) @@ -652,11 +653,10 @@ class ItemDiff: if self._item1.is_fifo() or self._item2.is_fifo(): changes.append(self._presence_diff('fifo')) - if not (self._item1.get('deleted') or self._item2.get('deleted')): - changes.append(self._owner_diff()) - changes.append(self._mode_diff()) - - if not content_only: + if not self._content_only: + if not (self._item1.get('deleted') or self._item2.get('deleted')): + changes.append(self._owner_diff()) + changes.append(self._mode_diff()) changes.extend(self._time_diffs()) # filter out empty changes @@ -675,8 +675,12 @@ class ItemDiff: if self._item1.get('deleted') and self._item2.get('deleted'): return True - attr_list = ['deleted', 'mode', 'target', 'ctime', 'mtime'] - attr_list += ['uid', 'gid'] if self._numeric_ids else ['user', 'group'] + attr_list = ['deleted', 'target'] + + if not self._content_only: + attr_list += ['mode', 'ctime', 'mtime'] + attr_list += ['uid', 'gid'] if self._numeric_ids else ['user', 'group'] + for attr in attr_list: if self._item1.get(attr) != self._item2.get(attr): return False @@ -744,9 +748,9 @@ class ItemDiff: attrs = ["ctime", "mtime"] for attr in attrs: if attr in self._item1 and attr in self._item2 and self._item1.get(attr) != self._item2.get(attr): - timestamp1 = OutputTimestamp(safe_timestamp(self._item1.get(attr))) - timestamp2 = OutputTimestamp(safe_timestamp(self._item2.get(attr))) - changes.append(({"type": attr, f"old_{attr}": timestamp1, f"new_{attr}": timestamp2}, '[{}: {} -> {}]'.format(attr, timestamp1, timestamp2))) + ts1 = OutputTimestamp(safe_timestamp(self._item1.get(attr))) + ts2 = OutputTimestamp(safe_timestamp(self._item2.get(attr))) + changes.append(({"type": attr, f"old_{attr}": ts1, f"new_{attr}": ts2}, '[{}: {} -> {}]'.format(attr, ts1, ts2))) return changes def _content_equal(self, chunk_iterator1, chunk_iterator2): diff --git a/src/borg/testsuite/__init__.py b/src/borg/testsuite/__init__.py index 3d01fd619d..f060560c96 100644 --- a/src/borg/testsuite/__init__.py +++ b/src/borg/testsuite/__init__.py @@ -14,6 +14,7 @@ import tempfile import time import unittest +import re from ..xattr import get_all from ..platform import get_flags @@ -187,6 +188,9 @@ def assert_dirs_equal(self, dir1, dir2, **kwargs): diff = filecmp.dircmp(dir1, dir2) self._assert_dirs_equal_cmp(diff, **kwargs) + def assert_line_exists(self, lines, expected_regexpr): + assert any(re.search(expected_regexpr, line) for line in lines), f"no match for {expected_regexpr} in {lines}" + def _assert_dirs_equal_cmp(self, diff, ignore_flags=False, ignore_xattrs=False, ignore_ns=False): self.assert_equal(diff.left_only, []) self.assert_equal(diff.right_only, []) diff --git a/src/borg/testsuite/archiver/__init__.py b/src/borg/testsuite/archiver/__init__.py index 97f9354737..b10cee76f7 100644 --- a/src/borg/testsuite/archiver/__init__.py +++ b/src/borg/testsuite/archiver/__init__.py @@ -195,11 +195,6 @@ def create_regular_file(self, name, size=0, contents=None): filename = os.path.join(self.input_path, name) if not os.path.exists(os.path.dirname(filename)): os.makedirs(os.path.dirname(filename)) - self.write_to_file(filename, size=size, append_path=False, contents=contents) - - def write_to_file(self, filename, size=0, append_path=True, contents=None): - if append_path: - filename = os.path.join(self.input_path, filename) with open(filename, "wb") as fd: if contents is None: contents = b"X" * size diff --git a/src/borg/testsuite/archiver/diff_cmd.py b/src/borg/testsuite/archiver/diff_cmd.py index dccefb393e..0f9a68f6b6 100644 --- a/src/borg/testsuite/archiver/diff_cmd.py +++ b/src/borg/testsuite/archiver/diff_cmd.py @@ -71,18 +71,19 @@ def test_basic_functionality(self): self.cmd(f"--repo={self.repository_location}", "create", "test1a", "input") self.cmd(f"--repo={self.repository_location}", "create", "test1b", "input", "--chunker-params", "16,18,17,4095") - def do_asserts(output, can_compare_ids): + def do_asserts(output, can_compare_ids, content_only=False): # File contents changed (deleted and replaced with a new file) change = "B" if can_compare_ids else "{:<19}".format("modified") + lines = output.splitlines() assert "file_replaced" in output # added to debug #3494 - assert f"{change} input/file_replaced" in output + assert "input/file_replaced" in output # File unchanged assert "input/file_unchanged" not in output # Directory replaced with a regular file - if "BORG_TESTS_IGNORE_MODES" not in os.environ and not is_win32: - assert "[drwxr-xr-x -> -rwxr-xr-x] input/dir_replaced_with_file" in output + if "BORG_TESTS_IGNORE_MODES" not in os.environ and not is_win32 and not content_only: + self.assert_line_exists(lines, "drwxr-xr-x -> -rwxr-xr-x.*input/dir_replaced_with_file") # Basic directory cases assert "added directory input/dir_added" in output @@ -90,13 +91,13 @@ def do_asserts(output, can_compare_ids): if are_symlinks_supported(): # Basic symlink cases - assert "changed link input/link_changed" in output - assert "added link input/link_added" in output - assert "removed link input/link_removed" in output + self.assert_line_exists(lines, "changed link.*input/link_changed") + self.assert_line_exists(lines, "added link.*input/link_added") + self.assert_line_exists(lines, "removed link.*input/link_removed") # Symlink replacing or being replaced - assert "] input/dir_replaced_with_link" in output - assert "] input/link_replaced_by_file" in output + assert "input/dir_replaced_with_link" in output + assert "input/link_replaced_by_file" in output # Symlink target removed. Should not affect the symlink at all. assert "input/link_target_removed" not in output @@ -105,9 +106,9 @@ def do_asserts(output, can_compare_ids): # should notice the changes in both links. However, the symlink # pointing to the file is not changed. change = "0 B" if can_compare_ids else "{:<19}".format("modified") - assert f"{change} input/empty" in output + self.assert_line_exists(lines, "%s.*input/empty" % change) if are_hardlinks_supported(): - assert f"{change} input/hardlink_contents_changed" in output + self.assert_line_exists(lines, "%s.*input/hardlink_contents_changed" % change) if are_symlinks_supported(): assert "input/link_target_contents_changed" not in output @@ -128,16 +129,16 @@ def do_asserts(output, can_compare_ids): # Another link (marked previously as the source in borg) to the # same inode was removed. This should not change this link at all. - if are_hardlinks_supported(): + if are_hardlinks_supported() and content_only: assert "input/hardlink_target_removed" not in output # Another link (marked previously as the source in borg) to the # same inode was replaced with a new regular file. This should not # change this link at all. - if are_hardlinks_supported(): + if are_hardlinks_supported() and content_only: assert "input/hardlink_target_replaced" not in output - def do_json_asserts(output, can_compare_ids): + def do_json_asserts(output, can_compare_ids, content_only=False): def get_changes(filename, data): chgsets = [j["changes"] for j in data if j["path"] == filename] assert len(chgsets) < 2 @@ -155,7 +156,7 @@ def get_changes(filename, data): assert not any(get_changes("input/file_unchanged", joutput)) # Directory replaced with a regular file - if "BORG_TESTS_IGNORE_MODES" not in os.environ and not is_win32: + if "BORG_TESTS_IGNORE_MODES" not in os.environ and not is_win32 and not content_only: assert {"type": "mode", "old_mode": "drwxr-xr-x", "new_mode": "-rwxr-xr-x"} in get_changes( "input/dir_replaced_with_file", joutput ) @@ -171,14 +172,17 @@ def get_changes(filename, data): assert {"type": "removed link"} in get_changes("input/link_removed", joutput) # Symlink replacing or being replaced - assert any( - chg["type"] == "mode" and chg["new_mode"].startswith("l") - for chg in get_changes("input/dir_replaced_with_link", joutput) - ) - assert any( - chg["type"] == "mode" and chg["old_mode"].startswith("l") - for chg in get_changes("input/link_replaced_by_file", joutput) - ) + + # TODO: Add mode test flag to assert function + if not content_only: + assert any( + chg["type"] == "mode" and chg["new_mode"].startswith("l") + for chg in get_changes("input/dir_replaced_with_link", joutput) + ), get_changes("input/dir_replaced_with_link", joutput) + assert any( + chg["type"] == "mode" and chg["old_mode"].startswith("l") + for chg in get_changes("input/link_replaced_by_file", joutput) + ) # Symlink target removed. Should not affect the symlink at all. assert not any(get_changes("input/link_target_removed", joutput)) @@ -209,44 +213,56 @@ def get_changes(filename, data): assert {"type": "removed", "size": 256} in get_changes("input/hardlink_removed", joutput) # Another link (marked previously as the source in borg) to the - # same inode was removed. This should not change this link at all. - if are_hardlinks_supported(): - assert not any(get_changes("input/hardlink_target_removed", joutput)) - - # Another link (marked previously as the source in borg) to the - # same inode was replaced with a new regular file. This should not - # change this link at all. - if are_hardlinks_supported(): - assert not any(get_changes("input/hardlink_target_replaced", joutput)) - - output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a", "--content-only") + # same inode was removed. This should not change this link except for its ctime mtime. + if content_only: + if are_hardlinks_supported(): + assert not any(get_changes("input/hardlink_target_removed", joutput)) + + # Another link (marked previously as the source in borg) to the + # same inode was replaced with a new regular file. This should not + # change this link at all. + if are_hardlinks_supported(): + assert not any(get_changes("input/hardlink_target_replaced", joutput)) + + output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a") do_asserts(output, True) # We expect exit_code=1 due to the chunker params warning output = self.cmd( f"--repo={self.repository_location}", "diff", "test0", "test1b", "--content-only", exit_code=1 ) - do_asserts(output, False) + do_asserts(output, False, content_only=True) + + output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a", "--json-lines") + do_json_asserts(output, True) + output = self.cmd( f"--repo={self.repository_location}", "diff", "test0", "test1a", "--json-lines", "--content-only" ) - do_json_asserts(output, True) + do_json_asserts(output, True, content_only=True) def test_time_diffs(self): - self.create_regular_file("test_file", size=10) self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION) + self.create_regular_file("test_file", size=10) self.cmd(f"--repo={self.repository_location}", "create", "archive1", "input") - sleep(1) + sleep(0.1) os.unlink("input/test_file") - self.create_regular_file("test_file", size=10) - self.cmd(f"--repo={self.repository_location}", "create", "archive2", "input", exit_code=0) + if is_win32: + # Sleeping for 15s because Windows doesn't refresh ctime if file is deleted and recreated within 15 seconds. + sleep(15) + self.create_regular_file("test_file", size=15) + self.cmd(f"--repo={self.repository_location}", "create", "archive2", "input") output = self.cmd(f"--repo={self.repository_location}", "diff", "archive1", "archive2") self.assert_in("mtime", output) - self.assert_in("ctime", output) + self.assert_in("ctime", output) # Should show up on windows as well since it is a new file. os.chmod("input/test_file", 777) - self.cmd(f"--repo={self.repository_location}", "create", "archive3", "input", exit_code=0) + self.cmd(f"--repo={self.repository_location}", "create", "archive3", "input") output = self.cmd(f"--repo={self.repository_location}", "diff", "archive2", "archive3") self.assert_not_in("mtime", output) - self.assert_in("ctime", output) + # Checking platform because ctime should not be shown on windows since it wasn't recreated. + if not is_win32: + self.assert_in("ctime", output) + else: + self.assert_not_in("ctime", output) def test_sort_option(self): self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION) @@ -276,7 +292,6 @@ def test_sort_option(self): "e_file_changed", "f_file_removed", ] - assert all(x in line for x, line in zip(expected, output.splitlines())) From e9f6ea2bba65627a082f343e43756cf114430fb5 Mon Sep 17 00:00:00 2001 From: Michael Deyaso Date: Mon, 6 Mar 2023 21:27:02 +0300 Subject: [PATCH 3/4] Simplified regex template and added test-case description. Fixes #7248 --- src/borg/item.pyx | 1 + src/borg/testsuite/__init__.py | 2 +- src/borg/testsuite/archiver/diff_cmd.py | 36 ++++++++++++------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/borg/item.pyx b/src/borg/item.pyx index 679807698e..ee385cc686 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -10,6 +10,7 @@ from .helpers import format_file_size from .helpers.msgpack import timestamp_to_int, int_to_timestamp, Timestamp from .helpers.time import OutputTimestamp, safe_timestamp + cdef extern from "_item.c": object _object_to_optr(object obj) object _optr_to_object(object bytes) diff --git a/src/borg/testsuite/__init__.py b/src/borg/testsuite/__init__.py index f060560c96..d8df3e111c 100644 --- a/src/borg/testsuite/__init__.py +++ b/src/borg/testsuite/__init__.py @@ -8,13 +8,13 @@ except ImportError: posix = None +import re import stat import sys import sysconfig import tempfile import time import unittest -import re from ..xattr import get_all from ..platform import get_flags diff --git a/src/borg/testsuite/archiver/diff_cmd.py b/src/borg/testsuite/archiver/diff_cmd.py index 0f9a68f6b6..694fb9fc9d 100644 --- a/src/borg/testsuite/archiver/diff_cmd.py +++ b/src/borg/testsuite/archiver/diff_cmd.py @@ -1,13 +1,13 @@ import json import os import stat +import time import unittest from ...constants import * # NOQA from .. import are_symlinks_supported, are_hardlinks_supported from ..platform import is_win32 from . import ArchiverTestCaseBase, RemoteArchiverTestCaseBase, ArchiverTestCaseBinaryBase, RK_ENCRYPTION, BORG_EXES -from time import sleep class ArchiverTestCase(ArchiverTestCaseBase): @@ -76,7 +76,7 @@ def do_asserts(output, can_compare_ids, content_only=False): change = "B" if can_compare_ids else "{:<19}".format("modified") lines = output.splitlines() assert "file_replaced" in output # added to debug #3494 - assert "input/file_replaced" in output + self.assert_line_exists(lines, f"{change}.*input/file_replaced") # File unchanged assert "input/file_unchanged" not in output @@ -106,9 +106,9 @@ def do_asserts(output, can_compare_ids, content_only=False): # should notice the changes in both links. However, the symlink # pointing to the file is not changed. change = "0 B" if can_compare_ids else "{:<19}".format("modified") - self.assert_line_exists(lines, "%s.*input/empty" % change) + self.assert_line_exists(lines, f"{change}.*input/empty") if are_hardlinks_supported(): - self.assert_line_exists(lines, "%s.*input/hardlink_contents_changed" % change) + self.assert_line_exists(lines, f"{change}.*input/hardlink_contents_changed") if are_symlinks_supported(): assert "input/link_target_contents_changed" not in output @@ -127,15 +127,15 @@ def do_asserts(output, can_compare_ids, content_only=False): if are_hardlinks_supported(): assert "removed 256 B input/hardlink_removed" in output - # Another link (marked previously as the source in borg) to the - # same inode was removed. This should not change this link at all. if are_hardlinks_supported() and content_only: + # Another link (marked previously as the source in borg) to the + # same inode was removed. This should only change the ctime since removing + # the link would result in the decrementation of the inode's hard-link count. assert "input/hardlink_target_removed" not in output - # Another link (marked previously as the source in borg) to the - # same inode was replaced with a new regular file. This should not - # change this link at all. - if are_hardlinks_supported() and content_only: + # Another link (marked previously as the source in borg) to the + # same inode was replaced with a new regular file. This should only change + # its ctime. This should not be reflected in the output if content-only is set assert "input/hardlink_target_replaced" not in output def do_json_asserts(output, can_compare_ids, content_only=False): @@ -212,16 +212,16 @@ def get_changes(filename, data): if are_hardlinks_supported(): assert {"type": "removed", "size": 256} in get_changes("input/hardlink_removed", joutput) - # Another link (marked previously as the source in borg) to the - # same inode was removed. This should not change this link except for its ctime mtime. if content_only: if are_hardlinks_supported(): + # Another link (marked previously as the source in borg) to the + # same inode was removed. This should only change the ctime since removing + # the link would result in the decrementation of the inode's hard-link count. assert not any(get_changes("input/hardlink_target_removed", joutput)) - # Another link (marked previously as the source in borg) to the - # same inode was replaced with a new regular file. This should not - # change this link at all. - if are_hardlinks_supported(): + # Another link (marked previously as the source in borg) to the + # same inode was replaced with a new regular file. This should only change + # its ctime. This should not be reflected in the output if content-only is set assert not any(get_changes("input/hardlink_target_replaced", joutput)) output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a") @@ -244,11 +244,11 @@ def test_time_diffs(self): self.cmd(f"--repo={self.repository_location}", "rcreate", RK_ENCRYPTION) self.create_regular_file("test_file", size=10) self.cmd(f"--repo={self.repository_location}", "create", "archive1", "input") - sleep(0.1) + time.sleep(0.1) os.unlink("input/test_file") if is_win32: # Sleeping for 15s because Windows doesn't refresh ctime if file is deleted and recreated within 15 seconds. - sleep(15) + time.sleep(15) self.create_regular_file("test_file", size=15) self.cmd(f"--repo={self.repository_location}", "create", "archive2", "input") output = self.cmd(f"--repo={self.repository_location}", "diff", "archive1", "archive2") From 2b97111025e31116614954248387bd08723d976b Mon Sep 17 00:00:00 2001 From: Michael Deyaso Date: Mon, 6 Mar 2023 23:10:50 +0300 Subject: [PATCH 4/4] Time diff now only executed if items are not deleted. Fixes #7248 --- src/borg/item.pyx | 2 +- src/borg/testsuite/archiver/diff_cmd.py | 24 +++++++++++------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/borg/item.pyx b/src/borg/item.pyx index ee385cc686..3b5b104237 100644 --- a/src/borg/item.pyx +++ b/src/borg/item.pyx @@ -658,7 +658,7 @@ class ItemDiff: if not (self._item1.get('deleted') or self._item2.get('deleted')): changes.append(self._owner_diff()) changes.append(self._mode_diff()) - changes.extend(self._time_diffs()) + changes.extend(self._time_diffs()) # filter out empty changes self._changes = [ch for ch in changes if ch] diff --git a/src/borg/testsuite/archiver/diff_cmd.py b/src/borg/testsuite/archiver/diff_cmd.py index 694fb9fc9d..8689bb019d 100644 --- a/src/borg/testsuite/archiver/diff_cmd.py +++ b/src/borg/testsuite/archiver/diff_cmd.py @@ -173,7 +173,6 @@ def get_changes(filename, data): # Symlink replacing or being replaced - # TODO: Add mode test flag to assert function if not content_only: assert any( chg["type"] == "mode" and chg["new_mode"].startswith("l") @@ -182,7 +181,7 @@ def get_changes(filename, data): assert any( chg["type"] == "mode" and chg["old_mode"].startswith("l") for chg in get_changes("input/link_replaced_by_file", joutput) - ) + ), get_changes("input/link_replaced_by_file", joutput) # Symlink target removed. Should not affect the symlink at all. assert not any(get_changes("input/link_target_removed", joutput)) @@ -212,17 +211,16 @@ def get_changes(filename, data): if are_hardlinks_supported(): assert {"type": "removed", "size": 256} in get_changes("input/hardlink_removed", joutput) - if content_only: - if are_hardlinks_supported(): - # Another link (marked previously as the source in borg) to the - # same inode was removed. This should only change the ctime since removing - # the link would result in the decrementation of the inode's hard-link count. - assert not any(get_changes("input/hardlink_target_removed", joutput)) - - # Another link (marked previously as the source in borg) to the - # same inode was replaced with a new regular file. This should only change - # its ctime. This should not be reflected in the output if content-only is set - assert not any(get_changes("input/hardlink_target_replaced", joutput)) + if are_hardlinks_supported() and content_only: + # Another link (marked previously as the source in borg) to the + # same inode was removed. This should only change the ctime since removing + # the link would result in the decrementation of the inode's hard-link count. + assert not any(get_changes("input/hardlink_target_removed", joutput)) + + # Another link (marked previously as the source in borg) to the + # same inode was replaced with a new regular file. This should only change + # its ctime. This should not be reflected in the output if content-only is set + assert not any(get_changes("input/hardlink_target_replaced", joutput)) output = self.cmd(f"--repo={self.repository_location}", "diff", "test0", "test1a") do_asserts(output, True)