8357862: Java argument file is parsed unexpectedly with trailing comment

[sormuras]

Please review this fix to correctly parse tokens in an argument file that contains trailing comments without preceding whitespace characters before the '#' comment marker, the number sign.

The specifaction of the java Command has in section java Command-Line Argument Files:

Use the number sign `#` in the argument file to identify comments.
All characters following the `#` are ignored until the end of line.

There is not requirement specified to prepend a comment in a line with at least one argument with one or more preceding whitespace characters.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8357862: Java argument file is parsed unexpectedly with trailing comment (Bug - P4)

Reviewers

• Vicente Romero (@vicente-romero-oracle - Reviewer)

Contributors

• Stuart Marks <[email protected]>

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25589/head:pull/25589
$ git checkout pull/25589

Update a local copy of the PR:
$ git checkout pull/25589
$ git pull https://git.openjdk.org/jdk.git pull/25589/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25589

View PR using the GUI difftool:
$ git pr show -t 25589

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25589.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back cstein! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@sormuras This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8357862: Java argument file is parsed unexpectedly with trailing comment

Co-authored-by: Stuart Marks <[email protected]>
Reviewed-by: vromero

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 166 new commits pushed to the master branch:

• 4291582: 8357382: runtime/cds/appcds/aotClassLinking/BulkLoaderTest.java#aot fails with Xcomp and C1
• ef4cbec: 8358556: Assert when running with -XX:-UseLibmIntrinsic
• e9216ef: 8358734: Remove JavaTimeSupplementary resource bundles
• ... and 163 more: https://git.openjdk.org/jdk/compare/eb9badd8a4ea6dca834525fd49429e2ce771a76c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@sormuras The following labels will be automatically applied to this pull request:

• core-libs
• hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (bea8cc79)
• 00: Full (21afeb37)

[vicente-romero-oracle]

the fact that not including a space between the comment and the previous element created havoc seems to indicate that the scanner is pretty fragile and could break again in the future. We should probably file a follow up to make this code more reliable

[sormuras]

the fact that not including a space between the comment and the previous element created havoc seems to indicate that the scanner is pretty fragile and could break again in the future. We should probably file a follow up to make this code more reliable

Yes, seems reasonable. My initial solution was to try to make use of the shared CommandLine class, but the parsing of the launcher is too early in the game.

[vicente-romero-oracle]

lgtm

[sormuras]

/contributor add @stuart-marks

[openjdk]

@sormuras
Contributor Stuart Marks <[email protected]> successfully added.

[sormuras]

/integrate

[openjdk]

Going to push as commit 21b72de.
Since your change was applied there have been 238 commits pushed to the master branch:

• 51877f5: 8359207: Remove runtime/signal/TestSigusr2.java since it is always skipped
• c1deb9e: 8356556: AArch64: No need for acquire fence in safepoint poll in FFM
• f62f117: 8359701: G1: Move heap expansion time tracking of G1CollectedHeap:expand to call site
• ... and 235 more: https://git.openjdk.org/jdk/compare/eb9badd8a4ea6dca834525fd49429e2ce771a76c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@sormuras Pushed as commit 21b72de.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.