diffoscope/comparators/fit.py

@@ -3,6 +3,7 @@
 # diffoscope: in-depth comparison of files, archives, and directories
 #
 # Copyright © 2020-2021 Conrad Ratschan <ratschance@gmail.com>
+# Copyright © 2021 Chris Lamb <lamby@debian.org>
 #
 # diffoscope is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -16,15 +17,17 @@
 #
 # 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
 import re
+import logging
+import subprocess
 
 from diffoscope.tools import tool_required, tool_check_installed
 from diffoscope.difference import Difference
+
 from .utils import command
 from .utils.archive import Archive
-
 from .utils.file import File
 from .utils.command import Command
 
@@ -58,6 +61,7 @@ class FitContainer(Archive):
             # Save mapping of name -> position as dumpimage takes position as an argument
             self._members[member_name] = pos
             members.append(member_name)
+
         return members
 
     @tool_required("dumpimage")
@@ -66,18 +70,23 @@ class FitContainer(Archive):
         dest_path = os.path.join(dest_dir, os.path.basename(member_name))
         logger.debug("fit image extracting %s to %s", member_name, dest_path)
 
-        command.our_check_output(
-            [
-                "dumpimage",
-                "-T",
-                "flat_dt",
-                "-p",
-                pos,
-                self.source.path,
-                "-o",
-                dest_path,
-            ],
+        cmd = (
+            "dumpimage",
+            "-T",
+            "flat_dt",
+            "-p",
+            pos,
+            self.source.path,
+            "-o",
+            dest_path,
         )
+
+        output = command.our_check_output(cmd)
+
+        # Cannot rely on dumpimage returning a non-zero exit code on failure.
+        if not os.path.exists(dest_path):
+            raise subprocess.CalledProcessError(1, cmd)
+
         return dest_path
 
 

tests/comparators/test_apk.py

@@ -2,7 +2,7 @@
 # diffoscope: in-depth comparison of files, archives, and directories
 #
 # Copyright © 2017 Maria Glukhova <siammezzze@gmail.com>
-# Copyright © 2017, 2019-2020 Chris Lamb <lamby@debian.org>
+# Copyright © 2017, 2019-2021 Chris Lamb <lamby@debian.org>
 #
 # diffoscope is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

tests/comparators/test_fit.py

@@ -3,6 +3,7 @@
 # diffoscope: in-depth comparison of files, archives, and directories
 #
 # Copyright © 2020 Conrad Ratschan <ratschance@gmail.com>
+# Copyright © 2021 Chris Lamb <lamby@debian.org>
 #
 # diffoscope is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -26,13 +27,22 @@ from diffoscope.comparators.fit import FlattenedImageTreeFile
 from diffoscope.comparators.utils.specialize import specialize
 
 from ..utils.data import data, assert_diff, load_fixture
-from ..utils.tools import skip_unless_tools_exist
+from ..utils.tools import skip_unless_tools_exist, skip_unless_tool_is_at_least
 from ..utils.nonexisting import assert_non_existing
 
 cpio1 = load_fixture("test1.cpio")
 cpio2 = load_fixture("test2.cpio")
 
 
+def dumpimage_version():
+    return (
+        subprocess.check_output(("dumpimage", "-V"))
+        .decode("utf-8")
+        .strip()
+        .split(" ")[-1]
+    )
+
+
 def fit_fixture(prefix, entrypoint):
     @pytest.fixture
     def uboot_fit(tmp_path):
@@ -98,21 +108,21 @@ def nested_differences(uboot_fit1, uboot_fit2):
     return uboot_fit1.compare(uboot_fit2).details[1].details
 
 
-@skip_unless_tools_exist("dumpimage")
+@skip_unless_tool_is_at_least("dumpimage", dumpimage_version, "2021.01")
 @skip_unless_tools_exist("fdtdump")
 def test_file_differences(differences):
     assert_diff(differences[0], "fit_expected_diff")
 
 
 @skip_unless_tools_exist("cpio")
-@skip_unless_tools_exist("dumpimage")
+@skip_unless_tool_is_at_least("dumpimage", dumpimage_version, "2021.01")
 @skip_unless_tools_exist("fdtdump")
 def test_nested_listing(nested_differences):
     assert_diff(nested_differences[0], "cpio_listing_expected_diff")
 
 
 @skip_unless_tools_exist("cpio")
-@skip_unless_tools_exist("dumpimage")
+@skip_unless_tool_is_at_least("dumpimage", dumpimage_version, "2021.01")
 @skip_unless_tools_exist("fdtdump")
 def test_nested_symlink(nested_differences):
     assert nested_differences[1].source1 == "dir/link"
@@ -121,7 +131,7 @@ def test_nested_symlink(nested_differences):
 
 
 @skip_unless_tools_exist("cpio")
-@skip_unless_tools_exist("dumpimage")
+@skip_unless_tool_is_at_least("dumpimage", dumpimage_version, "2021.01")
 @skip_unless_tools_exist("fdtdump")
 def test_nested_compressed_files(nested_differences):
     assert nested_differences[2].source1 == "dir/text"
@@ -130,7 +140,7 @@ def test_nested_compressed_files(nested_differences):
 
 
 @skip_unless_tools_exist("cpio")
-@skip_unless_tools_exist("dumpimage")
+@skip_unless_tool_is_at_least("dumpimage", dumpimage_version, "2021.01")
 @skip_unless_tools_exist("fdtdump")
 def test_compare_non_existing(monkeypatch, uboot_fit1):
     assert_non_existing(monkeypatch, uboot_fit1)

tests/test_source.py

 #
 # diffoscope: in-depth comparison of files, archives, and directories
 #
-# Copyright © 2018-2020 Chris Lamb <lamby@debian.org>
+# Copyright © 2018-2021 Chris Lamb <lamby@debian.org>
 #
 # diffoscope is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -238,7 +238,7 @@ def black_version():
         out = subprocess.check_output(("black", "--version"))
     except subprocess.CalledProcessError as e:
         out = e.output
-    return out.decode("utf-8").rsplit(" ", 1)[-1]
+    return out.strip().decode("utf-8").rsplit(" ", 1)[-1]
 
 
 @skip_unless_tool_is_at_least("black", black_version, "20.8b1")