[diffoscope] 01/01: comparators: simplify feed_stdin into a simple stdin() instead
Ximin Luo
infinity0 at debian.org
Tue Nov 28 21:02:46 CET 2017
This is an automated email from the git hooks/post-receive script.
infinity0 pushed a commit to branch master
in repository diffoscope.
commit dcf2d40c0418da8e2c9c0a348988f02bd979a6d1
Author: Ximin Luo <infinity0 at debian.org>
Date: Tue Nov 28 21:01:24 2017 +0100
comparators: simplify feed_stdin into a simple stdin() instead
---
diffoscope/comparators/png.py | 6 ++----
diffoscope/comparators/utils/command.py | 32 +++++++++++++-------------------
diffoscope/comparators/zip.py | 5 ++++-
tests/comparators/test_utils.py | 13 ++++++++++---
4 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/diffoscope/comparators/png.py b/diffoscope/comparators/png.py
index 2e193b9..e11d3a1 100644
--- a/diffoscope/comparators/png.py
+++ b/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):
diff --git a/diffoscope/comparators/utils/command.py b/diffoscope/comparators/utils/command.py
index 9453a1b..1d4cfde 100644
--- a/diffoscope/comparators/utils/command.py
+++ b/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
diff --git a/diffoscope/comparators/zip.py b/diffoscope/comparators/zip.py
index 950abc6..58a63ce 100644
--- a/diffoscope/comparators/zip.py
+++ b/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
diff --git a/tests/comparators/test_utils.py b/tests/comparators/test_utils.py
index 7de6350..dd7b028 100644
--- a/tests/comparators/test_utils.py
+++ b/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
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/diffoscope.git
More information about the diffoscope
mailing list