debian/changelog

-diffoscope (147) UNRELEASED; urgency=medium
+diffoscope (147) unstable; urgency=medium
 
-  * WIP (generated upon release).
+  * New features:
+
+    - Add output from strings(1) to ELF binaries. It is intended this will
+      expose expose build paths that are hidden somewhere within the objdump(1)
+      output. (Closes: reproducible-builds/diffoscope#148)
+    - Add basic zsh shell tab-completion support.
+      (Closes: reproducible-builds/diffoscope#158)
+
+  * Bug fixes:
+
+    - Prevent a traceback when comparing a PDF document that does not contain
+      any metadata, ie. it is missing a PDF "/Info" stanza.
+      (Closes: reproducible-builds/diffoscope#150)
+    - Fix compatibility with jsondiff 1.2.0 which was causing a traceback and
+      log the version of jsondiff we are using to aid debugging in the future.
+      (Closes: reproducible-builds/diffoscope#159
+    - Fix an issue in GnuPG keybox handling that left filenames in the diff.
+    - Don't mask an existing test name; ie. ensure it is actually run.
+
+  * Reporting:
+
+    - Log all calls to subprocess.check_output by using our own wrapper utility.
+      (Closes: reproducible-builds/diffoscope#151)
+
+  * Code improvements:
 
- -- Chris Lamb <lamby@debian.org>  Sat, 30 May 2020 12:44:42 +0100
+    - Replace references to "WF" with "Wagner-Fischer" for clarity.
+    - Drop a large number of unused imports (list_libarchive,
+      ContainerExtractionError, etc.)
+    - Don't assign exception to a variable that we do not use.
+    - Compare string values with the equality operator, not via "is" identity.
+    - Don't alias an open file to a variable when we don't use it.
+    - Don't alias "filter" builtin.
+    - Refactor many small parts of the HTML generation, dropping explicit
+      u"unicode" strings, tidying the generation of the "Offset X, Y lines
+      modified" messages, moving to PEP 498 f-strings where appropriate, etc.
+    - Inline a number of single-used utility methods.
+
+ -- Chris Lamb <lamby@debian.org>  Thu, 11 Jun 2020 12:50:34 +0100
 
 diffoscope (146) unstable; urgency=medium
 

debian/diffoscope.install

 bin/diffoscope usr/bin
+debian/zsh-completion/_diffoscope usr/share/zsh/vendor-completions

debian/zsh-completion/_diffoscope

+#compdef diffoscope
+
+typeset -A opt_args
+
+_diffoscope() {
+
+  _arguments \
+    '--debug[Display debug messages]' \
+    '--pdb[Open the Python pdb debugger in case of crashes]' \
+    '--status-fd=[Send machine-readable status to file descriptor FD]:' \
+    '(--progress --no-progress)'{--progress,--no-progress}'=[Show an approximate progress bar. Default: yes if stdin is a tty, otherwise no.]:' \
+    '--no-default-limits[Disable most default output limits and diff calculation limits.]' \
+    '--text=[Write plain text output to given file (use - for stdout)]:' \
+    '--text-color=[When to output color diff. Default: auto, meaning yes if the output is a terminal, otherwise no.]:--text-color :(never auto always)' \
+    '--output-empty[If there was no difference, then output an empty diff for each output type that was specified. In --text output, an empty file is written.]' \
+    '--html=[Write HTML report to given file (use - for stdout)]:' \
+    '--html-dir=[Write multi-file HTML report to given directory]:' \
+    '--css=[Link to an extra CSS for the HTML report]:' \
+    '--jquery=[URL link to jQuery, for --html and --html-dir output. If this is a non-existent relative URL, diffoscope will create a symlink to a system installation. (Paths searched: /usr/share/javascript/jquery/jquery.js.) If not given, --html output will not use JS but --html-dir will if it can be found; give "disable" to disable JS on all outputs.]:' \
+    '--json=[Write JSON text output to given file (use - for stdout)]:' \
+    '--markdown=[Write Markdown text output to given file (use - for stdout)]:' \
+    '--restructured-text=[Write RsT text output to given file (use - for stdout)]:' \
+    '--difftool=[Compare differences one-by-one using the specified external command similar to git-difftool(1)]:' \
+    '--profile=[Write profiling info to given file (use - for stdout)]:' \
+    '--max-text-report-size=[Maximum bytes written in --text report. (0 to disable, default: 0)]:' \
+    '--max-report-size=[Maximum bytes of a report in a given format, across all of its pages. Note that some formats, such as --html, may be restricted by even smaller limits such as --max-page-size. (0 to disable, default: 41943040)]:' \
+    '--max-diff-block-lines=[Maximum number of lines output per unified-diff block, across all pages. (0 to disable, default: 1024)]:' \
+    '--max-page-size=[Maximum bytes of the top-level (--html-dir) or sole (--html) page. (default: %(default)s, remains in effect even with --no-default-limits)]:' \
+    '--max-page-size-child=[In --html-dir output, this is the maximum bytes of each child page (default: %(default)s, remains in effect even with --no-default-limits)]:' \
+    '--max-page-diff-block-lines=[Maximum number of lines output per unified-diff block on the top-level (--html-dir) or sole (--html) page, before spilling it into child pages (--html-dir) or skipping the rest of the diff block. Child pages are limited instead by --max-page-size-child. (default: %(default)s, remains in effect even with --no-default-limits)]:' \
+    '--new-file[Treat absent files as empty]' \
+    '--exclude=[Exclude files whose names (including any directory part) match %(metavar)s. Use this option to ignore files based on their names.]:' \
+    '--exclude-command=[Exclude commands that match %(metavar)s. For example "^readelf.*\s--debug-dump=info" can take a long time and differences here are likely secondary differences caused by something represented elsewhere. Use this option to disable commands that use a lot of resources.]:' \
+    '--exclude-directory-metadata=[Exclude directory metadata. Useful if comparing files whose filesystem-level metadata is not intended to be distributed to other systems. This is true for most distributions package builders, but not true for the output of commands such as `make install`. Metadata of archive members remain un-excluded except if "recursive" choice is set. Use this option to ignore permissions, timestamps, xattrs etc. Default: False if comparing two directories, else True. Note that "file" metadata actually a property of its containing directory, and is not relevant when distributing the file across systems.]:--exclude-directory-metadata :(auto yes no recursive)' \
+    '--fuzzy-threshold=[Threshold for fuzzy-matching (0 to disable, %(default)s is default, 400 is high fuzziness)]:' \
+    '--tool-prefix-binutils=[Prefix for binutils program names, e.g. "aarch64-linux-gnu-" for a foreign-arch binary or "g" if you"re on a non-GNU system.]:' \
+    '--max-diff-input-lines=[Maximum number of lines fed to diff(1) (0 to disable, default: 4194304)]:' \
+    '--max-container-depth=[Maximum depth to recurse into containers. (Cannot be disabled for security reasons, default: %(default)s)]:' \
+    '--max-diff-block-lines-saved=[Maximum number of lines saved per diff block. Most users should not need this, unless you run out of memory. This truncates diff(1) output before emitting it in a report, and affects all types of output, including --text and --json. (0 to disable, default: %(default)s)]:' \
+    '--use-dbgsym=[When to automatically use corresponding -dbgsym packages when comparing .deb files. WHEN is one of "never", "auto", "always". Default: auto, meaning yes if two .changes or .buildinfo files are specified, otherwise no.]:--use-dbgsym :(no auto yes)' \
+    '--force-details[Force recursing into the depths of file formats even if files have the same content, only really useful for debugging diffoscope. Default: %(default)s]' \
+    '(--help -h)'{--help,-h}'[Show this help and exit]' \
+    '--version[Show program"s version number and exit]' \
+    '--list-tools=[Show external tools required and exit. If specified, the output will list packages in that distribution that satisfy these dependencies.]:--list-tools :(arch debian FreeBSD guix)' \
+    '--list-debian-substvars[List packages needed for Debian in "substvar" format.]' \
+    '--list-missing-tools=[Show missing external tools and exit. If specified, the output will list packages in that distribution that satisfy these dependencies.]:--list-missing-tools :(arch debian FreeBSD guix)' \
+    '*:: :->args'
+    _files
+}
+_diffoscope

diffoscope/__init__.py

@@ -18,4 +18,4 @@
 # You should have received a copy of the GNU General Public License
 # along with diffoscope.  If not, see <https://www.gnu.org/licenses/>.
 
-VERSION = "146"
+VERSION = "147"

diffoscope/changes.py

@@ -196,6 +196,8 @@ class Changes:
             if item.endswith('.dsc'):
                 return item
 
+        return None
+
     def get_pool_path(self):
         """
         Returns the path the changes file would be

diffoscope/comparators/elf.py

@@ -263,13 +263,6 @@ READELF_COMMANDS = (
 )
 
 
-def _compare_elf_data(path1, path2):
-    return [
-        Difference.from_command(x, path1, path2, ignore_returncodes={1})
-        for x in list(READELF_COMMANDS) + READELF_DEBUG_DUMP_COMMANDS
-    ]
-
-
 def _should_skip_section(name, type):
     for x in READELF_COMMANDS:
         if x.should_skip_section(name, type):
@@ -616,10 +609,27 @@ class ElfContainer(Container):
         return self._sections[member_name]
 
 
+class Strings(Command):
+    @tool_required("strings")
+    def cmdline(self):
+        return ("strings", "--all", self.path)
+
+
 class ElfFile(File):
     DESCRIPTION = "ELF binaries"
     CONTAINER_CLASSES = [ElfContainer]
     FILE_TYPE_RE = re.compile(r"^ELF ")
 
     def compare_details(self, other, source=None):
-        return _compare_elf_data(self.path, other.path)
+        differences = [
+            Difference.from_command(
+                x, self.path, other.path, ignore_returncodes={1}
+            )
+            for x in list(READELF_COMMANDS) + READELF_DEBUG_DUMP_COMMANDS
+        ]
+
+        differences.append(
+            Difference.from_command(Strings, self.path, other.path)
+        )
+
+        return differences

diffoscope/comparators/json.py

@@ -19,6 +19,7 @@
 
 import re
 import json
+import logging
 import collections
 
 from diffoscope.difference import Difference
@@ -26,6 +27,8 @@ from diffoscope.tools import python_module_missing
 
 from .utils.file import File
 
+logger = logging.getLogger(__name__)
+
 try:
     import jsondiff
 except ImportError:  # noqa
@@ -74,11 +77,13 @@ class JSONFile(File):
         if jsondiff is None:
             return
 
+        logger.debug(f"Comparing using jsondiff {jsondiff.__version__}")
+
         a = getattr(self, "parsed", {})
         b = getattr(other, "parsed", {})
 
         try:
-            diff = {repr(x): y for x, y in jsondiff.diff(a, b).items()}
+            diff = {repr(x): repr(y) for x, y in jsondiff.diff(a, b).items()}
         except Exception:
             return
 

diffoscope/comparators/kbx.py

@@ -32,7 +32,6 @@ class Kbxutil(Command):
         return ("kbxutil", self.path)
 
     def filter(self, line):
-        return line
         if line.decode("utf-8").strip() == self.path:
             return b""
         return line

diffoscope/diff.py

@@ -427,7 +427,8 @@ def linediff(s, t, diffon, diffoff):
 
     s1 = "".join(to_string(*p) for p in l1)
     t1 = "".join(to_string(*p) for p in l2)
-    return prefix + s1 + suffix, prefix + t1 + suffix
+
+    return f"{prefix}{s1}{suffix}", f"{prefix}{t1}{suffix}"
 
 
 def linediff_wagnerfischer(s, t):

diffoscope/external_tools.py

@@ -185,6 +185,7 @@ EXTERNAL_TOOLS = {
         "guix": "openssh",
     },
     "stat": {"debian": "coreutils", "arch": "coreutils", "guix": "coreutils"},
+    "strings": {"debian": "binutils-multiarch"},
     "sqlite3": {
         "debian": "sqlite3",
         "arch": "sqlite",

diffoscope/presenters/html/html.py

@@ -156,7 +156,7 @@ def convert(s, ponct=0, tag=""):
         elif c == "\n" and ponct == 1:
             t.write('<br/><span class="diffponct">\\</span>')
         elif ord(c) < 32:
-            conv = u"\\x%x" % ord(c)
+            conv = "\\x%x" % ord(c)
             t.write("<em>%s</em>" % conv)
             i += len(conv)
         else:
@@ -178,7 +178,7 @@ def output_visual(ctx, visual, path, indentstr, indentnum):
     indent = tuple(indentstr * (indentnum + x) for x in range(3))
     anchor = output_anchor(ctx, path)
     id = 'id="{}"'.format(anchor) if anchor else ""
-    return u"""{0[0]}<div class="difference">
+    return """{0[0]}<div class="difference">
 {0[1]}<div class="diffheader">
 {0[1]}<div class="diffcontrol">⊟</div>
 {0[1]}<div {5}><span class="source">{1}</span>
@@ -201,12 +201,12 @@ def output_node_frame(ctx, difference, path, indentstr, indentnum, body):
     anchor = output_anchor(ctx, path)
     id = 'id="{}"'.format(anchor) if anchor else ""
     dctrl_class, dctrl = (
-        ("diffcontrol", u"⊟")
+        ("diffcontrol", "⊟")
         if difference.has_visible_children()
-        else ("diffcontrol-nochildren", u"⊡")
+        else ("diffcontrol-nochildren", "⊡")
     )
     if difference.source1 == difference.source2:
-        header = u"""{0[1]}<div class="{1}">{2}</div>
+        header = """{0[1]}<div class="{1}">{2}</div>
 {0[1]}<div><span class="diffsize">{3}</span></div>
 {0[1]}<div {6}><span class="source">{5}</span>
 {0[2]}<a class="anchor" href="#{4}">\xb6</a>
@@ -221,7 +221,7 @@ def output_node_frame(ctx, difference, path, indentstr, indentnum, body):
             id,
         )
     else:
-        header = u"""{0[1]}<div class="{1} diffcontrol-double">{2}</div>
+        header = """{0[1]}<div class="{1} diffcontrol-double">{2}</div>
 {0[1]}<div><span class="diffsize">{3}</span></div>
 {0[1]}<div><span class="source">{5}</span> vs.</div>
 {0[1]}<div {7}><span class="source">{6}</span>
@@ -239,7 +239,7 @@ def output_node_frame(ctx, difference, path, indentstr, indentnum, body):
         )
 
     return PartialString.numl(
-        u"""{0[1]}<div class="diffheader">
+        """{0[1]}<div class="diffheader">
 {1}{0[1]}</div>
 {2}""",
         3,
@@ -259,22 +259,22 @@ def output_node(ctx, difference, path, indentstr, indentnum):
     indent = tuple(indentstr * (indentnum + x) for x in range(3))
     t, cont = PartialString.cont()
 
-    comments = u""
+    comments = ""
     if difference.comments:
-        comments = u'{1[1]}<div class="comment {0}">{2}{1[1]}</div>\n'.format(
+        comments = '{1[1]}<div class="comment {0}">{2}{1[1]}</div>\n'.format(
             "multiline" if len(difference.comments) > 1 else "",
             indent,
             "\n".join(
-                u"{1}".format(indent, html.escape(x))
+                "{1}".format(indent, html.escape(x))
                 for x in difference.comments
             ),
         )
 
-    visuals = u""
+    visuals = ""
     for visual in difference.visuals:
         visuals += output_visual(ctx, visual, path, indentstr, indentnum + 1)
 
-    udiff = u""
+    udiff = ""
     ud_cont = None
     if difference.unified_diff:
         ud_cont = HTMLSideBySidePresenter().output_unified_diff(
@@ -290,7 +290,7 @@ def output_node(ctx, difference, path, indentstr, indentnum):
             ud_cont = None
 
     # PartialString for this node
-    body = PartialString.numl(u"{0}{1}{2}{-1}", 3, cont).pformatl(
+    body = PartialString.numl("{0}{1}{2}{-1}", 3, cont).pformatl(
         comments, visuals, udiff
     )
     if len(path) == 1:
@@ -306,7 +306,7 @@ def output_node(ctx, difference, path, indentstr, indentnum):
             ctx, d, path + [d], indentstr, indentnum + 1, PartialString.of(d)
         )
         child = PartialString.numl(
-            u"""{0[1]}<div class="difference">
+            """{0[1]}<div class="difference">
 {1}{0[1]}</div>
 {-1}""",
             2,
@@ -316,20 +316,19 @@ def output_node(ctx, difference, path, indentstr, indentnum):
 
     # there might be extra holes for the unified diff continuation
     assert len(t.holes) >= len(difference.details) + 1
-    return cont(t, u""), ud_cont
+    return cont(t, ""), ud_cont
 
 
 def output_header(css_url, our_css_url=False, icon_url=None):
     if css_url:
         css_link = (
-            u'  <link href="%s" type="text/css" rel="stylesheet" />\n'
-            % css_url
+            '  <link href="%s" type="text/css" rel="stylesheet" />\n' % css_url
         )
     else:
-        css_link = u""
+        css_link = ""
     if our_css_url:
         css_style = (
-            u'  <link href="%s" type="text/css" rel="stylesheet" />\n'
+            '  <link href="%s" type="text/css" rel="stylesheet" />\n'
             % our_css_url
         )
     else:
@@ -337,7 +336,7 @@ def output_header(css_url, our_css_url=False, icon_url=None):
     if icon_url:
         favicon = icon_url
     else:
-        favicon = u"data:image/png;base64," + FAVICON_BASE64
+        favicon = "data:image/png;base64," + FAVICON_BASE64
     return templates.HEADER % {
         "title": html.escape(" ".join(sys.argv)),
         "favicon": favicon,
@@ -415,51 +414,43 @@ class HTMLSideBySidePresenter:
         self.error_row = None
 
     def output_hunk_header(self, hunk_off1, hunk_size1, hunk_off2, hunk_size2):
-        self.spl_print_func(
-            u'<tr class="diffhunk"><td colspan="2">Offset %d, %d lines modified</td>'
-            % (hunk_off1, hunk_size1)
-        )
-        self.spl_print_func(
-            u'<td colspan="2">Offset %d, %d lines modified</td></tr>\n'
-            % (hunk_off2, hunk_size2)
-        )
+        self.spl_print_func('<tr class="diffhunk">')
+
+        for a, b in ((hunk_off1, hunk_size1), (hunk_off2, hunk_size2)):
+            self.spl_print_func(
+                f'<td colspan="2">Offset {a}, {b} lines modified</td>'
+            )
+
+        self.spl_print_func("</tr>\n")
 
     def output_line(
         self, has_internal_linenos, type_name, s1, line1, s2, line2
     ):
-        self.spl_print_func(u'<tr class="diff%s">' % type_name)
+        self.spl_print_func('<tr class="diff%s">' % type_name)
         try:
             if s1:
                 if has_internal_linenos:
-                    self.spl_print_func(
-                        u'<td colspan="2" class="diffpresent">'
-                    )
+                    self.spl_print_func('<td colspan="2" class="diffpresent">')
                 else:
-                    self.spl_print_func(
-                        u'<td class="diffline">%d </td>' % line1
-                    )
-                    self.spl_print_func(u'<td class="diffpresent">')
+                    self.spl_print_func(f'<td class="diffline">{line1} </td>')
+                    self.spl_print_func('<td class="diffpresent">')
                 self.spl_print_func(convert(s1, ponct=1, tag="del"))
-                self.spl_print_func(u"</td>")
+                self.spl_print_func("</td>")
             else:
-                self.spl_print_func(u'<td colspan="2">\xa0</td>')
+                self.spl_print_func('<td colspan="2">\xa0</td>')
 
             if s2:
                 if has_internal_linenos:
-                    self.spl_print_func(
-                        u'<td colspan="2" class="diffpresent">'
-                    )
+                    self.spl_print_func('<td colspan="2" class="diffpresent">')
                 else:
-                    self.spl_print_func(
-                        u'<td class="diffline">%d </td>' % line2
-                    )
-                    self.spl_print_func(u'<td class="diffpresent">')
+                    self.spl_print_func(f'<td class="diffline">{line2} </td>')
+                    self.spl_print_func('<td class="diffpresent">')
                 self.spl_print_func(convert(s2, ponct=1, tag="ins"))
-                self.spl_print_func(u"</td>")
+                self.spl_print_func("</td>")
             else:
-                self.spl_print_func(u'<td colspan="2">\xa0</td>')
+                self.spl_print_func('<td colspan="2">\xa0</td>')
         finally:
-            self.spl_print_func(u"</tr>\n")
+            self.spl_print_func("</tr>\n")
 
     def spl_print_enter(self, print_context, rotation_params):
         # Takes ownership of print_context
@@ -532,7 +523,7 @@ class HTMLSideBySidePresenter:
                 templates.UD_TABLE_FOOTER
                 % {"filename": html.escape(filename), "text": "load diff"}
             )
-            self.spl_print_func(u"</table>\n")
+            self.spl_print_func("</table>\n")
             self.spl_print_exit(None, None, None)
 
         # rotate to the next child page
@@ -567,7 +558,7 @@ class HTMLSideBySidePresenter:
                 elif t == "H":
                     self.output_hunk_header(*args)
                 elif t == "C":
-                    self.spl_print_func(u'<td colspan="2">%s</td>\n' % args)
+                    self.spl_print_func('<td colspan="2">%s</td>\n' % args)
                 else:
                     raise AssertionError()
                 self.spl_rows += 1
@@ -594,7 +585,7 @@ class HTMLSideBySidePresenter:
             wrote_all = False
         finally:
             # no footer on the last page, just a close tag
-            self.spl_print_func(u"</table>")
+            self.spl_print_func("</table>")
         yield wrote_all
 
     def output_unified_diff(self, ctx, unified_diff, has_internal_linenos):
@@ -620,7 +611,7 @@ class HTMLSideBySidePresenter:
                 # size-limit to write the remaining pages with
                 # exhaust the iterator and save the last item in wrote_all
                 new_limit = yield PartialString(
-                    PartialString.escape(udiff.getvalue()) + u"{0}</table>\n",
+                    PartialString.escape(udiff.getvalue()) + "{0}</table>\n",
                     None,
                 )
                 wrote_all = send_and_exhaust(it, new_limit, wrote_all)
@@ -820,8 +811,8 @@ class HTMLPresenter(Presenter):
 
                 outputs[node] = node_output.frame(
                     output_header(ctx.css_url, ctx.our_css_url, ctx.icon_url)
-                    + u'<div class="difference">\n',
-                    u"</div>\n" + footer,
+                    + '<div class="difference">\n',
+                    "</div>\n" + footer,
                 )
                 assert not ctx.single_page or node is root_difference
                 printers[node] = (

tests/comparators/test_elf.py

@@ -227,10 +227,12 @@ def test_differences_with_dbgsym(dbgsym_differences):
     assert dbgsym_differences.details[2].source1 == "data.tar.xz"
     bin_details = dbgsym_differences.details[2].details[0].details[0]
     assert bin_details.source1 == "./usr/bin/test"
-    assert bin_details.details[1].source1.startswith("objdump")
+    assert bin_details.details[1].source1.startswith("strings --all")
+    assert "shstrtab" in bin_details.details[1].unified_diff
+    assert bin_details.details[2].source1.startswith("objdump")
     assert (
         "test-cases/dbgsym/package/test.c:2"
-        in bin_details.details[1].unified_diff
+        in bin_details.details[2].unified_diff
     )
 
 
@@ -239,9 +241,9 @@ def test_differences_with_dbgsym(dbgsym_differences):
 @skip_unless_module_exists("debian.deb822")
 def test_original_gnu_debuglink(dbgsym_differences):
     bin_details = dbgsym_differences.details[2].details[0].details[0]
-    assert ".gnu_debuglink" in bin_details.details[2].source1
+    assert ".gnu_debuglink" in bin_details.details[3].source1
     expected_gnu_debuglink = get_data("gnu_debuglink_expected_diff")
-    assert bin_details.details[2].unified_diff == expected_gnu_debuglink
+    assert bin_details.details[3].unified_diff == expected_gnu_debuglink
 
 
 def test_ignore_readelf_errors1_identify(ignore_readelf_errors1):