Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress modified changes for files which weren't actually modified in JSON output. #8460

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 42 additions & 17 deletions src/borg/archiver/diff_cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,46 @@ class DiffMixIn:
@with_repository(compatibility=(Manifest.Operation.READ,))
def do_diff(self, args, repository, manifest):
"""Diff contents of two archives"""

def actual_change(j):
j = j.to_dict()
if j["type"] == "modified":
# Added/removed keys will not exist if chunker params differ
# between the two archives. Err on the side of caution and assume
# a real modification in this case (short-circuiting retrieving
# non-existent keys).
return not {"added", "removed"} <= j.keys() or not (j["added"] == 0 and j["removed"] == 0)
else:
# All other change types are indeed changes.
return True

def print_json_output(diff):
print(
json.dumps(
{
"path": diff.path,
"changes": [
change.to_dict()
for name, change in diff.changes().items()
if actual_change(change) and (not args.content_only or (name not in DiffFormatter.METADATA))
],
},
sort_keys=True,
cls=BorgJsonEncoder,
)
)

def print_text_output(diff, formatter):
actual_changes = dict(
(name, change)
for name, change in diff.changes().items()
if actual_change(change) and (not args.content_only or (name not in DiffFormatter.METADATA))
)
diff._changes = actual_changes
res: str = formatter.format_item(diff)
if res.strip():
sys.stdout.write(res)

if args.format is not None:
format = args.format
elif args.content_only:
Expand Down Expand Up @@ -56,24 +96,9 @@ def do_diff(self, args, repository, manifest):
formatter = DiffFormatter(format, args.content_only)
for diff in diffs:
if args.json_lines:
print(
json.dumps(
{
"path": diff.path,
"changes": [
change.to_dict()
for name, change in diff.changes().items()
if not args.content_only or (name not in DiffFormatter.METADATA)
],
},
sort_keys=True,
cls=BorgJsonEncoder,
)
)
print_json_output(diff)
else:
res: str = formatter.format_item(diff)
if res.strip():
sys.stdout.write(res)
print_text_output(diff, formatter)

for pattern in matcher.get_unmatched_include_patterns():
self.print_warning_instance(IncludePatternNeverMatchedWarning(pattern))
Expand Down
6 changes: 6 additions & 0 deletions src/borg/testsuite/archiver/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,12 @@ def assert_line_exists(lines, expected_regexpr):
assert any(re.search(expected_regexpr, line) for line in lines), f"no match for {expected_regexpr} in {lines}"


def assert_line_not_exists(lines, expected_regexpr):
assert not any(
re.search(expected_regexpr, line) for line in lines
), f"unexpected match for {expected_regexpr} in {lines}"


def _assert_dirs_equal_cmp(diff, ignore_flags=False, ignore_xattrs=False, ignore_ns=False):
assert diff.left_only == []
assert diff.right_only == []
Expand Down
32 changes: 31 additions & 1 deletion src/borg/testsuite/archiver/diff_cmd_test.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
import json
import os
from pathlib import Path
import stat
import time

from ...constants import * # NOQA
from .. import are_symlinks_supported, are_hardlinks_supported
from ...platformflags import is_win32, is_darwin
from . import cmd, create_regular_file, RK_ENCRYPTION, assert_line_exists, generate_archiver_tests
from . import (
cmd,
create_regular_file,
RK_ENCRYPTION,
assert_line_exists,
generate_archiver_tests,
assert_line_not_exists,
)

pytest_generate_tests = lambda metafunc: generate_archiver_tests(metafunc, kinds="local,remote,binary") # NOQA

Expand All @@ -19,6 +27,7 @@ def test_basic_functionality(archivers, request):
create_regular_file(archiver.input_path, "file_removed", size=256)
create_regular_file(archiver.input_path, "file_removed2", size=512)
create_regular_file(archiver.input_path, "file_replaced", size=1024)
create_regular_file(archiver.input_path, "file_touched", size=128)
os.mkdir("input/dir_replaced_with_file")
os.chmod("input/dir_replaced_with_file", stat.S_IFDIR | 0o755)
os.mkdir("input/dir_removed")
Expand All @@ -44,6 +53,7 @@ def test_basic_functionality(archivers, request):
create_regular_file(archiver.input_path, "file_replaced", contents=b"0" * 4096)
os.unlink("input/file_removed")
os.unlink("input/file_removed2")
Path("input/file_touched").touch()
os.rmdir("input/dir_replaced_with_file")
create_regular_file(archiver.input_path, "dir_replaced_with_file", size=8192)
os.chmod("input/dir_replaced_with_file", stat.S_IFREG | 0o755)
Expand Down Expand Up @@ -102,6 +112,15 @@ def do_asserts(output, can_compare_ids, content_only=False):
# pointing to the file is not changed.
change = "modified.*0 B" if can_compare_ids else r"modified: \(can't get size\)"
assert_line_exists(lines, f"{change}.*input/empty")

# Do not show a 0 byte change for a file whose contents weren't modified.
assert_line_not_exists(lines, "0 B.*input/file_touched")
if not content_only:
assert_line_exists(lines, "[cm]time:.*input/file_touched")
else:
# And if we're doing content-only, don't show the file at all.
assert "input/file_touched" not in output

if are_hardlinks_supported():
assert_line_exists(lines, f"{change}.*input/hardlink_contents_changed")
if are_symlinks_supported():
Expand Down Expand Up @@ -150,6 +169,17 @@ def get_changes(filename, data):
# File unchanged
assert not any(get_changes("input/file_unchanged", joutput))

# Do not show a 0 byte change for a file whose contents weren't modified.
unexpected = {"type": "modified", "added": 0, "removed": 0}
assert unexpected not in get_changes("input/file_touched", joutput)
if not content_only:
# on win32, ctime is the file creation time and does not change.
expected = {"mtime"} if is_win32 else {"mtime", "ctime"}
assert expected.issubset({c["type"] for c in get_changes("input/file_touched", joutput)})
else:
# And if we're doing content-only, don't show the file at all.
assert not any(get_changes("input/file_touched", joutput))

# Directory replaced with a regular file
if "BORG_TESTS_IGNORE_MODES" not in os.environ and not is_win32 and not content_only:
assert {"type": "changed mode", "item1": "drwxr-xr-x", "item2": "-rwxr-xr-x"} in get_changes(
Expand Down
Loading