Windows: add --incompatible_windows_escape_jvm_flags #7490
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add --incompatible_windows_escape_jvm_flags flag (default: false). This flag has no effect on platforms other than Windows. This flag affects how Bazel builds the launcher of java_binary and java_test targets. (The launcher is the .exe file that sets up the environment for the Java program and launches the JVM.) In particular, the flag controls whether or not Bazel will Bash-tokenize the jvm_flags that were declared in the BUILD file, and whether or not these jvm_flags are escaped by the launcher when it invokes the JVM. When the flag is enabled: - Bazel will Bash-tokenize java_binary.jvm_flags and java_test.jvm_flags (as documented by the Build Encyclopedia) before embedding them into the launcher. The build fails if an entry cannot be Bash-tokenized, e.g. if it has unterminated quotes. - The launcher will properly escape these flags when it runs the JVM as a subprocess (using launcher_util::WindowsEscapeArg2). - The result is that the jvm_flags declared in the BUILD file will arrive to the Java program as intended. However, due to the extra Bash-tokenization step, some ill-formed flags are no longer accepted, e.g. `jvm_flags=["-Dfoo='a"]` now results in a build error. When the flag is disabled: - Bazel does not Bash-tokenize the jvm_flags before embedding them in the launcher. This preserves quoting meant to be stripped away, and it also means Bazel won't check whether the argument is properly quoted. - The launcher escapes the flags with a Bash-like escaping logic (launcher_util::WindowsEscapeArg) which cannot properly quote and escape everything. - The result: the jvm_flags declared in the BUILD file might get messed up as they are passed to the JVM, or the launcher may not even be able to run the JVM. However, due to the lack of Bash-tokenization, Bazel propagates some flags to the Java binary that it would no longer accept if the new `--incompatible_windows_escape_jvm_flags` were enabled, e.g. `jvm_flags=["'a"]` is fine. Incompatible flag: bazelbuild#7486 Related: bazelbuild#7072 RELNOTES[NEW]: Added --incompatible_windows_escape_jvm_flags flag: enables correct java_binary.jvm_flags and java_test.jvm_flags tokenization and escaping on Windows. (No-op on other platforms.) Change-Id: I531cc63cdfeccbe4b6d48876cb82870c1726a723
|
/CC @cushon |
iirina
approved these changes
Feb 21, 2019
lfpino
reviewed
Feb 21, 2019
| } | ||
|
|
||
| function assert_jvm_flags() { | ||
| local -r enable_windows_escaping=$1 |
Change-Id: I8ff717731053d0291681afc94f7ee6d918a65fc8
|
@lfpino : PTAL |
lfpino
suggested changes
Feb 21, 2019
| # ---------------------------------------------------------------------- | ||
|
|
||
| function test_no_windows_jvm_flags_escaping() { | ||
| local -r pkg="${FUNCNAME[0]}" |
There was a problem hiding this comment.
nit: Unless it's necessary, use concrete values for pkg, it removes one layer of indirection when looking at the lines of code using it.
There was a problem hiding this comment.
I'd like to keep it like this. This generates a unique package name for every test.
In case the test cleanup logic were flawed (which would certainly be a bug that should be fixed) such that it didn't fully clean up after tests, we'd still run every test with its own package, reducing the chance that tests overwrite each others files.
|
All done. |
philwo
added
the
team-OSS
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add --incompatible_windows_escape_jvm_flags flag
(default: false). This flag has no effect on
platforms other than Windows.
This flag affects how Bazel builds the launcher of
java_binary and java_test targets. (The launcher
is the .exe file that sets up the environment for
the Java program and launches the JVM.)
In particular, the flag controls whether or not
Bazel will Bash-tokenize the jvm_flags that were
declared in the BUILD file, and whether or not
these jvm_flags are escaped by the launcher when
it invokes the JVM.
When the flag is enabled:
Bazel will Bash-tokenize java_binary.jvm_flags
and java_test.jvm_flags (as documented by the
Build Encyclopedia) before embedding them into
the launcher. The build fails if an entry cannot
be Bash-tokenized, e.g. if it has unterminated
quotes.
The launcher will properly escape these flags
when it runs the JVM as a subprocess (using
launcher_util::WindowsEscapeArg2).
Result: the jvm_flags declared in the BUILD file
will arrive to the Java program as intended.
However, due to the extra Bash-tokenization
step, some ill-formed flags are no longer
accepted, e.g.
jvm_flags=["-Dfoo='a"]nowresults in a build error.
When the flag is disabled:
Bazel does not Bash-tokenize the jvm_flags
before embedding them in the launcher. This
preserves quoting meant to be stripped away, and
it also means Bazel won't check whether the
argument is properly quoted.
The launcher escapes the flags with a Bash-like
escaping logic (launcher_util::WindowsEscapeArg)
which cannot properly quote and escape
everything.
Result: the jvm_flags declared in the BUILD file
might get messed up as they are passed to the
JVM, or the launcher may not even be able to run
the JVM. However, due to the lack of
Bash-tokenization, Bazel propagates some flags
to the Java binary that it would no longer
accept if the new
--incompatible_windows_escape_jvm_flagswereenabled, e.g.
jvm_flags=["'a"]is fine.Incompatible flag: #7486
Related: #7072
RELNOTES[NEW]: Added --incompatible_windows_escape_jvm_flags flag: enables correct java_binary.jvm_flags and java_test.jvm_flags tokenization and escaping on Windows. (No-op on other platforms.)
Change-Id: I531cc63cdfeccbe4b6d48876cb82870c1726a723