[Git][reproducible-builds/diffoscope][master] 3 commits: Drop unnecessary and unused assignment to "diff" variable.

Chris Lamb gitlab at salsa.debian.org
Wed Apr 15 18:11:41 UTC 2020



Chris Lamb pushed to branch master at Reproducible Builds / diffoscope


Commits:
f2170274 by Chris Lamb at 2020-04-15T19:10:32+01:00
Drop unnecessary and unused assignment to "diff" variable.

- - - - -
125b1406 by Michael Osipov at 2020-04-15T19:10:32+01:00
Revert to using zipinfo(1) directly instead of piping input via /dev/stdin for BSD portability. (Closes: reproducible-builds/diffoscope#97)

I have noticed that 25fee28c [via #879011] uses a non-portable approach by
utilizing the /dev/stdin symlink. zipinfo exits with 9. I have described the
problem with /dev/stdin in detail here, and about its non-portability.  The
upshot is:

 * /dev/stdin is not POSIX
 * It may not behave the way you expect
 * unzip explicitly does not support pipe or character devices:

    Archives read from standard input are not yet supported, except with
    funzip (and then only the first member of the archive can be
    extracted).

When you read the source code you'll see that stat(2) output is not
checked for regular file and it queries the stat struct for st_size is
is 0 for streams like pipes or character devices.

It maybe some special feature of Linux/glibc to replace the stat
information of /dev/stdin of the file read, but that is, IMHO, highly
questionable.

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

- - - - -
5ed866e6 by Chris Lamb at 2020-04-15T19:11:33+01:00
Strip paths from the output of zipinfo(1) warnings. (Re. reproducible-builds/diffoscope#97)

- - - - -


5 changed files:

- diffoscope/comparators/zip.py
- tests/comparators/test_apk.py
- tests/comparators/test_zip.py
- tests/data/jmod_zipinfo_expected_diff
- tests/data/mozzip_zipinfo_expected_diff


Changes:

=====================================
diffoscope/comparators/zip.py
=====================================
@@ -41,20 +41,23 @@ class Zipinfo(Command):
     # which are safe to ignore.
     VALID_RETURNCODES = {0, 1, 2}
 
+    re_strip_path = re.compile(r'^(warning|error) \[[^\]]+\]:\s+(.*)$')
+
     @tool_required('zipinfo')
     def cmdline(self):
-        # zipinfo (without -v) puts warning messages (some of which contain
-        # $path) into stdin when stderr is not a tty, see #879011 for details.
-        # to work around it, we run it on /dev/stdin instead, seems to work ok.
-        return ['zipinfo', '/dev/stdin']
-
-    def stdin(self):
-        return open(self.path, 'rb')
+        return ['zipinfo', self.path]
 
     def filter(self, line):
         # we don't care about the archive file path
         if line.startswith(b'Archive:'):
             return b''
+
+        # Strip paths from errors and warnings
+        # eg: "warning [/full/path]: 472 extra bytes at beginning or within zipfile"
+        m = self.re_strip_path.match(line.decode('utf-8'))
+        if m is not None:
+            return '{}: {}\n'.format(m.group(1), m.group(2)).encode('utf-8')
+
         return line
 
 


=====================================
tests/comparators/test_apk.py
=====================================
@@ -66,8 +66,8 @@ def test_compare_non_existing(monkeypatch, apk1):
 
 @skip_unless_tools_exist('apktool', 'zipinfo')
 def test_zipinfo(differences):
-    assert differences[0].source1 == 'zipinfo /dev/stdin'
-    assert differences[0].source2 == 'zipinfo /dev/stdin'
+    assert differences[0].source1 == 'zipinfo {}'
+    assert differences[0].source2 == 'zipinfo {}'
     expected_diff = get_data('apk_zipinfo_expected_diff')
     assert differences[0].unified_diff == expected_diff
 


=====================================
tests/comparators/test_zip.py
=====================================
@@ -100,12 +100,7 @@ def mozzip_differences(mozzip1, mozzip2):
 @skip_unless_tools_exist('zipinfo')
 def test_mozzip_metadata(mozzip_differences, mozzip1, mozzip2):
     expected_diff = get_data('mozzip_zipinfo_expected_diff')
-    diff = mozzip_differences[0].unified_diff
-    assert (
-        diff.replace(mozzip1.path, 'test1.mozzip').replace(
-            mozzip2.path, 'test2.mozzip'
-        )
-    ) == expected_diff
+    assert mozzip_differences[0].unified_diff == expected_diff
 
 
 @skip_unless_tools_exist('zipinfo')
@@ -138,7 +133,6 @@ def jmod_differences(jmod1, jmod2):
 @skip_unless_tools_exist('zipinfo')
 def test_jmod_metadata(jmod_differences, jmod1, jmod2):
     expected_diff = get_data('jmod_zipinfo_expected_diff')
-    diff = jmod_differences[0].unified_diff
     assert jmod_differences[0].unified_diff == expected_diff
 
 


=====================================
tests/data/jmod_zipinfo_expected_diff
=====================================
@@ -1,7 +1,7 @@
 @@ -1,8 +1,11 @@
 -Zip file size: 9857 bytes, number of entries: 4
 +Zip file size: 11672 bytes, number of entries: 7
- warning [/dev/stdin]:  4 extra bytes at beginning or within zipfile
+ warning: 4 extra bytes at beginning or within zipfile
    (attempting to process anyway)
 --rw----     2.0 fat      680 bl defN 19-Jul-17 04:54 classes/module-info.class
 +-rw----     2.0 fat      263 bl defN 19-Jul-17 04:54 classes/module-info.class


=====================================
tests/data/mozzip_zipinfo_expected_diff
=====================================
@@ -1,10 +1,10 @@
 @@ -1,8 +1,8 @@
 -Zip file size: 409 bytes, number of entries: 1
--warning [/dev/stdin]:  329 extra bytes at beginning or within zipfile
+-warning: 329 extra bytes at beginning or within zipfile
 +Zip file size: 552 bytes, number of entries: 1
-+warning [/dev/stdin]:  472 extra bytes at beginning or within zipfile
++warning: 472 extra bytes at beginning or within zipfile
    (attempting to process anyway)
- error [/dev/stdin]:  reported length of central directory is
+ error: reported length of central directory is
 -  -329 bytes too long (Atari STZip zipfile?  J.H.Holm ZIPSPLIT 1.1
 +  -472 bytes too long (Atari STZip zipfile?  J.H.Holm ZIPSPLIT 1.1
    zipfile?).  Compensating...



View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/75be0e2e8dd4d63c431c23b1d49209a9ffdf1bb2...5ed866e60555e28d4d7fa51fab132be592e9d1e1

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/diffoscope/-/compare/75be0e2e8dd4d63c431c23b1d49209a9ffdf1bb2...5ed866e60555e28d4d7fa51fab132be592e9d1e1
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/20200415/1b914728/attachment.htm>


More information about the rb-commits mailing list