8298046: Fix hidden but significant trailing whitespace in properties files for serviceability code

[magicus]

According to the specification trailing whitespaces in the values of properties files are (somewhat surprisingly) actually significant.

We have multiple files in the JDK with trailing whitespaces in the values. For most of this files, this is likely incorrect and due to oversight, but in a few cases it might actually be intended (like "The value is: ").

After a discussion in the PR for JDK-8295729, the consensus was to replace valid trailing spaces with the corresponding unicode sequence, \u0020. (And of course remove non-wanted trailing spaces.)

Doing so has a dual benefit:

1. It makes it clear to everyone reading the code that there is a trailing space and it is intended

2. It will allow us to remove all actual trailing space characters, and turn on the corresponding check in jcheck to keep the properties files, just like all other source code files, free of trailing spaces.

Ultimately, the call of whether a trailing space is supposed to be there, or is a bug, lies with the respective component teams owning these files. Thus I have split up the set of properties files with trailing spaces in several groups, to match the JDK teams, and open a JBS issue for each of them. This issue is for code I believe belong with the serviceability team.

---

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-8298046: Fix hidden but significant trailing whitespace in properties files for serviceability code (Bug - P4)

Reviewers

• Chris Plummer (@plummercj - Reviewer)
• Kevin Walls (@kevinjwalls - Reviewer) ⚠️ Review applies to 1b418216

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 11490

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

Using diff file

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

Webrev

Link to Webrev Comment

[magicus]

A note to reviewers:

This PR has been automatically generated by converting all trailing spaces in key-value lines (ignoring empty lines and comments) in the property files into unicode sequences.

In a way, this is a no-op, converting one representation of the already existing space into another. But I think that most of these instances are likely to be bugs, and should thus be fixed.

I think the easiest way to fix those instances is to use the Github "suggestion" review mechanism:
If you see a value where the trailing space should be removed, press the blue + in the gutter as usual, and then in the comment box press the +/- icon (or press ctrl-G/cmd-G). Now you get a copy of the offending line, and can edit it to remove the trailing unicode sequence. I can then easily accept the suggestion into the PR.

But you can also come with review feedback like "remove trailing spaces for all files in directory XXX" and I'll fix that for you.

[bridgekeeper]

👋 Welcome back ihse! 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]

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

• jmx
• serviceability

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

• 03: Full - Incremental (5acd13a4)
• 02: Full (0c91a370)
• 01: Full - Incremental (60fda42b)
• 00: Full (1b418216)

[bridgekeeper]

@magicus This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@magicus This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[RogerRiggs]

It appears that the suggested changes could be include to resume this issue.

[magicus]

/open

[openjdk]

@magicus This pull request is now open

[kevinjwalls]

The suggestions from Chris look good, i.e. the trailing \u0020 were unintentional.

[openjdk]

@magicus this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout properties-eol-serviceability
git fetch https://git.openjdk.org/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[bridgekeeper]

@magicus This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@magicus This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[plummercj]

What was the reason for not moving forward with this PR?

[magicus]

What was the reason for not moving forward with this PR?

Me going on medical leave most of 2023... :-/

I intend to get this done now.

/open

[openjdk]

@magicus This pull request is now open

[openjdk]

@magicus 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:

8298046: Fix hidden but significant trailing whitespace in properties files for serviceability code

Reviewed-by: cjplummer, kevinw

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 9 new commits pushed to the master branch:

• 209d87a: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
• 7777eb5: 8321931: memory_swap_current_in_bytes reports 0 as "unlimited"
• 51853f7: 8324724: Add Stub routines for FP16 conversions on aarch64
• c3adc61: 8325199: (zipfs) jdk/nio/zipfs/TestPosix.java failed 6 sub-tests
• 1993652: 8324983: race in CompileBroker::possibly_add_compiler_threads
• 55c1446: 8321468: Remove StringUTF16::equals
• 19e9220: 8325169: Reduce String::indexOf overheads
• 89e6a02: 8325064: C2 SuperWord: refactor construct_bb
• d395ac2: 8321373: Build should use LC_ALL=C.UTF-8

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[magicus]

Several of the original problems has been resolved since this PR was opened, but some remain. I believe this is now ready for integration, but I'd like to get a re-review from @plummercj.

[magicus]

/integrate

[openjdk]

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

• 6d911f6: 8317299: safepoint scalarization doesn't keep track of the depth of the JVM state
• 542b0b6: 8324126: Error message for mistyping -XX:+Unlock...Options is not helpful
• 9ee9f28: 8325213: Flags introduced by configure script are not passed to ADLC build
• 729ae1d: 8325266: Enable this-escape javac warning in jdk.javadoc
• e0fd3f4: 8325081: Move '_soft_ref_policy' to 'CollectedHeap'
• f1f9398: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern
• ab3b941: 8325270: ProblemList two compiler/intrinsics/float16 tests that fail due to JDK-8324724
• f31957e: 8317636: Improve heap walking API tests to verify correctness of field indexes
• fd3042a: 8318566: Heap walking functions should not use FilteredFieldStream
• 209d87a: 8324960: Unsafe.allocateMemory documentation incorrect regarding zero return value
• ... and 8 more: https://git.openjdk.org/jdk/compare/51671c0b92ce9ee581bc850dff382b35a528b1cd...master

Your commit was automatically rebased without conflicts.

[openjdk]

@magicus Pushed as commit b02599d.

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