Windows, java launcher: clean up after flag flip
Remove --incompatible_windows_escape_jvm_flags.
The flag is now flipped to true, and the
false-branch logic is removed.
See https://github.com/bazelbuild/bazel/issues/7486
RELNOTES[INC]: --incompatible_windows_escape_jvm_flags is enabled by default, and the flag no longer exists
Change-Id: I4ba9a626c28ce9690c0850fa052c302f80bccc1c
Closes #7972.
Change-Id: I4ba9a626c28ce9690c0850fa052c302f80bccc1c
PiperOrigin-RevId: 242482659
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index ce2ffac..2fe0580 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -404,32 +404,18 @@
arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", jvmFlagsList));
if (OS.getCurrent() == OS.WINDOWS) {
- boolean windowsEscapeJvmFlags =
- ruleContext
- .getConfiguration()
- .getFragment(JavaConfiguration.class)
- .windowsEscapeJvmFlags();
-
List<String> jvmFlagsForLauncher = jvmFlagsList;
- if (windowsEscapeJvmFlags) {
- try {
- jvmFlagsForLauncher = new ArrayList<>(jvmFlagsList.size());
- for (String f : jvmFlagsList) {
- ShellUtils.tokenize(jvmFlagsForLauncher, f);
- }
- } catch (TokenizationException e) {
- ruleContext.attributeError("jvm_flags", "could not Bash-tokenize flag: " + e);
+ try {
+ jvmFlagsForLauncher = new ArrayList<>(jvmFlagsList.size());
+ for (String f : jvmFlagsList) {
+ ShellUtils.tokenize(jvmFlagsForLauncher, f);
}
+ } catch (TokenizationException e) {
+ ruleContext.attributeError("jvm_flags", "could not Bash-tokenize flag: " + e);
}
return createWindowsExeLauncher(
- ruleContext,
- javaExecutable,
- classpath,
- javaStartClass,
- jvmFlagsForLauncher,
- executable,
- windowsEscapeJvmFlags);
+ ruleContext, javaExecutable, classpath, javaStartClass, jvmFlagsForLauncher, executable);
}
ruleContext.registerAction(new TemplateExpansionAction(
@@ -443,8 +429,7 @@
NestedSet<Artifact> classpath,
String javaStartClass,
List<String> jvmFlags,
- Artifact javaLauncher,
- boolean windowsEscapeJvmFlags) {
+ Artifact javaLauncher) {
LaunchInfo launchInfo =
LaunchInfo.builder()
.addKeyValuePair("binary_type", "Java")
@@ -464,7 +449,6 @@
"classpath",
";",
Iterables.transform(classpath, Artifact.ROOT_RELATIVE_PATH_STRING))
- .addKeyValuePair("escape_jvmflags", windowsEscapeJvmFlags ? "1" : "0")
// TODO(laszlocsomor): Change the Launcher to accept multiple jvm_flags entries. As of
// 2019-02-13 the Launcher accepts just one jvm_flags entry, which contains all the
// flags, joined by TAB characters. The Launcher splits up the string to get the
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
index 99a6f85..aa113b0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
@@ -178,7 +178,6 @@
private final ImmutableList<Label> pluginList;
private final boolean requireJavaToolchainHeaderCompilerDirect;
private final boolean disallowResourceJars;
- private final boolean windowsEscapeJvmFlags;
// TODO(dmarting): remove once we have a proper solution for #2539
private final boolean useLegacyBazelJavaTest;
@@ -216,7 +215,6 @@
this.isJlplStrictDepsEnforced = javaOptions.isJlplStrictDepsEnforced;
this.disallowResourceJars = javaOptions.disallowResourceJars;
this.addTestSupportToCompileTimeDeps = javaOptions.addTestSupportToCompileTimeDeps;
- this.windowsEscapeJvmFlags = javaOptions.windowsEscapeJvmFlags;
ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
for (String s : javaOptions.translationTargets) {
@@ -491,8 +489,4 @@
public boolean disallowResourceJars() {
return disallowResourceJars;
}
-
- public boolean windowsEscapeJvmFlags() {
- return windowsEscapeJvmFlags;
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index 63f6a75..ac9affb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -613,27 +613,6 @@
"Disables the resource_jars attribute; use java_import and deps or runtime_deps instead.")
public boolean disallowResourceJars;
- @Option(
- name = "incompatible_windows_escape_jvm_flags",
- defaultValue = "true",
- documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
- effectTags = {
- OptionEffectTag.ACTION_COMMAND_LINES,
- OptionEffectTag.AFFECTS_OUTPUTS,
- },
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help =
- "On Linux/macOS/non-Windows: no-op. On Windows: this flag affects how java_binary and"
- + " java_test targets are built; in particular, how the launcher of these targets"
- + " escapes flags at the time of running the java_binary/java_test. When the flag is"
- + " true, the launcher escapes the JVM flags using Windows-style escaping (correct"
- + " behavior). When the flag is false, the launcher uses Bash-style escaping"
- + " (buggy behavior). See https://github.com/bazelbuild/bazel/issues/7072")
- public boolean windowsEscapeJvmFlags;
-
Label defaultJavaBase() {
return Label.parseAbsoluteUnchecked(DEFAULT_JAVABASE);
}
@@ -695,8 +674,6 @@
host.disallowResourceJars = disallowResourceJars;
- host.windowsEscapeJvmFlags = windowsEscapeJvmFlags;
-
return host;
}
diff --git a/src/test/shell/integration/jvm_flags_escaping_test.sh b/src/test/shell/integration/jvm_flags_escaping_test.sh
index 9d80412..50c51ca 100755
--- a/src/test/shell/integration/jvm_flags_escaping_test.sh
+++ b/src/test/shell/integration/jvm_flags_escaping_test.sh
@@ -255,26 +255,6 @@
# TESTS
# ----------------------------------------------------------------------
-function test_jvm_flags_escaping_when_it_is_disabled_on_windows() {
- local -r pkg="${FUNCNAME[0]}" # unique package name for this test
-
- create_java_file_that_prints_jvm_args "$pkg"
- create_build_file_with_many_jvm_flags "$pkg"
-
- # On all platforms, Bazel can build the target.
- bazel build --verbose_failures --noincompatible_windows_escape_jvm_flags \
- "${pkg}:x" &>"$TEST_log" || fail "expected success"
- if "$is_windows"; then
- # On Windows, the target cannot run.
- expect_program_cannot_run "bazel-bin/$pkg/x${EXE_EXT}"
- expect_log "Could not find or load main class"
- else
- # On other platforms, the program can run fine.
- expect_program_runs "bazel-bin/$pkg/x${EXE_EXT}"
- assert_output_of_the_program_with_many_jvm_flags
- fi
-}
-
function test_jvm_flags_escaping() {
local -r pkg="${FUNCNAME[0]}" # unique package name for this test
@@ -282,51 +262,28 @@
create_build_file_with_many_jvm_flags "$pkg"
# On all platforms, Bazel can build and run the target.
- bazel build --verbose_failures --incompatible_windows_escape_jvm_flags \
+ bazel build --verbose_failures \
"${pkg}:x" &>"$TEST_log" || fail "expected success"
expect_program_runs "bazel-bin/$pkg/x${EXE_EXT}"
assert_output_of_the_program_with_many_jvm_flags
}
-function test_untokenizable_jvm_flag_when_escaping_is_disabled() {
- local -r pkg="${FUNCNAME[0]}" # unique package name for this test
-
- create_java_file_that_prints_jvm_args "$pkg"
- create_build_file_for_untokenizable_flag "$pkg"
-
- # On all platforms, Bazel can build the target.
- bazel build --verbose_failures --noincompatible_windows_escape_jvm_flags \
- "${pkg}:cannot_tokenize" 2>"$TEST_log" || fail "expected success"
- if "$is_windows"; then
- # On Windows, Bazel will not check the flag. It will just propagate the flag
- # to the launcher, which also just passes it to the JVM. This is bad, the
- # flag should have been rejected because it cannot be Bash-tokenized.
- expect_program_runs "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
- expect_log "arg0=('abc)"
- else
- # On other platforms, Bazel will build the target but it fails to run.
- expect_program_cannot_run "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
- expect_log "syntax error"
- fi
-}
-
function test_untokenizable_jvm_flag_when_escaping_is_enabled() {
local -r pkg="${FUNCNAME[0]}" # unique package name for this test
create_java_file_that_prints_jvm_args "$pkg"
create_build_file_for_untokenizable_flag "$pkg"
- local -r flag="--incompatible_windows_escape_jvm_flags"
if "$is_windows"; then
# On Windows, Bazel will check the flag. It will just propagate the flag
# to the launcher, which also just passes the flag to the JVM. This is bad,
# the flag should have been rejected because it cannot be Bash-tokenized.
- bazel build --verbose_failures "$flag" "${pkg}:cannot_tokenize" \
+ bazel build --verbose_failures "${pkg}:cannot_tokenize" \
2>"$TEST_log" && fail "expected failure" || true
expect_log "ERROR:.*in jvm_flags attribute of java_binary rule"
else
# On other platforms, Bazel will build the target but it fails to run.
- bazel build --verbose_failures "$flag" "${pkg}:cannot_tokenize" \
+ bazel build --verbose_failures "${pkg}:cannot_tokenize" \
2>"$TEST_log" || fail "expected success"
expect_program_cannot_run "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
expect_log "syntax error"
diff --git a/src/tools/launcher/java_launcher.cc b/src/tools/launcher/java_launcher.cc
index 353b5d2..01af0cf 100644
--- a/src/tools/launcher/java_launcher.cc
+++ b/src/tools/launcher/java_launcher.cc
@@ -43,7 +43,6 @@
static constexpr const char* CLASSPATH = "classpath";
static constexpr const char* JAVA_START_CLASS = "java_start_class";
static constexpr const char* JVM_FLAGS = "jvm_flags";
-static constexpr const char* WINDOWS_STYLE_ESCAPE_JVM_FLAGS = "escape_jvmflags";
// Check if a string start with a certain prefix.
// If it's true, store the substring without the prefix in value.
@@ -409,15 +408,10 @@
arguments.push_back(arg);
}
- wstring (*const escape_arg_func)(const wstring&) =
- this->GetLaunchInfoByKey(WINDOWS_STYLE_ESCAPE_JVM_FLAGS) == L"1"
- ? WindowsEscapeArg2
- : WindowsEscapeArg;
-
vector<wstring> escaped_arguments;
// Quote the arguments if having spaces
for (const auto& arg : arguments) {
- escaped_arguments.push_back(escape_arg_func(arg));
+ escaped_arguments.push_back(WindowsEscapeArg2(arg));
}
ExitCode exit_code = this->LaunchProcess(java_bin, escaped_arguments);
