[Git][reproducible-builds/diffoscope][master] 7 commits: Factor out the set of the Config() global out of the run_diffoscope method.

Chris Lamb gitlab at salsa.debian.org
Wed Jan 22 16:42:48 UTC 2020



Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
8eb852ac by Chris Lamb at 2020-01-22T16:42:26+00:00
Factor out the set of the Config() global out of the run_diffoscope method.

(diff is kinda misleading; nothing is actually changed.)

- - - - -
d6585834 by Chris Lamb at 2020-01-22T16:42:29+00:00
Drop the "config" argument to maybe_set_limit.

- - - - -
d2e283c1 by Chris Lamb at 2020-01-22T16:42:29+00:00
Inline the functionality of maybe_set_limit.

- - - - -
d377d3b7 by Chris Lamb at 2020-01-22T16:42:29+00:00
Tidy diffoscope.main's configure method.

- - - - -
fee25e59 by Chris Lamb at 2020-01-22T16:42:29+00:00
Drop --max-report-size-child and --max-diff-block-lines-parent; scheduled for removal in January 2018.

- - - - -
5010f522 by Chris Lamb at 2020-01-22T16:42:29+00:00
Support external build tools. (Closes: reproducible-builds/diffoscope#87, reproducible-builds/diffoscope!41)

- - - - -
8993168a by Chris Lamb at 2020-01-22T16:42:29+00:00
Add a "noqa" line to avoid a false-positive flake8 "unused import" warning.

- - - - -


3 changed files:

- diffoscope/comparators/utils/compare.py
- diffoscope/config.py
- diffoscope/main.py


Changes:

=====================================
diffoscope/comparators/utils/compare.py
=====================================
@@ -20,8 +20,10 @@
 import io
 import os
 import sys
+import shlex
 import logging
 import binascii
+import subprocess
 
 from diffoscope.tools import tool_required
 from diffoscope.exc import RequiredToolNotFound
@@ -111,6 +113,9 @@ def compare_files(file1, file2, source=None, diff_content_only=False):
 
     specialize(file1)
     specialize(file2)
+
+    call_difftool(file1, file2)
+
     if isinstance(file1, MissingFile):
         file1.other_file = file2
     elif isinstance(file2, MissingFile):
@@ -123,6 +128,25 @@ def compare_files(file1, file2, source=None, diff_content_only=False):
         return file1.compare(file2, source)
 
 
+def call_difftool(file1, file2):
+    """
+    Call an external difftool one-by-one, similar to git-difftool(1).
+    """
+
+    if Config().difftool is None:
+        return
+
+    a = '/dev/null' if isinstance(file1, MissingFile) else file1.path
+    b = '/dev/null' if isinstance(file2, MissingFile) else file2.path
+
+    if os.path.isdir(a) or os.path.isdir(b):
+        return
+
+    cmd = " ".join((Config().difftool, shlex.quote(a), shlex.quote(b)))
+    logger.debug("Calling external command %r", cmd)
+    subprocess.call(cmd, shell=True)
+
+
 def bail_if_non_existing(*paths):
     if not all(map(os.path.lexists, paths)):
         for path in paths:


=====================================
diffoscope/config.py
=====================================
@@ -55,6 +55,7 @@ class Config:
 
         self.max_text_report_size = 0
 
+        self.difftool = None
         self.new_file = False
         self.fuzzy_threshold = 60
         self.enforce_constraints = True


=====================================
diffoscope/main.py
=====================================
@@ -202,6 +202,13 @@ def create_parser():
         dest='restructuredtext_output',
         help='Write RsT text output to given file (use - for stdout)',
     )
+    group1.add_argument(
+        '--difftool',
+        metavar='TOOL',
+        dest='difftool',
+        help='Compare differences one-by-one using the specified external '
+        'command similar to git-difftool(1)',
+    )
     group1.add_argument(
         '--profile',
         metavar='OUTPUT_FILE',
@@ -257,14 +264,6 @@ def create_parser():
         'effect even with --no-default-limits)',
         default=str(Config().max_page_size_child),
     ).completer = RangeCompleter(Config().max_page_size_child)
-    # TODO: old flag kept for backwards-compat, drop 6 months after v84
-    group2.add_argument(
-        '--max-report-size-child',
-        metavar='BYTES',
-        type=int,
-        help=argparse.SUPPRESS,
-        default=None,
-    )
     group2.add_argument(
         '--max-page-diff-block-lines',
         metavar='LINES',
@@ -277,14 +276,6 @@ def create_parser():
         'effect even with --no-default-limits)',
         default=Config().max_page_diff_block_lines,
     ).completer = RangeCompleter(Config().max_page_diff_block_lines)
-    # TODO: old flag kept for backwards-compat, drop 6 months after v84
-    group2.add_argument(
-        "--max-diff-block-lines-parent",
-        metavar='LINES',
-        type=int,
-        help=argparse.SUPPRESS,
-        default=None,
-    )
 
     group3 = parser.add_argument_group('diff calculation')
     group3.add_argument(
@@ -594,72 +585,75 @@ class ListDebianSubstvarsAction(argparse._StoreTrueAction):
         sys.exit(0)
 
 
-def maybe_set_limit(config, parsed_args, key):
-    # apply limits affected by "no-default-limits"
-    v = getattr(parsed_args, key)
-    if v is not None:
-        setattr(config, key, float("inf") if v == 0 else v)
-    elif parsed_args.no_default_limits:
-        setattr(config, key, float("inf"))
-
-
-def run_diffoscope(parsed_args):
-    """
-    (This should not be considered a stable API suitable for external
-    consumption, and the lack of configuration of globals may result in
-    unpredictable behaviour.)
-    """
+def configure(parsed_args):
+    for x in (
+        "max_report_size",
+        "max_text_report_size",
+        "max_diff_block_lines",
+        "max_diff_block_lines_saved",
+        "max_diff_input_lines",
+    ):
+        # Apply limits affected by "no-default-limits"
+        v = getattr(parsed_args, x)
+        if v is not None:
+            setattr(Config(), x, float("inf") if v == 0 else v)
 
-    logger.debug("Starting diffoscope %s", VERSION)
+        elif parsed_args.no_default_limits:
+            setattr(Config(), x, float("inf"))
 
-    ProfileManager().setup(parsed_args)
-    PresenterManager().configure(parsed_args)
-    if not tlsh:
-        logger.warning(
-            'Fuzzy-matching is currently disabled as the "tlsh" module is unavailable.'
-        )
-    maybe_set_limit(Config(), parsed_args, "max_report_size")
-    maybe_set_limit(Config(), parsed_args, "max_text_report_size")
-    maybe_set_limit(Config(), parsed_args, "max_diff_block_lines")
     Config().max_page_size = parsed_args.max_page_size
-    # TODO: old flag kept for backwards-compat, drop 6 months after v84
-    if parsed_args.max_report_size_child is not None:
-        logger.warning(
-            "Detected deprecated flag --max-report-size-child; use --max-page-size-child instead."
-        )
-        Config().max_page_size_child = parsed_args.max_report_size_child
     Config().max_page_size_child = parsed_args.max_page_size_child
-    # TODO: old flag kept for backwards-compat, drop 6 months after v84
-    if parsed_args.max_diff_block_lines_parent is not None:
-        logger.warning(
-            "Detected deprecated flag --max-diff-block-lines-parent; use --max-page-diff-block-lines instead."
-        )
-        logger.warning(
-            "Note that the new flag --max-page-diff-block-lines also applies to --html output."
-        )
-        Config().max_page_diff_block_lines = (
-            parsed_args.max_diff_block_lines_parent
-        )
     Config().max_page_diff_block_lines = parsed_args.max_page_diff_block_lines
 
-    maybe_set_limit(Config(), parsed_args, "max_diff_block_lines_saved")
-    maybe_set_limit(Config(), parsed_args, "max_diff_input_lines")
-    Config().max_container_depth = parsed_args.max_container_depth
+    Config().difftool = parsed_args.difftool
+    Config().new_file = parsed_args.new_file
     Config().use_dbgsym = parsed_args.use_dbgsym
     Config().force_details = parsed_args.force_details
     Config().fuzzy_threshold = parsed_args.fuzzy_threshold
-    Config().new_file = parsed_args.new_file
+    Config().max_container_depth = parsed_args.max_container_depth
+
     Config().excludes = parsed_args.excludes
     Config().exclude_commands = parsed_args.exclude_commands
     Config().exclude_directory_metadata = (
         parsed_args.exclude_directory_metadata
     )
+
     Config().compute_visual_diffs = PresenterManager().compute_visual_diffs()
-    Config().check_constraints()
+
     tool_prepend_prefix(
         parsed_args.tool_prefix_binutils,
-        *"ar as ld ld.bfd nm objcopy objdump ranlib readelf strip".split(),
+        'ar',
+        'as',
+        'ld',
+        'ld.bfd',
+        'nm',
+        'objcopy',
+        'objdump',
+        'ranlib',
+        'readelf',
+        'strip',
     )
+
+    Config().check_constraints()
+
+
+def run_diffoscope(parsed_args):
+    """
+    (This should not be considered a stable API suitable for external
+    consumption, and the lack of configuration of globals may result in
+    unpredictable behaviour.)
+    """
+
+    logger.debug("Starting diffoscope %s", VERSION)
+
+    ProfileManager().setup(parsed_args)
+    PresenterManager().configure(parsed_args)
+    if not tlsh:
+        logger.warning(
+            'Fuzzy-matching is currently disabled as the "tlsh" module is unavailable.'
+        )
+
+    configure(parsed_args)
     set_path()
     normalize_environment()
     path1, path2 = parsed_args.path1, parsed_args.path2
@@ -712,7 +706,7 @@ def main(args=None):
     signal.signal(signal.SIGTERM, sigterm_handler)
 
     try:
-        import libarchive
+        import libarchive  # noqa
     except (ImportError, AttributeError):
         traceback.print_exc()
         python_module_missing('libarchive')



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/21cb69c242cb7098f86c94a969992ec842a4ab7c...8993168aa68511d1f27ec6d9caf6e1427c34aeac

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/21cb69c242cb7098f86c94a969992ec842a4ab7c...8993168aa68511d1f27ec6d9caf6e1427c34aeac
You're receiving this email because of your account on salsa.debian.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.reproducible-builds.org/pipermail/rb-commits/attachments/20200122/5b9763fa/attachment.htm>


More information about the rb-commits mailing list