From e68aafe3e247d950d007cfe1204ca3f54300c8c6 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Fri, 11 Oct 2024 16:21:51 +0200 Subject: [PATCH] do not constrain the output of the Groups command in opengrok-groups (#4671) fixes #4670 Also, do not install Python packages on Linux as the system environment is externally managed --- dev/before_install | 6 --- .../src/main/python/opengrok_tools/groups.py | 5 ++- .../python/opengrok_tools/utils/command.py | 29 +++++++++--- .../main/python/opengrok_tools/utils/java.py | 6 ++- tools/src/test/python/test_command.py | 44 +++++++++++++++---- 5 files changed, 64 insertions(+), 26 deletions(-) diff --git a/dev/before_install b/dev/before_install index f7ff69c982d..22ed36a5194 100755 --- a/dev/before_install +++ b/dev/before_install @@ -28,12 +28,6 @@ if [[ "$RUNNER_OS" == "Linux" ]]; then # Bitkeeper install failure is not critical, so exit code is not checked. sudo ./dev/install-bitkeeper.sh - sudo -H ./dev/install-python-packages.sh - if [[ $? != 0 ]]; then - echo "cannot install Python packages" - exit 1 - fi - sudo ./dev/install-universal_ctags.sh if [[ $? != 0 ]]; then echo "cannot install Universal ctags" diff --git a/tools/src/main/python/opengrok_tools/groups.py b/tools/src/main/python/opengrok_tools/groups.py index a034eff629f..7c0689e94af 100755 --- a/tools/src/main/python/opengrok_tools/groups.py +++ b/tools/src/main/python/opengrok_tools/groups.py @@ -18,7 +18,7 @@ # CDDL HEADER END # -# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved. # import argparse @@ -51,7 +51,8 @@ def main(): cmd = Java(args.options, classpath=args.jar, java=args.java, java_opts=args.java_opts, redirect_stderr=False, main_class='org.opengrok.indexer.configuration.Groups', - logger=logger, doprint=args.doprint) + logger=logger, doprint=args.doprint, + max_line_length=-1, max_lines=-1) cmd.execute() ret = cmd.getretcode() if ret is None or ret != SUCCESS_EXITVAL: diff --git a/tools/src/main/python/opengrok_tools/utils/command.py b/tools/src/main/python/opengrok_tools/utils/command.py index 190fe8d96fd..06cd9ce9797 100644 --- a/tools/src/main/python/opengrok_tools/utils/command.py +++ b/tools/src/main/python/opengrok_tools/utils/command.py @@ -18,7 +18,7 @@ # # -# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved. # import logging @@ -49,10 +49,13 @@ class Command: ERRORED = "errored" TIMEDOUT = "timed out" + MAX_LINE_LENGTH_DEFAULT = 250 + MAX_LINES_DEFAULT = 10000 + def __init__(self, cmd, args_subst=None, args_append=None, logger=None, excl_subst=False, work_dir=None, env_vars=None, timeout=None, redirect_stderr=True, resource_limits=None, doprint=False, - max_line_length=250, max_lines=10000): + max_line_length=None, max_lines=None): if doprint is None: doprint = False @@ -72,8 +75,17 @@ def __init__(self, cmd, args_subst=None, args_append=None, logger=None, self.doprint = doprint self.err = None self.returncode = None - self.max_line_length = int(max_line_length) - self.max_lines = int(max_lines) + + # Convert the maximums to integers to avoid exceptions when using them as indexes + # in case they are passed as floats. + if (max_line_length is None): + self.max_line_length = self.MAX_LINE_LENGTH_DEFAULT + else: + self.max_line_length = int(max_line_length) + if (max_lines is None): + self.max_lines = self.MAX_LINES_DEFAULT + else: + self.max_lines = int(max_lines) self.logger = logger or logging.getLogger(__name__) @@ -162,7 +174,10 @@ class OutputThread(threading.Thread): stdout/stderr buffers fill up. """ - def __init__(self, event, logger, doprint=False, max_line_length=250, max_lines=10000): + def __init__(self, event, logger, doprint=False, + max_line_length=Command.MAX_LINE_LENGTH_DEFAULT, + max_lines=Command.MAX_LINES_DEFAULT): + super(OutputThread, self).__init__() self.read_fd, self.write_fd = os.pipe() self.pipe_fobj = os.fdopen(self.read_fd, encoding='utf8') @@ -195,11 +210,11 @@ def run(self): line = line.rstrip() # This will remove not only newline but also whitespace. # Assuming that self.max_line_length is bigger than 3. - if len(line) > self.max_line_length: + if self.max_line_length > 0 and len(line) > self.max_line_length: line = line[:self.max_line_length] + "..." # Shorten the list to be one less than the maximum because a line is going to be added. - if len(self.out) >= self.max_lines: + if self.max_lines > 0 and len(self.out) >= self.max_lines: self.out = self.out[-self.max_lines + 1:] self.out[0] = "... " diff --git a/tools/src/main/python/opengrok_tools/utils/java.py b/tools/src/main/python/opengrok_tools/utils/java.py index d8d659e7ee7..1dc0b44e596 100755 --- a/tools/src/main/python/opengrok_tools/utils/java.py +++ b/tools/src/main/python/opengrok_tools/utils/java.py @@ -37,7 +37,8 @@ class Java(Command): def __init__(self, command, logger=None, main_class=None, java=None, jar=None, java_opts=None, classpath=None, env_vars=None, - redirect_stderr=True, doprint=False): + redirect_stderr=True, doprint=False, + max_line_length=None, max_lines=None): if not java: java = self.FindJava(logger) @@ -72,7 +73,8 @@ def __init__(self, command, logger=None, main_class=None, java=None, logger.debug("Java command: {}".format(java_command)) super().__init__(java_command, logger=logger, env_vars=env, - redirect_stderr=redirect_stderr, doprint=doprint) + redirect_stderr=redirect_stderr, doprint=doprint, + max_line_length=max_line_length, max_lines=max_lines) def FindJava(self, logger): """ diff --git a/tools/src/test/python/test_command.py b/tools/src/test/python/test_command.py index ef77a58b1c9..3543ddd3a02 100755 --- a/tools/src/test/python/test_command.py +++ b/tools/src/test/python/test_command.py @@ -20,7 +20,7 @@ # # -# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2017, 2024, Oracle affiliates. All rights reserved. # Portions Copyright (c) 2020, Krystof Tulinger # @@ -29,6 +29,8 @@ import tempfile import time +from enum import Enum + import pytest from conftest import posix_only, system_binary @@ -181,19 +183,39 @@ def test_stderr(): assert 'root' in "\n".join(cmd.getoutput()) +class LongTestMode(Enum): + """ + Enum to parametrize the long output test below. + """ + ORIGINAL = 1 + SHORTEN = 2 + UNLIMITED = 3 + + # This test needs the "/bin/cat" command, therefore it is Unix only. @posix_only -@pytest.mark.parametrize('shorten', [True, False]) -def test_long_output(shorten): +@pytest.mark.parametrize("mode", [LongTestMode.SHORTEN, LongTestMode.ORIGINAL, LongTestMode.UNLIMITED]) +def test_long_output(mode): """ Test that output thread in the Command class captures all the output. (and also it does not hang the command by filling up the pipe) + Also test the truncation. + By default, stderr is redirected to stdout. """ - num_lines = 5000 - line_length = 1000 - # should be enough to fill a pipe + num_lines_orig = 15000 + line_length_orig = 1000 + + # By using different values than the defaults, the modes which set the lengths + # to non-negative/not-None values will verify that non-default values are acted upon. + assert line_length_orig > Command.MAX_LINE_LENGTH_DEFAULT + assert num_lines_orig > Command.MAX_LINES_DEFAULT + + num_lines = num_lines_orig + line_length = line_length_orig + + # Should be enough to fill a pipe on most systems. num_bytes = num_lines * (line_length + 1) with tempfile.NamedTemporaryFile() as file: for _ in range(num_lines): @@ -202,9 +224,12 @@ def test_long_output(shorten): file.flush() assert os.path.getsize(file.name) == num_bytes - if shorten: + if mode is LongTestMode.SHORTEN: num_lines //= 2 line_length //= 2 + elif mode is LongTestMode.UNLIMITED: + num_lines = -1 + line_length = -1 cmd = Command(["/bin/cat", file.name], max_lines=num_lines, max_line_length=line_length) cmd.execute() @@ -212,11 +237,12 @@ def test_long_output(shorten): assert cmd.getstate() == Command.FINISHED assert cmd.getretcode() == 0 assert cmd.geterroutput() is None - assert len(cmd.getoutput()) == num_lines - if shorten: + if mode is LongTestMode.SHORTEN: + assert len(cmd.getoutput()) == num_lines # Add 3 for the '...' suffix. assert all(len(x) <= line_length + 3 for x in cmd.getoutput()) else: + assert len(cmd.getoutput()) == num_lines_orig assert len("\n".join(cmd.getoutput()) + "\n") == num_bytes