[Git][reproducible-builds/diffoscope][master] 3 commits: Avoid a possible traceback caused by a str/bytes confusion when handling the...

[Chris Lamb]

Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
b803d430 by Chris Lamb at 2019-08-27T12:40:06Z
Avoid a possible traceback caused by a str/bytes confusion when handling the output of failing external commands.

- - - - -
4689755d by Chris Lamb at 2019-08-27T13:13:22Z
Include either standard error or standard output (and not just the latter) when an external command fails.

- - - - -
31c2df52 by Chris Lamb at 2019-08-27T13:13:32Z
Skip calls to unsquashfs when we are not root or fakeroot. (Closes: reproducible-builds/diffoscope#63)

This is a little unfortunate but currently easier than recreating the
test1.squashfs and test2.squashfs test input files to remove the character
device.

See also 4689755d393d76b8e4f7536c8c0523c40faebe46 so this general issue is
visible to end-users.

- - - - -


3 changed files:

- diffoscope/comparators/utils/file.py
- tests/comparators/test_squashfs.py
- tests/utils/tools.py


Changes:

=====================================
diffoscope/comparators/utils/file.py
=====================================
@@ -452,14 +452,34 @@ class File(object, metaclass=abc.ABCMeta):
                 cmd = ' '.join(e.cmd)
                 if difference is None:
                     return None
-                output = '<none>'
-                if e.output:
-                    output = '\n{}'.format(
-                        re.sub(r'^', '    ', e.output, flags=re.MULTILINE)
-                    )
+
+                # Include either stderr (prefered) or stdout in the hexdump
+                # difference
+                suffix = None
+                for prefix, val in (
+                    ("Standard output", e.stdout),
+                    ("Standard error", e.stderr),
+                ):
+                    if not val:
+                        continue
+                    suffix = ' {}:\n{}'.format(
+                        prefix,
+                        re.sub(
+                            r'^',
+                            '    ',
+                            val.decode('utf-8'),
+                            flags=re.MULTILINE,
+                        ),
+                    ).strip()
+
+                    # Truncate output
+                    max_len = 250
+                    if len(suffix) > max_len:
+                        suffix = '{}  [...]'.format(suffix[:max_len])
+
                 difference.add_comment(
-                    "Command `{}` exited with return code {}. Output: {}".format(
-                        cmd, e.returncode, output
+                    "Command `{}` exited with return code {}.{}".format(
+                        cmd, e.returncode, suffix or " (No output)"
                     )
                 )
             except RequiredToolNotFound as e:


=====================================
tests/comparators/test_squashfs.py
=====================================
@@ -23,7 +23,11 @@ import subprocess
 from diffoscope.comparators.squashfs import SquashfsFile
 
 from ..utils.data import load_fixture, get_data
-from ..utils.tools import skip_unless_tools_exist, skip_unless_tool_is_at_least
+from ..utils.tools import (
+    skip_unless_tools_exist,
+    skip_unless_tool_is_at_least,
+    skip_unless_root,
+)
 from ..utils.nonexisting import assert_non_existing
 
 
@@ -50,6 +54,7 @@ def test_no_differences(squashfs1):
     assert difference is None
 
 
+ at skip_unless_root
 def test_no_warnings(capfd, squashfs1, squashfs2):
     _ = squashfs1.compare(squashfs2)
     _, err = capfd.readouterr()
@@ -61,12 +66,14 @@ def differences(squashfs1, squashfs2):
     return squashfs1.compare(squashfs2).details
 
 
+ at skip_unless_root
 @skip_unless_tool_is_at_least('unsquashfs', unsquashfs_version, '4.4')
 def test_superblock(differences):
     expected_diff = get_data('squashfs_superblock_expected_diff')
     assert differences[0].unified_diff == expected_diff
 
 
+ at skip_unless_root
 @skip_unless_tools_exist('unsquashfs')
 def test_symlink(differences):
     assert differences[2].comment == 'symlink'
@@ -74,6 +81,7 @@ def test_symlink(differences):
     assert differences[2].unified_diff == expected_diff
 
 
+ at skip_unless_root
 @skip_unless_tools_exist('unsquashfs')
 def test_compressed_files(differences):
     assert differences[3].source1 == '/text'
@@ -82,6 +90,7 @@ def test_compressed_files(differences):
     assert differences[3].unified_diff == expected_diff
 
 
+ at skip_unless_root
 @skip_unless_tools_exist('unsquashfs')
 def test_compare_non_existing(monkeypatch, squashfs1):
     assert_non_existing(monkeypatch, squashfs1)


=====================================
tests/utils/tools.py
=====================================
@@ -201,3 +201,8 @@ def skip_unless_module_exists(name):
 
 def skip_unless_file_version_is_at_least(version):
     return skip_unless_tool_is_at_least('file', file_version, version)
+
+
+skip_unless_root = pytest.mark.skipif(
+    os.geteuid() != 0, reason="requires root/fakeroot"
+)



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/c67a04ee57f9e3f04b3009d3027d7215aa76a6e9...31c2df528be7f4fc18b4254e8b458c109654b75d

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/compare/c67a04ee57f9e3f04b3009d3027d7215aa76a6e9...31c2df528be7f4fc18b4254e8b458c109654b75d
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/20190827/e0e1ed9a/attachment.html>