Commit dcf2d40c authored by Ximin Luo's avatar Ximin Luo

comparators: simplify feed_stdin into a simple stdin() instead

parent 87d3050f
......@@ -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):
......
......@@ -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
......
......@@ -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
......
......@@ -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
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment