You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2022/03/03 20:08:42 UTC
[lucene] branch branch_9x updated: LUCENE-10447: always use utf8 for forked process encoding. Use the sa… (#717)
This is an automated email from the ASF dual-hosted git repository.
dweiss pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/lucene.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 8f92ec1 LUCENE-10447: always use utf8 for forked process encoding. Use the sa… (#717)
8f92ec1 is described below
commit 8f92ec157f1a01e7903186da8607e3d1003b1829
Author: Dawid Weiss <da...@carrotsearch.com>
AuthorDate: Thu Mar 3 20:53:20 2022 +0100
LUCENE-10447: always use utf8 for forked process encoding. Use the sa… (#717)
---
.../apache/lucene/distribution/TestScripts.java | 25 ++++++++++++++--------
.../distribution/src/binary-release/bin/luke.cmd | 15 ++++++++-----
lucene/distribution/src/binary-release/bin/luke.sh | 13 ++++++++++-
3 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/lucene/distribution.tests/src/test/org/apache/lucene/distribution/TestScripts.java b/lucene/distribution.tests/src/test/org/apache/lucene/distribution/TestScripts.java
index 9c3be02..26565b0 100644
--- a/lucene/distribution.tests/src/test/org/apache/lucene/distribution/TestScripts.java
+++ b/lucene/distribution.tests/src/test/org/apache/lucene/distribution/TestScripts.java
@@ -25,6 +25,7 @@ import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
@@ -48,13 +49,19 @@ public class TestScripts extends AbstractLuceneDistributionTest {
distributionPath = getDistributionPath();
}
+ Path currentJava =
+ Paths.get(System.getProperty("java.home"), "bin", WINDOWS ? "java.exe" : "java");
+ Assertions.assertThat(currentJava).exists();
+
Path lukeScript = resolveScript(distributionPath.resolve("bin").resolve("luke"));
Launcher launcher =
new ProcessBuilderLauncher()
.executable(lukeScript)
- // tweak Windows launcher scripts so that they don't fork asynchronous java.
- .envvar("DISTRIBUTION_TESTING", "true")
+ // pass the same JVM which the tests are currently using; this also forces UTF-8 as
+ // console
+ // encoding so that we know we can safely read it.
+ .envvar("LAUNCH_CMD", currentJava.toAbsolutePath().toString())
.viaShellLauncher()
.cwd(distributionPath)
.args("--sanity-check");
@@ -63,7 +70,9 @@ public class TestScripts extends AbstractLuceneDistributionTest {
launcher,
0,
60,
- (output) -> {
+ (outputBytes) -> {
+ // We know it's UTF-8 because we set file.encoding explicitly.
+ var output = Files.readString(outputBytes, StandardCharsets.UTF_8);
Assertions.assertThat(output).contains("[Vader] Hello, Luke.");
});
}
@@ -96,14 +105,14 @@ public class TestScripts extends AbstractLuceneDistributionTest {
() -> {
// The default charset for a forked java process could be computed for the current
// platform but it adds more complexity. For now, assume it's just parseable ascii.
- return StandardCharsets.US_ASCII;
+ return StandardCharsets.ISO_8859_1;
};
- protected String execute(
+ protected void execute(
Launcher launcher,
int expectedExitCode,
long timeoutInSeconds,
- ThrowingConsumer<String> consumer)
+ ThrowingConsumer<Path> processOutputConsumer)
throws Exception {
try (ForkedProcess forkedProcess = launcher.execute()) {
@@ -122,9 +131,7 @@ public class TestScripts extends AbstractLuceneDistributionTest {
.as("forked process exit status")
.isEqualTo(expectedExitCode);
- String output = Files.readString(forkedProcess.getProcessOutputFile(), charset);
- consumer.accept(output);
- return output;
+ processOutputConsumer.accept(forkedProcess.getProcessOutputFile());
} catch (Throwable t) {
logSubprocessOutput(
command, Files.readString(forkedProcess.getProcessOutputFile(), charset));
diff --git a/lucene/distribution/src/binary-release/bin/luke.cmd b/lucene/distribution/src/binary-release/bin/luke.cmd
index 9c20710..bd1f66a 100644
--- a/lucene/distribution/src/binary-release/bin/luke.cmd
+++ b/lucene/distribution/src/binary-release/bin/luke.cmd
@@ -18,14 +18,19 @@
SETLOCAL
SET MODULES=%~dp0..
-REM For distribution testing we want plain 'java' command, otherwise we can't block
-REM on luke invocation and can't intercept the return status.
+IF DEFINED LAUNCH_CMD GOTO testing
SET LAUNCH_CMD=start javaw
-IF NOT "%DISTRIBUTION_TESTING%"=="true" GOTO launch
-SET LAUNCH_CMD=java
+SET LAUNCH_OPTS=
+goto launch
+
+:testing
+REM For distribution testing we don't use start and pass an explicit launch ('java') command,
+REM otherwise we can't block on luke invocation and can't intercept the return status.
+REM We also force UTF-8 encoding.
+SET LAUNCH_OPTS=-Dfile.encoding=UTF-8
:launch
-%LAUNCH_CMD% --module-path "%MODULES%\modules;%MODULES%\modules-thirdparty" --module org.apache.lucene.luke %*
+"%LAUNCH_CMD%" %LAUNCH_OPTS% --module-path "%MODULES%\modules;%MODULES%\modules-thirdparty" --module org.apache.lucene.luke %*
SET EXITVAL=%errorlevel%
EXIT /b %EXITVAL%
ENDLOCAL
diff --git a/lucene/distribution/src/binary-release/bin/luke.sh b/lucene/distribution/src/binary-release/bin/luke.sh
index b2d7d5e..3d92087 100644
--- a/lucene/distribution/src/binary-release/bin/luke.sh
+++ b/lucene/distribution/src/binary-release/bin/luke.sh
@@ -17,5 +17,16 @@
MODULES=`dirname "$0"`/..
MODULES=`cd "$MODULES" && pwd`
-java --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@"
+
+# check for overridden launch command (for use in integration tests), otherwise
+# use the default.
+if [ -z "$LAUNCH_CMD" ]; then
+ LAUNCH_CMD=java
+ LAUNCH_OPTS=
+else
+ # We are integration-testing. Force UTF-8 as the encoding.
+ LAUNCH_OPTS=-Dfile.encoding=UTF-8
+fi
+
+"$LAUNCH_CMD" $LAUNCH_OPTS --module-path "$MODULES/modules:$MODULES/modules-thirdparty" --module org.apache.lucene.luke "$@"
exit $?