[diffoscope] Non-portable regression caused by 25fee28c/#879011

[Osipov, Michael]

Folks,

I started using diffoscope on FreeBSD to aid Hervé Boutemy's work for 
Maven reproducibility. I have noticed that 
25fee28c8b29dbc66cd7bb57a2ab427651050c23 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 [1], and 
about its non-portability [2].

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.

The attached patch solves the problem for me. It don't want to live with 
downstream patch forever. Please consider upstreaming it.

Regards,

Michael

[1] 
https://lists.freebsd.org/pipermail/freebsd-questions/2020-March/288177.html
[2] https://unix.stackexchange.com/a/36404/40618

-------------- next part --------------
From 5d551d2f0eda3a41bc7e282970ec4440b786ba52 Mon Sep 17 00:00:00 2001
From: Michael Osipov <michael.osipov at siemens.com>
Date: Thu, 12 Mar 2020 10:45:08 +0100
Subject: [PATCH] Restore portability of zipinfo call caused by a regression in
 25fee28c

/dev/stdin is a non-portable non-POSIX extension having different semantics on
different operating systems. zininfo(1) exits with 9 when /dev/stdin is
supplied on FreeBSD. In fact, unzip(1) explicitly documents that it does not
support reading from stdin.
---
 diffoscope/comparators/zip.py | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/diffoscope/comparators/zip.py b/diffoscope/comparators/zip.py
index 5bd5eae..a07947e 100644
--- a/diffoscope/comparators/zip.py
+++ b/diffoscope/comparators/zip.py
@@ -43,13 +43,7 @@ class Zipinfo(Command):
 
     @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
-- 
2.25.0