diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java index 3550f5956e7..1cf30a4fc76 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/util/Executor.java @@ -33,6 +33,7 @@ import java.io.Reader; import java.lang.Thread.UncaughtExceptionHandler; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -67,6 +68,7 @@ public class Executor { private byte[] stdout; private byte[] stderr; private int timeout; // in milliseconds, 0 means no timeout + private final Map environ; /** * Create a new instance of the Executor. @@ -88,47 +90,65 @@ public Executor(List cmdList) { * Create a new instance of the Executor with default command timeout value. * The timeout value will be based on the running context (indexer or web application). * @param cmdList A list containing the command to execute - * @param workingDirectory The directory the process should have as the - * working directory + * @param workingDirectory The directory the process should have as the working directory */ - public Executor(List cmdList, File workingDirectory) { - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - int timeoutSec = env.isIndexer() ? env.getIndexerCommandTimeout() : env.getInteractiveCommandTimeout(); - - this.cmdList = cmdList; - this.workingDirectory = workingDirectory; - this.timeout = timeoutSec * 1000; + public Executor(List cmdList, @Nullable File workingDirectory) { + this(cmdList, workingDirectory, new HashMap<>()); } /** * Create a new instance of the Executor with specific timeout value. * @param cmdList A list containing the command to execute - * @param workingDirectory The directory the process should have as the - * working directory + * @param workingDirectory The directory the process should have as the working directory * @param timeout If the command runs longer than the timeout (seconds), * it will be terminated. If the value is 0, no timer * will be set up. */ - public Executor(List cmdList, File workingDirectory, int timeout) { + public Executor(List cmdList, @Nullable File workingDirectory, int timeout) { + this(cmdList, workingDirectory, new HashMap<>()); + + this.timeout = timeout * 1000; + } + + /** + * Create a new instance of the Executor with default command timeout value and environment. + * The timeout value will be based on the running context (indexer or web application). + * @param cmdList A list containing the command to execute + * @param workingDirectory The directory the process should have as the working directory + * @param environ map of environment variables to be added/overridden + */ + public Executor(List cmdList, @Nullable File workingDirectory, Map environ) { this.cmdList = cmdList; this.workingDirectory = workingDirectory; - this.timeout = timeout * 1000; + this.environ = environ; + + setDefaultTimeout(); } /** * Create a new instance of the Executor with or without timeout. * @param cmdList A list containing the command to execute - * @param workingDirectory The directory the process should have as the - * working directory + * @param workingDirectory The directory the process should have as the working directory * @param useTimeout terminate the process after default timeout or not */ - public Executor(List cmdList, File workingDirectory, boolean useTimeout) { + public Executor(List cmdList, @Nullable File workingDirectory, boolean useTimeout) { this(cmdList, workingDirectory); if (!useTimeout) { this.timeout = 0; } } + private void setDefaultTimeout() { + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + int timeoutSec = env.isIndexer() ? env.getIndexerCommandTimeout() : env.getInteractiveCommandTimeout(); + + this.timeout = timeoutSec * 1000; + } + + int getTimeout() { + return timeout / 1000; + } + /** * Execute the command and collect the output. All exceptions will be * logged. @@ -180,6 +200,8 @@ public int exec(final boolean reportExceptions, StreamHandler handler) { .map(File::toString) .orElseGet(() -> System.getProperty("user.dir")); + processBuilder.environment().putAll(environ); + String envStr = ""; if (LOGGER.isLoggable(Level.FINER)) { Map envMap = processBuilder.environment(); @@ -222,7 +244,7 @@ public int exec(final boolean reportExceptions, StreamHandler handler) { @Override public void run() { LOGGER.log(Level.WARNING, String.format("Terminating process of command [%s] in directory '%s' " + - "due to timeout %d seconds", cmd_str, dir_str, timeout / 1000)); + "due to timeout %d seconds", cmd_str, dir_str, getTimeout())); proc.destroy(); } }, timeout); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 3230461d0f4..c9b29e4e02d 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -50,7 +50,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.TreeSet; @@ -187,15 +189,18 @@ void testGetHistoryPartial() throws Exception { * @param args {@code hg} command arguments */ public static void runHgCommand(File reposRoot, String... args) { - List cmdargs = new ArrayList<>(); + List commandWithArgs = new ArrayList<>(); MercurialRepository repo = new MercurialRepository(); - cmdargs.add(repo.getRepoCommand()); - cmdargs.addAll(Arrays.asList(args)); + commandWithArgs.add(repo.getRepoCommand()); + commandWithArgs.addAll(Arrays.asList(args)); - Executor exec = new Executor(cmdargs, reposRoot); + final Map env = new HashMap<>(); + // Set the user to record commits in order to prevent hg commands entering interactive mode. + env.put("HGUSER", "Snufkin "); + Executor exec = new Executor(commandWithArgs, reposRoot, env); int exitCode = exec.exec(); - assertEquals(0, exitCode, "hg command '" + cmdargs + "' failed." + assertEquals(0, exitCode, "command '" + commandWithArgs + "' failed." + "\nexit code: " + exitCode + "\nstdout:\n" + exec.getOutputString() + "\nstderr:\n" + exec.getErrorString()); @@ -299,10 +304,7 @@ void testGetHistoryGetRenamed() throws Exception { String exp_str = "This is totally plaintext file.\n"; byte[] buffer = new byte[1024]; - /* - * In our test repository the file was renamed twice since - * revision 3. - */ + // In our test repository the file was renamed twice since revision 3. InputStream input = mr.getHistoryGet(repositoryRoot.getCanonicalPath(), "novel.txt", "3"); assertNotNull(input); @@ -350,8 +352,7 @@ void testGetHistorySinceTillNullRev() throws Exception { assertNotNull(history); assertNotNull(history.getHistoryEntries()); assertEquals(6, history.getHistoryEntries().size()); - List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision). - collect(Collectors.toList()); + List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList(); assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 4, REVISIONS.length)), revisions); } @@ -362,8 +363,7 @@ void testGetHistorySinceTillRevNull() throws Exception { assertNotNull(history); assertNotNull(history.getHistoryEntries()); assertEquals(3, history.getHistoryEntries().size()); - List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision). - collect(Collectors.toList()); + List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList(); assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 0, 3)), revisions); } @@ -374,8 +374,7 @@ void testGetHistorySinceTillRevRev() throws Exception { assertNotNull(history); assertNotNull(history.getHistoryEntries()); assertEquals(5, history.getHistoryEntries().size()); - List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision). - collect(Collectors.toList()); + List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList(); assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 2, 7)), revisions); } @@ -389,8 +388,7 @@ void testGetHistoryRenamedFileTillRev() throws Exception { assertNotNull(history); assertNotNull(history.getHistoryEntries()); assertEquals(5, history.getHistoryEntries().size()); - List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision). - collect(Collectors.toList()); + List revisions = history.getHistoryEntries().stream().map(HistoryEntry::getRevision).toList(); assertEquals(List.of(Arrays.copyOfRange(REVISIONS, 2, 7)), revisions); } @@ -443,8 +441,10 @@ void testAnnotationNegative() throws Exception { } private static Stream, List>> provideParametersForPositiveAnnotationTest() { - return Stream.of(Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")), - Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268"))); + return Stream.of( + Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")), + Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268")) + ); } @ParameterizedTest @@ -579,7 +579,7 @@ void testBuildTagListOneMore() throws Exception { assertNotNull(tags); assertEquals(3, tags.size()); expectedTags = List.of("start_of_novel", newTagName, branchTagName); - assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).collect(Collectors.toList())); + assertEquals(expectedTags, tags.stream().map(TagEntry::getTags).toList()); // cleanup IOUtils.removeRecursive(repositoryRootPath); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ExecutorTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ExecutorTest.java index b2602abbe61..3b03ccd4ae6 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ExecutorTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/util/ExecutorTest.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2019, Chris Fraire . */ package org.opengrok.indexer.util; @@ -30,9 +30,12 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.io.Reader; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -45,6 +48,13 @@ */ class ExecutorTest { + @Test + void testConstructorWithTimeout() { + int timeout = 42; + Executor executor = new Executor(List.of("foo"), null, timeout); + assertEquals(timeout, executor.getTimeout()); + } + @Test void testString() { List cmdList = new ArrayList<>(); @@ -64,10 +74,14 @@ void testReader() throws IOException { cmdList.add("testing org.opengrok.indexer.util.Executor"); Executor instance = new Executor(cmdList); assertEquals(0, instance.exec()); - BufferedReader in = new BufferedReader(instance.getOutputReader()); + Reader outputReader = instance.getOutputReader(); + assertNotNull(outputReader); + BufferedReader in = new BufferedReader(outputReader); assertEquals("testing org.opengrok.indexer.util.Executor", in.readLine()); in.close(); - in = new BufferedReader(instance.getErrorReader()); + Reader errorReader = instance.getErrorReader(); + assertNotNull(errorReader); + in = new BufferedReader(errorReader); assertNull(in.readLine()); in.close(); } @@ -81,14 +95,44 @@ void testStream() throws IOException { assertEquals(0, instance.exec()); assertNotNull(instance.getOutputStream()); assertNotNull(instance.getErrorStream()); - BufferedReader in = new BufferedReader(instance.getOutputReader()); + Reader outputReader = instance.getOutputReader(); + assertNotNull(outputReader); + BufferedReader in = new BufferedReader(outputReader); assertEquals("testing org.opengrok.indexer.util.Executor", in.readLine()); in.close(); - in = new BufferedReader(instance.getErrorReader()); + Reader errorReader = instance.getErrorReader(); + assertNotNull(errorReader); + in = new BufferedReader(errorReader); assertNull(in.readLine()); in.close(); } + /** + * Test setting environment variable. Assumes the {@code env} program exists + * and reports list of environment variables along with their values. + */ + @Test + void testEnv() throws IOException { + List cmdList = List.of("env"); + final Map env = new HashMap<>(); + env.put("foo", "bar"); + Executor instance = new Executor(cmdList, null, env); + assertEquals(0, instance.exec()); + Reader outputReader = instance.getOutputReader(); + assertNotNull(outputReader); + BufferedReader in = new BufferedReader(outputReader); + String line; + boolean found = false; + while ((line = in.readLine()) != null) { + if (line.equals("foo=bar")) { + found = true; + break; + } + } + assertTrue(found); + in.close(); + } + /** * {@link Executor.StreamHandler} implementation that always fails with {@link IOException}. */