comparators: simplify feed_stdin into a simple stdin() instead

diffoscope/comparators/png.py

@@ -38,10 +38,8 @@ class Sng(Command):
     def cmdline(self):
         return ['sng']
 
-    def feed_stdin(self, stdin):
-        with open(self.path, 'rb') as f:
-            for buf in iter(functools.partial(f.read, 32768), b''):
-                stdin.write(buf)
+    def stdin(self):
+        return open(self.path, 'rb')
 
 
 class PngFile(File):

diffoscope/comparators/utils/command.py

@@ -33,19 +33,19 @@ class Command(object, metaclass=abc.ABCMeta):
 
     def start(self):
         logger.debug("Executing %s", ' '.join([shlex.quote(x) for x in self.cmdline()]))
+        self._stdin = self.stdin()
+        # "stdin" used to be a feeder but we didn't need the functionality so
+        # it was simplified into the current form. it can be recovered from git
+        # the extra functionality is needed in the future. alternatively,
+        # consider using a shell pipeline ("sh -ec $script") to implement what
+        # you need, because that involves much less code - like it or not (I
+        # don't) shell is still the most readable option for composing processes
         self._process = subprocess.Popen(self.cmdline(),
                                          shell=False, close_fds=True,
                                          env=self.env(),
-                                         stdin=subprocess.PIPE,
+                                         stdin=self._stdin,
                                          stdout=subprocess.PIPE,
                                          stderr=subprocess.PIPE)
-        if hasattr(self, 'feed_stdin'):
-            self._stdin_feeder = threading.Thread(target=self._feed_stdin, args=(self._process.stdin,))
-            self._stdin_feeder.daemon = True
-            self._stdin_feeder.start()
-        else:
-            self._stdin_feeder = None
-            self._process.stdin.close()
         self._stderr = io.BytesIO()
         self._stderr_line_count = 0
         self._stderr_reader = threading.Thread(target=self._read_stderr)
@@ -56,6 +56,9 @@ class Command(object, metaclass=abc.ABCMeta):
     def path(self):
         return self._path
 
+    def stdin(self):
+        return None
+
     @abc.abstractmethod
     def cmdline(self):
         raise NotImplementedError()
@@ -66,15 +69,6 @@ class Command(object, metaclass=abc.ABCMeta):
     def env(self):
         return None  # inherit parent environment by default
 
-    # Define only if needed. We take care of closing stdin.
-    #def feed_stdin(self, stdin)
-
-    def _feed_stdin(self, stdin):
-        try:
-            self.feed_stdin(stdin)
-        finally:
-            stdin.close()
-
     def filter(self, line):
         # Assume command output is utf-8 by default
         return line
@@ -86,8 +80,6 @@ class Command(object, metaclass=abc.ABCMeta):
         return self._process.terminate()
 
     def wait(self):
-        if self._stdin_feeder:
-            self._stdin_feeder.join()
         self._stderr_reader.join()
         returncode = self._process.wait()
         logger.debug(
@@ -95,6 +87,8 @@ class Command(object, metaclass=abc.ABCMeta):
             ' '.join([shlex.quote(x) for x in self.cmdline()]),
             returncode,
         )
+        if self._stdin:
+            self._stdin.close()
         return returncode
 
     MAX_STDERR_LINES = 50

diffoscope/comparators/zip.py

@@ -38,7 +38,10 @@ class Zipinfo(Command):
         # 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 ['sh', '-ec', 'exec zipinfo /dev/stdin < "$1"', '-', self.path]
+        return ['zipinfo', '/dev/stdin']
+
+    def stdin(self):
+        return open(self.path, 'rb')
 
     def filter(self, line):
         # we don't care about the archive file path

tests/comparators/test_utils.py

@@ -18,7 +18,9 @@
 # along with diffoscope.  If not, see <https://www.gnu.org/licenses/>.
 
 import codecs
+import os
 import pytest
+import threading
 
 from diffoscope.config import Config
 from diffoscope.difference import Difference
@@ -107,8 +109,13 @@ def test_trim_stderr_in_command():
         def cmdline(self):
             return ['tee', '/dev/stderr']
 
-        def feed_stdin(self, stdin):
-            for dummy in range(0, Command.MAX_STDERR_LINES + 1):
-                stdin.write('error {}\n'.format(self.path).encode('utf-8'))
+        def stdin(self):
+            r, w = os.pipe()
+            r, w = os.fdopen(r), os.fdopen(w, "w")
+            def write():
+                for dummy in range(0, Command.MAX_STDERR_LINES + 1):
+                    w.write('error {}\n'.format(self.path))
+            threading.Thread(target=write).start()
+            return r
     difference = Difference.from_command(FillStderr, 'dummy1', 'dummy2')
     assert '[ 1 lines ignored ]' in difference.comment