8357862: Java argument file is parsed unexpectedly with trailing comment #25589
Conversation
|
👋 Welcome back cstein! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
| if (pctx->parts->size == 0) { | ||
| token = clone_substring(anchor, nextc - anchor); | ||
| } else { | ||
| JLI_List_addSubstring(pctx->parts, anchor, nextc - anchor); | ||
| token = JLI_List_combine(pctx->parts); | ||
| JLI_List_free(pctx->parts); | ||
| pctx->parts = JLI_List_new(4); | ||
| } | ||
| pctx->cptr = nextc + 1; | ||
| return token; |
There was a problem hiding this comment.
This code duplicates a large part of the block of the previous case. Perhaps an internal helper method could be extracted?
There was a problem hiding this comment.
right I agree that if there is common code we should extract it in a helper method
There was a problem hiding this comment.
Or the handling of # can be merged with the previous case ... somehow.
|
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. |
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
javaCommand has in section java Command-Line Argument Files: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
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25589/head:pull/25589$ git checkout pull/25589Update a local copy of the PR:
$ git checkout pull/25589$ git pull https://git.openjdk.org/jdk.git pull/25589/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25589View PR using the GUI difftool:
$ git pr show -t 25589Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25589.diff
Using Webrev
Link to Webrev Comment