Merge pull request #8460 from ThomasWaldmann/json-no-changes-master

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

src/borg/archiver/diff_cmd.py

@@ -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:
@@ -56,24 +96,9 @@ class DiffMixIn:
         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))

src/borg/testsuite/archiver/__init__.py

@@ -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 == []

src/borg/testsuite/archiver/diff_cmd_test.py

@@ -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
 
@@ -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")
@@ -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)
@@ -102,6 +112,15 @@ def test_basic_functionality(archivers, request):
         # 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():
@@ -150,6 +169,17 @@ def test_basic_functionality(archivers, request):
         # 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(