[diffoscope] 01/01: Mostly restore the behaviour of file comparison and --exclude-directory-metadata

[Ximin Luo]

This is an automated email from the git hooks/post-receive script.

infinity0 pushed a commit to branch master
in repository diffoscope.

commit b4c0be0eb62127af12af2e9bab19ddd8ea3595eb
Author: Ximin Luo <infinity0 at debian.org>
Date:   Fri Jan 26 01:07:45 2018 +0100

    Mostly restore the behaviour of file comparison and --exclude-directory-metadata
    
    i.e. exclude metadata by default, when not comparing two directories.
    
    The reasoning is explained in the additional help text, and is implicitly why
    "compare_meta" was always defined in directory.py and why originally
    --no-exclude-directory-metadata did not apply to file comparison. (It now does,
    to fix #888402).
---
 diffoscope/comparators/utils/compare.py | 24 +++++++++++++++++-------
 diffoscope/main.py                      |  7 +++++--
 tests/test_presenters.py                |  8 ++------
 3 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/diffoscope/comparators/utils/compare.py b/diffoscope/comparators/utils/compare.py
index beaf7ff..e9ffba3 100644
--- a/diffoscope/comparators/utils/compare.py
+++ b/diffoscope/comparators/utils/compare.py
@@ -57,20 +57,30 @@ def compare_root_paths(path1, path2):
         bail_if_non_existing(path1, path2)
     if any_excluded(path1, path2):
         return None
+
+    default_metadata = Config().exclude_directory_metadata is None
+
     if os.path.isdir(path1) and os.path.isdir(path2):
+        if default_metadata:
+            Config().exclude_directory_metadata = False
         return compare_directories(path1, path2)
+
+    if default_metadata:
+        Config().exclude_directory_metadata = True
     container1 = FilesystemDirectory(os.path.dirname(path1)).as_container
     file1 = specialize(FilesystemFile(path1, container=container1))
     container2 = FilesystemDirectory(os.path.dirname(path2)).as_container
     file2 = specialize(FilesystemFile(path2, container=container2))
     difference = compare_files(file1, file2)
-    meta = compare_meta(path1, path2)
-    if meta:
-        # Create an "empty" difference so we have something to attach file
-        # metadata to.
-        if difference is None:
-            difference = Difference(None, file1.name, file2.name)
-        difference.add_details(meta)
+
+    if not Config().exclude_directory_metadata:
+        meta = compare_meta(path1, path2)
+        if meta:
+            # Create an "empty" difference so we have something to attach file
+            # metadata to.
+            if difference is None:
+                difference = Difference(None, file1.name, file2.name)
+            difference.add_details(meta)
     return difference
 
 
diff --git a/diffoscope/main.py b/diffoscope/main.py
index 37650a0..23e227a 100644
--- a/diffoscope/main.py
+++ b/diffoscope/main.py
@@ -185,14 +185,17 @@ def create_parser():
                         'only secondary differences caused by something that '
                         'is already represented elsewhere in the diff.')
     group3.add_argument('--exclude-directory-metadata', '--no-exclude-directory-metadata',
-                        action=BooleanAction, default=False,
+                        action=BooleanAction, default=None,
                         help='Exclude directory metadata. Useful if comparing '
                         'files whose filesystem-level metadata is not intended '
                         'to be distributed to other systems. For example, this '
                         'is true for most distros\' package builders, but not '
                         'true for the output of commands like `make install`. '
                         'Metadata of archive members remain un-excluded. '
-                        'Default: %(default)s')
+                        'Default: False if comparing two directories, else '
+                        'True. Note that "file" metadata is actually a '
+                        'property of its containing directory, and is not '
+                        'relevant when distributing the file across systems.')
     group3.add_argument('--fuzzy-threshold', type=int,
                         help='Threshold for fuzzy-matching '
                         '(0 to disable, %(default)s is default, 400 is high fuzziness)',
diff --git a/tests/test_presenters.py b/tests/test_presenters.py
index b365fae..66561db 100644
--- a/tests/test_presenters.py
+++ b/tests/test_presenters.py
@@ -36,7 +36,7 @@ re_html = re.compile(r'.*<body(?P<body>.*)<div class="footer">', re.MULTILINE |
 
 def run(capsys, *args, pair=('test1.tar', 'test2.tar')):
     with pytest.raises(SystemExit) as exc, cwd_data():
-        main(('--exclude-directory-metadata',) + args + pair)
+        main(args + pair)
     out, err = capsys.readouterr()
 
     assert err == ''
@@ -174,12 +174,8 @@ def test_html_regression_875281(tmpdir, capsys):
     # this test fails when you `git revert -Xtheirs ccd926f`
     diff_path = expand_collapsed_json(tmpdir, 'debian-bug-875281')
     report_path = str(tmpdir.join('report.html'))
-    with pytest.raises(SystemExit) as exc, cwd_data():
-        main(('--html', report_path, diff_path,))
-    out, err = capsys.readouterr()
-    assert exc.value.code == 1
+    out = run(capsys, '--html', report_path, pair=(diff_path,))
     assert out == ''
-    assert err == ''
 
 
 def test_limited_print():

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/diffoscope.git