[diffoscope] 01/01: More long-term fix for the shared-singleton issue

[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 498edb5252b5481c4d1d53bfdca10be2bd4cf75c
Author: Ximin Luo <infinity0 at debian.org>
Date:   Fri Jan 26 02:18:47 2018 +0100

    More long-term fix for the shared-singleton issue
---
 diffoscope/comparators/utils/compare.py |  6 ----
 diffoscope/config.py                    | 55 ++++++++++++++++++---------------
 diffoscope/main.py                      | 10 ++++++
 tests/comparators/test_directory.py     |  4 ---
 4 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/diffoscope/comparators/utils/compare.py b/diffoscope/comparators/utils/compare.py
index e9ffba3..e525066 100644
--- a/diffoscope/comparators/utils/compare.py
+++ b/diffoscope/comparators/utils/compare.py
@@ -58,15 +58,9 @@ def compare_root_paths(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
diff --git a/diffoscope/config.py b/diffoscope/config.py
index db6b3af..ff07695 100644
--- a/diffoscope/config.py
+++ b/diffoscope/config.py
@@ -29,36 +29,41 @@ class defaultint(int):
     pass
 
 
+# Avoid setting values on this anywhere other than main.run_diffoscope(),
+# otherwise tests may fail unpredictably depending on order-of-execution.
 class Config(object):
-    # GNU diff cannot process arbitrary large files :(
-    max_diff_input_lines = 2 ** 22
-    max_diff_block_lines_saved = float("inf")
-
-    # hard limits, restricts single-file and multi-file formats
-    max_report_size = defaultint(40 * 2 ** 20)  # 40 MB
-    max_diff_block_lines = defaultint(2 ** 10)  # 1024 lines
-    # structural limits, restricts single-file formats
-    # semi-restricts multi-file formats
-    max_page_size = defaultint(400 * 2 ** 10)  # 400 kB
-    max_page_size_child = defaultint(200 * 2 ** 10)  # 200 kB
-    max_page_diff_block_lines = defaultint(2 ** 7)  # 128 lines
-
-    max_text_report_size = 0
-
-    new_file = False
-    fuzzy_threshold = 60
-    enforce_constraints = True
-    excludes = ()
-    exclude_commands = ()
-    exclude_directory_metadata = False
-    compute_visual_diffs = False
-    max_container_depth = 50
-    force_details = False
-
     _singleton = {}
 
     def __init__(self):
         self.__dict__ = self._singleton
+        if not self._singleton:
+            self.reset()
+
+    def reset(self):
+        # GNU diff cannot process arbitrary large files :(
+        self.max_diff_input_lines = 2 ** 22
+        self.max_diff_block_lines_saved = float("inf")
+
+        # hard limits, restricts single-file and multi-file formats
+        self.max_report_size = defaultint(40 * 2 ** 20)  # 40 MB
+        self.max_diff_block_lines = defaultint(2 ** 10)  # 1024 lines
+        # structural limits, restricts single-file formats
+        # semi-restricts multi-file formats
+        self.max_page_size = defaultint(400 * 2 ** 10)  # 400 kB
+        self.max_page_size_child = defaultint(200 * 2 ** 10)  # 200 kB
+        self.max_page_diff_block_lines = defaultint(2 ** 7)  # 128 lines
+
+        self.max_text_report_size = 0
+
+        self.new_file = False
+        self.fuzzy_threshold = 60
+        self.enforce_constraints = True
+        self.excludes = ()
+        self.exclude_commands = ()
+        self.exclude_directory_metadata = False
+        self.compute_visual_diffs = False
+        self.max_container_depth = 50
+        self.force_details = False
 
     def __setattr__(self, k, v):
         super(Config, self).__setattr__(k, v)
diff --git a/diffoscope/main.py b/diffoscope/main.py
index 23e227a..b24df03 100644
--- a/diffoscope/main.py
+++ b/diffoscope/main.py
@@ -382,6 +382,12 @@ def run_diffoscope(parsed_args):
         else:
             difference = load_diff_from_path(path1)
     else:
+        if Config().exclude_directory_metadata is None:
+            if os.path.isdir(path1) and os.path.isdir(path2):
+                Config().exclude_directory_metadata = False
+            else:
+                Config().exclude_directory_metadata = True
+
         logger.debug('Starting comparison')
         with Progress():
             with profile('main', 'outputs'):
@@ -426,6 +432,10 @@ def main(args=None):
             pdb.post_mortem()
         sys.exit(2)
     finally:
+        # Helps our tests run more predictably - some of them call main()
+        # which sets Config() values.
+        Config().reset()
+
         with profile('main', 'cleanup'):
             clean_all_temp_files()
 
diff --git a/tests/comparators/test_directory.py b/tests/comparators/test_directory.py
index 880f2e3..869e098 100644
--- a/tests/comparators/test_directory.py
+++ b/tests/comparators/test_directory.py
@@ -58,10 +58,6 @@ def differences(tmpdir):
     os.utime(str(tmpdir.join('b/dir')), (0, 0))
     os.utime(str(tmpdir.join('a')), (0, 0))
     os.utime(str(tmpdir.join('b')), (0, 0))
-    # Earlier tests set this to True, unfortunately.
-    # i.e. the Config() singleton pattern does not play nicely
-    # with our tests, eventually that should be cleaned up.
-    Config().exclude_directory_metadata = False
     return compare_directories(str(tmpdir.join('a')), str(tmpdir.join('b'))).details
 
 

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