[Git][reproducible-builds/diffoscope][master] 3 commits: Don't use the old-style "super" call.

Chris Lamb gitlab at salsa.debian.org
Sat Nov 28 11:34:37 UTC 2020



Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
d774de4b by Chris Lamb at 2020-11-27T10:40:30+00:00
Don't use the old-style "super" call.

- - - - -
0d851ae3 by Chris Lamb at 2020-11-27T10:40:30+00:00
Move the slightly confusing behaviour of loading an "existing diff" if a single file is passed to diffoscope to the new --load-existing-diff command.

- - - - -
06e3eab1 by Conrad Ratschan at 2020-11-28T11:33:28+00:00
Add comparator for "legacy" uboot uImage files. (MR: reproducible-builds/diffoscope!69)

Signed-off-by: Chris Lamb <lamby at debian.org>

- - - - -


7 changed files:

- diffoscope/comparators/__init__.py
- + diffoscope/comparators/uimage.py
- diffoscope/main.py
- + tests/comparators/test_uimage.py
- + tests/data/uimage_expected_diff
- tests/test_presenters.py
- tests/test_readers.py


Changes:

=====================================
diffoscope/comparators/__init__.py
=====================================
@@ -108,6 +108,7 @@ class ComparatorManager:
         ("kbx.KbxFile",),
         ("dtb.DeviceTreeFile",),
         ("ogg.OggFile",),
+        ("uimage.UimageFile",),
         ("xsb.XsbFile",),
         ("berkeley_db.BerkeleyDBFile",),
         ("zst.ZstFile",),


=====================================
diffoscope/comparators/uimage.py
=====================================
@@ -0,0 +1,67 @@
+# -*- coding: utf-8 -*-
+#
+# diffoscope: in-depth comparison of files, archives, and directories
+#
+# Copyright © 2020 Conrad Ratschan <ratschance at gmail.com>
+#
+# diffoscope is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# diffoscope is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with diffoscope.  If not, see <https://www.gnu.org/licenses/>.
+
+import logging
+import os.path
+import re
+import subprocess
+
+from .utils.file import File
+from .utils.archive import Archive
+
+logger = logging.getLogger(__name__)
+
+
+class UimageContainer(Archive):
+    def open_archive(self):
+        return self
+
+    def close_archive(self):
+        pass
+
+    def get_member_names(self):
+        return [self.get_compressed_content_name(".uboot")]
+
+    def extract(self, member_name, dest_dir):
+        dest_path = os.path.join(dest_dir, member_name)
+        logger.debug("uImage extracting to %s", dest_path)
+        with open(dest_path, "wb") as fp:
+            # Use tail to cut off the first 64 bytes without having to do a byte by byte copy in python
+            subprocess.check_call(
+                ["tail", "-c+65", self.source.path],
+                shell=False,
+                stdout=fp,
+                stderr=None,
+            )
+        return dest_path
+
+
+class UimageFile(File):
+    """
+    U-Boot legacy image files are standard OS boot files (kernel, rootfs) with a 64
+    byte header appended to the front of the file by mkimage to provide the load
+    address, entrypoint and a CRC for the contained image. By treating this as a
+    container and setting our extract method to just cut the first 64 bytes off, the
+    rest of diffoscope's tooling can be used to view the differences of the contained
+    files.
+    """
+
+    DESCRIPTION = "U-Boot legacy image files"
+    CONTAINER_CLASSES = [UimageContainer]
+    FILE_TYPE_RE = re.compile(r"^u-boot legacy uImage\b")


=====================================
diffoscope/main.py
=====================================
@@ -22,7 +22,6 @@
 
 import os
 import sys
-import json
 import errno
 import signal
 import logging
@@ -75,9 +74,7 @@ class BooleanAction(argparse.Action):
     def __init__(self, option_strings, dest, nargs=None, **kwargs):
         if nargs is not None:
             raise ValueError("nargs not allowed for BooleanAction")
-        super(BooleanAction, self).__init__(
-            option_strings, dest, nargs=0, **kwargs
-        )
+        super().__init__(option_strings, dest, nargs=0, **kwargs)
 
     def __call__(self, parser, namespace, values, option_string=None):
         setattr(namespace, self.dest, not option_string.startswith("--no"))
@@ -92,17 +89,9 @@ def create_parser():
     )
     parser.add_argument(
         "path1",
-        help='First file or directory to compare. Specify "-" to read a '
-        "diffoscope diff from stdin.",
-    )
-    parser.add_argument(
-        "path2",
-        nargs="?",
-        help="Second file or directory to "
-        "compare. If omitted, no comparison is done but instead we read a "
-        "diffoscope diff from path1 and will output this in the formats "
-        "specified by the rest of the command line.",
+        help="First file or directory to compare.",
     )
+    parser.add_argument("path2", help="Second file or directory to compare.")
     parser.add_argument(
         "--debug",
         action="store_true",
@@ -220,6 +209,13 @@ def create_parser():
         default=None,
         help="Write profiling info to given file (use - for stdout)",
     )
+    parser.add_argument(
+        "--load-existing-diff",
+        metavar="INPUT_FILE",
+        action=LoadExistingDiffAction,
+        dest="load_existing_diff",
+        help='Load existing diff from file. Specify "-" to read a diffoscope diff from stdin.',
+    )
 
     group2 = parser.add_argument_group("output limits")
     # everything marked with default=None below is affected by no-default-limits
@@ -446,7 +442,7 @@ def create_parser():
         sys.exit(1)
 
     def post_parse(parsed_args):
-        if parsed_args.path2 is None:
+        if parsed_args.load_existing_diff is not None:
             # warn about unusual flags in this mode
             ineffective_flags = [
                 f
@@ -509,6 +505,19 @@ class RangeCompleter:
         return (str(i) for i in self.choices if str(i).startswith(prefix))
 
 
+class LoadExistingDiffAction(argparse._StoreAction):
+    def __call__(self, parser, *args, **kwargs):
+        actions_by_dest = {x.dest: x for x in parser._actions}
+
+        # If we have passed a value for --load_existing_diff we don't require
+        # path1 or path2 anymore.
+        actions_by_dest["path1"].required = False
+        actions_by_dest["path2"].required = False
+
+        # Actually store the value.
+        super().__call__(parser, *args, **kwargs)
+
+
 class ListToolsAction(argparse.Action):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -680,23 +689,21 @@ def run_diffoscope(parsed_args):
     configure(parsed_args)
     set_path()
     normalize_environment()
+
     path1, path2 = parsed_args.path1, parsed_args.path2
-    if path2 is None:
-        if path1 == "-":
+
+    # Should we be loading an existing diff from a file
+    if parsed_args.load_existing_diff:
+        x = parsed_args.load_existing_diff
+
+        if x == "-":
             logger.debug("Loading diff from stdin")
             difference = load_diff(sys.stdin, "stdin")
         else:
-            logger.debug("Loading diff from %s", path1)
-            try:
-                difference = load_diff_from_path(path1)
-            except json.JSONDecodeError:
-                traceback.print_exc()
-                print(
-                    "E: Could not parse diff from '{}'. (Are you sure you "
-                    "only meant to specify a single file?)".format(path1),
-                    file=sys.stderr,
-                )
-                return 1
+            logger.debug(
+                "Loading diff from %s", parsed_args.load_existing_diff
+            )
+            difference = load_diff_from_path(x)
     else:
         if Config().exclude_directory_metadata in ("auto", None):
             # Default to ignoring metadata directory...


=====================================
tests/comparators/test_uimage.py
=====================================
@@ -0,0 +1,129 @@
+# -*- coding: utf-8 -*-
+#
+# diffoscope: in-depth comparison of files, archives, and directories
+#
+# Copyright © 2020 Conrad Ratschan <ratschance at gmail.com>
+#
+# diffoscope is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# diffoscope is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with diffoscope.  If not, see <https://www.gnu.org/licenses/>.
+
+import pytest
+
+from diffoscope.comparators.binary import FilesystemFile
+from diffoscope.comparators.uimage import UimageFile
+from diffoscope.comparators.utils.specialize import specialize
+
+from ..utils.data import load_fixture, get_data
+from ..utils.tools import skip_unless_tools_exist
+from ..utils.nonexisting import assert_non_existing
+
+cpio1 = load_fixture("test1.cpio")
+cpio2 = load_fixture("test2.cpio")
+
+# Bytes from a U-Boot legacy image header generated via mkimage
+uboot_header1 = bytes(
+    b"\x27\x05\x19\x56\xf8\x7a\xd2\x00"
+    b"\x5f\xc1\x58\x2c\x00\x00\x04\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x34\x71\x61\xa5\x05\x07\x03\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+)
+
+
+# Bytes from a U-Boot legacy image header generated via mkimage
+uboot_header2 = bytes(
+    b"\x27\x05\x19\x56\xe8\x66\x86\xf7"
+    b"\x5f\xc1\x58\x44\x00\x00\x04\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\xc6\x3c\x4a\x06\x05\x07\x03\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+    b"\x00\x00\x00\x00\x00\x00\x00\x00"
+)
+
+
+def uimage_fixture(header, prefix):
+    @pytest.fixture
+    def uboot_cpio(tmp_path, cpio1, cpio2):
+        if prefix == "test1":
+            cpio = cpio1
+        else:
+            cpio = cpio2
+        cpio_uboot = tmp_path / "{}.cpio.uboot".format(prefix)
+        with open(cpio_uboot, "wb") as t_fp:
+            t_fp.write(header)
+            with open(cpio.path, "rb") as r_fp:
+                t_fp.write(bytes(r_fp.read()))
+        return specialize(FilesystemFile(str(cpio_uboot)))
+
+    return uboot_cpio
+
+
+uboot_cpio1 = uimage_fixture(uboot_header1, "test1")
+uboot_cpio2 = uimage_fixture(uboot_header2, "test2")
+
+
+def test_identification(uboot_cpio1, uboot_cpio2):
+    assert isinstance(uboot_cpio1, UimageFile)
+    assert isinstance(uboot_cpio2, UimageFile)
+
+
+def test_no_differences(uboot_cpio1):
+    difference = uboot_cpio1.compare(uboot_cpio1)
+    assert difference is None
+
+
+ at pytest.fixture
+def differences(uboot_cpio1, uboot_cpio2):
+    return uboot_cpio1.compare(uboot_cpio2).details
+
+
+ at pytest.fixture
+def nested_differences(uboot_cpio1, uboot_cpio2):
+    return uboot_cpio1.compare(uboot_cpio2).details[1].details
+
+
+def test_file_differences(differences):
+    expected_diff = get_data("uimage_expected_diff")
+    assert differences[0].unified_diff == expected_diff
+
+
+ at skip_unless_tools_exist("cpio")
+def test_nested_listing(nested_differences):
+    expected_diff = get_data("cpio_listing_expected_diff")
+    assert nested_differences[0].unified_diff == expected_diff
+
+
+ at skip_unless_tools_exist("cpio")
+def test_nested_symlink(nested_differences):
+    assert nested_differences[1].source1 == "dir/link"
+    assert nested_differences[1].comment == "symlink"
+    expected_diff = get_data("symlink_expected_diff")
+    assert nested_differences[1].unified_diff == expected_diff
+
+
+ at skip_unless_tools_exist("cpio")
+def test_nested_compressed_files(nested_differences):
+    assert nested_differences[2].source1 == "dir/text"
+    assert nested_differences[2].source2 == "dir/text"
+    expected_diff = get_data("text_ascii_expected_diff")
+    assert nested_differences[2].unified_diff == expected_diff
+
+
+ at skip_unless_tools_exist("cpio")
+def test_compare_non_existing(monkeypatch, uboot_cpio1):
+    assert_non_existing(monkeypatch, uboot_cpio1)


=====================================
tests/data/uimage_expected_diff
=====================================
@@ -0,0 +1,3 @@
+@@ -1 +1 @@
+-u-boot legacy uImage, , Linux/PowerPC, RAMDisk Image (Not compressed), 1024 bytes, Fri Nov 27 19:49:00 2020, Load Address: 0x00000000, Entry Point: 0x00000000, Header CRC: 0xF87AD200, Data CRC: 0x347161A5
++u-boot legacy uImage, , Linux/PowerPC, RAMDisk Image (Not compressed), 1024 bytes, Fri Nov 27 19:49:24 2020, Load Address: 0x00000000, Entry Point: 0x00000000, Header CRC: 0xE86686F7, Data CRC: 0xC63C4A06


=====================================
tests/test_presenters.py
=====================================
@@ -191,7 +191,7 @@ def test_html_regression_875281(tmpdir, capsys):
         "--html",
         report_path,
         "--max-page-size=5000",
-        pair=(diff_path,),
+        f"--load-existing-diff={diff_path}",
     )
     assert out == ""
 


=====================================
tests/test_readers.py
=====================================
@@ -29,7 +29,7 @@ from .utils.data import cwd_data, get_data
 
 def run_read_write(capsys, diff, *args):
     with pytest.raises(SystemExit) as exc, cwd_data():
-        main(args + (diff,))
+        main(args + (f"--load-existing-diff={diff}",))
 
     out, err = capsys.readouterr()
 



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/716efb195cd2fdd89833b321790f65869c8c820a...06e3eab1618987bdb17a041abb9f416192cb59e8

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/716efb195cd2fdd89833b321790f65869c8c820a...06e3eab1618987bdb17a041abb9f416192cb59e8
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/20201128/60b9ded7/attachment.htm>


More information about the rb-commits mailing list