Re: RFR: 8320860: add-opens/add-exports require '=' in JAVA_TOOL_OPTIONS [v2]
On Mon, 4 Dec 2023 12:35:42 GMT, Alan Bateman wrote: >> Thanks for looking at this @AlanBateman and @sspitsyn . I was trying to >> refer to the JNI text in a casual way rather than duplicating the actual >> content from there. This was more a strong hint/suggestion to "go read the >> JNI spec for details". >> >> Happy to change 'form' to 'format'. > > I agree with not duplicating the entire sentence but what would you think > about replacing the text in the parenthesis with "Note the required = > between option and value". Only asking because "(not "option value")" isn't > as clear that we want the reader to see that you can't use a space here. Okay. I thought it very clear to say "use format X (not format Y)", but sure I can change it. - PR Review Comment: https://git.openjdk.org/jdk/pull/16896#discussion_r1414637979
Re: RFR: 8320860: add-opens/add-exports require '=' in JAVA_TOOL_OPTIONS [v2]
On Mon, 4 Dec 2023 04:43:46 GMT, David Holmes wrote: >> src/hotspot/share/prims/jvmti.xml line 746: >> >>> 744: JNI_CreateJavaVM (in the JNI Invocation API) will >>> prepend these options to the options supplied >>> 745: in its JavaVMInitArgs argument. Note that module >>> related options must be expressed in their >>> 746: "option=value" form (not "option value") for >>> JNI_CreateJavaVM to process them correctly. >> >> This looks okay. I'm just comparing it to the text that we put into the JNI >> spec: >> >> "The module related options ... as option strings using their "option=value" >> format instead of their "option value" format. (Note the required = between >> "option" and "value".)" >> >> It uses "format" instead of "form" and also, the bit I think works well, is >> to point out "required =" to force the reader to re-read the previous >> sentence and see what the difference is in the formats. > > Thanks for looking at this @AlanBateman and @sspitsyn . I was trying to > refer to the JNI text in a casual way rather than duplicating the actual > content from there. This was more a strong hint/suggestion to "go read the > JNI spec for details". > > Happy to change 'form' to 'format'. I agree with not duplicating the entire sentence but what would you think about replacing the text in the parenthesis with "Note the required = between option and value". Only asking because "(not "option value")" isn't as clear that we want the reader to see that you can't use a space here. - PR Review Comment: https://git.openjdk.org/jdk/pull/16896#discussion_r1413811389
Re: RFR: 8320860: add-opens/add-exports require '=' in JAVA_TOOL_OPTIONS [v2]
> Please review this simple clarification to the JVM TI spec regarding use of > `JAVA_TOOL_OPTIONS` in regards to module options and their format. > > I do not believe this clarification needs a CSR request. > > Thanks. David Holmes has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Change form -> format - Merge branch 'master' into 8320860-jvmti-spec - 8320860: add-opens/add-exports require '=' in JAVA_TOOL_OPTIONS - Changes: - all: https://git.openjdk.org/jdk/pull/16896/files - new: https://git.openjdk.org/jdk/pull/16896/files/7c8ca438..5417a109 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16896=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16896=00-01 Stats: 9803 lines in 335 files changed: 7601 ins; 1490 del; 712 mod Patch: https://git.openjdk.org/jdk/pull/16896.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16896/head:pull/16896 PR: https://git.openjdk.org/jdk/pull/16896
