Skip to content

Commit

Permalink
do not constrain the output of the Groups command in opengrok-groups (#…
Browse files Browse the repository at this point in the history
…4671)

fixes #4670

Also, do not install Python packages on Linux as the system environment is externally managed
  • Loading branch information
vladak authored Oct 11, 2024
1 parent 252a540 commit e68aafe
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 26 deletions.
6 changes: 0 additions & 6 deletions dev/before_install
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 3 additions & 2 deletions tools/src/main/python/opengrok_tools/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
29 changes: 22 additions & 7 deletions tools/src/main/python/opengrok_tools/utils/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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__)

Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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] = "... <truncated>"

Expand Down
6 changes: 4 additions & 2 deletions tools/src/main/python/opengrok_tools/utils/java.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
"""
Expand Down
44 changes: 35 additions & 9 deletions tools/src/test/python/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <[email protected]>
#

Expand All @@ -29,6 +29,8 @@
import tempfile
import time

from enum import Enum

import pytest

from conftest import posix_only, system_binary
Expand Down Expand Up @@ -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):
Expand All @@ -202,21 +224,25 @@ 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()

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


Expand Down

0 comments on commit e68aafe

Please sign in to comment.