8320919: Clarify Locale related system properties #17065

naotoj · 2023-12-11T18:54:25Z

This is a doc change to clarify what the Default Locale is, and how it is established during the system startup using the system properties. Those locale-related system properties have existed since the early days of Java, but have never been publicly documented before. It is also the intention of this PR to clarify those system properties and how they are overridden. A corresponding CSR has been drafted.

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
Change requires CSR request JDK-8321055 to be approved

Issues

JDK-8320919: Clarify Locale related system properties (Bug - P4)
JDK-8321055: Clarify Locale related system properties (CSR)

Reviewers

Stuart Marks (@stuart-marks - Reviewer) ⚠️ Review applies to 8baa56a9
Roger Riggs (@RogerRiggs - Reviewer) ⚠️ Review applies to 60b891a2

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 17065

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

Using diff file

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

Webrev

Link to Webrev Comment

bridgekeeper · 2023-12-11T18:56:07Z

👋 Welcome back naoto! 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 · 2023-12-11T18:58:09Z

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

core-libs
i18n

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 · 2023-12-11T19:02:01Z

Webrevs

justin-curtis-lu

LGTM; left some minor wording comments.

src/java.base/share/classes/java/util/Locale.java

justin-curtis-lu · 2023-12-11T22:42:56Z

src/java.base/share/classes/java/util/Locale.java

+ * of the Java runtime and established in the following three phases:
+ * <ol>
+ * <li>The locale-related system properties listed below are established from the
+ * host environment. Some system properties (except for {@code user.language}) may


IIUC, should it be all system properties, not some if we are defining the single exception?

The host-dependent part of the code always generates at least the language, thus it was excluded.

src/java.base/share/classes/java/util/Locale.java

justin-curtis-lu · 2023-12-11T22:45:25Z

src/java.base/share/classes/java/util/Locale.java

+ * at startup time. If the overriding value of the {@code user.extensions} property
+ * is unparsable, it is ignored. The overriding values of other properties are not
+ * checked for syntax or validity and are used directly in the default Locale.
+ * (Typically, system property values can be provided using the {@code -D} command-line


Do these last two sentences need to be enclosed in a parentheses? It seems pretty important as it is the main way to override the properties via command-line.

The -D command-line option is not a part of the Java SE specification but an allowed behavior, so it may not be a normative description here.

Right, I suggested putting that in parentheses. Historically we haven't been very formal about distinguishing between normative (Java SE) specifications and informative text that talks about implementations. In this case I felt that enclosing the text in parentheses and also adding hedging words ("typically") made it clear that this text isn't normative.

FWIW, for the FFM API, we describe the --enable-native-access command line flag in a separate @implNote:

jdk/src/java.base/share/classes/java/lang/foreign/package-info.java

Lines 164 to 171 in cf94854

* @implNote

* In the reference implementation, access to restricted methods can be granted to

* specific modules using the command line option {@code --enable-native-access=M1,M2, ... Mn},

* where {@code M1}, {@code M2}, {@code ... Mn} are module names (for the unnamed module,

* the special value {@code ALL-UNNAMED} can be used). If this option is specified,

* access to restricted methods are only granted to the modules listed by that option.

* If this option is not specified, access to restricted methods is enabled for all

* modules, but access to restricted methods will result in runtime warnings.

Thanks for the suggestion, Jorn. I tried embedding @implNote within the ordered list, but looks like it is not allowed. It would be nice if we could use it inside a list, but since it is about overriding the property values, I would rather not have the description apart from that list item.

justin-curtis-lu · 2023-12-11T22:49:13Z

src/java.base/share/classes/java/util/Locale.java

+ * {@code -Duser.language=foobarbaz} results in a default Locale whose language is
+ * "foobarbaz".)
+ * </li>
+ * <li>The default {@code Locale} instance is constructed from these system


Might read better as "... is constructed from the values of these system properties."

Thanks. modified

stuart-marks · 2023-12-13T05:31:44Z

src/java.base/share/classes/java/util/Locale.java

+ * Locale with {@link #getDefault()} and change it with {@link #setDefault(Locale)}.
+ * If the default Locale is changed with {@link #setDefault(Locale)}, the corresponding
+ * system properties are not altered. It is not recommended that applications read
+ * these system properties and parse/interpret them as their values may be out of sync.


Small wordsmithing: I would say "out of date" instead.

stuart-marks · 2023-12-13T05:32:43Z

src/java.base/share/classes/java/util/Locale.java

+ * These category specific default Locales can be queried by {@link #getDefault(Category)},
+ * and set by {@link #setDefault(Category, Locale)}. Construction of these category
+ * specific default Locales are determined by the corresponding system properties,
+ * which consist of the base system properties as listed above, appended by either


"appended" => "suffixed"

stuart-marks

Looks good, with a couple small wording changes.

naotoj · 2023-12-13T18:01:39Z

Thanks, Stuart. Modified the wordings as suggested.

RogerRiggs · 2023-12-13T18:33:13Z

src/java.base/share/classes/java/util/Locale.java

+ * Locale with {@link #getDefault()} and change it with {@link #setDefault(Locale)}.
+ * If the default Locale is changed with {@link #setDefault(Locale)}, the corresponding
+ * system properties are not altered. It is not recommended that applications read
+ * these system properties and parse/interpret them as their values may be out of date.


"parse/interpret" -> "parse or interpret".

Thanks, Roger. Modified.

RogerRiggs · 2023-12-13T18:34:24Z

src/java.base/share/classes/java/util/Locale.java

+ * option of a launcher. For example, specifying {@code -Duser.extensions=foobarbaz}
+ * results in a default Locale with no extensions, while specifying
+ * {@code -Duser.language=foobarbaz} results in a default Locale whose language is
+ * "foobarbaz".)


The parenthesized sentence is so long that its not effective in making it seem like an option.
The "Typical," is sufficient to indicate it is not part of the spec. Drop the parens.

RogerRiggs · 2023-12-14T21:27:06Z

src/java.base/share/classes/java/util/Locale.java

@@ -258,6 +259,74 @@
 * locales. For example, {@code Locale.US} is the {@code Locale} object
 * for the United States.
 *
+ * <h3><a id="default_locale">Default Locale</a></h3>
+ *
+ * <p>The default Locale is mainly provided for any locale-sensitive methods if no


The word "mainly" can be omitted, it doesn't add any clarity.

bchristi-git · 2023-12-15T20:39:47Z

src/java.base/share/classes/java/util/Locale.java

+ * {@code language} part of the default Locale for {@link Locale.Category#DISPLAY}
+ * category. In the absence of category specific system properties, the "category-less"


"...part of the default Locale for the Locale.Category#DISPLAY category."
?

Thanks, Brent. Inserted the

Co-authored-by: Justin Lu <Justin.lu@oracle.com>

openjdk · 2023-12-18T17:42:41Z

@naotoj Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

openjdk · 2024-01-09T22:20:07Z

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

8320919: Clarify Locale related system properties

Reviewed-by: smarks, rriggs

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

aba1933: 8323210: Update the usage of cmsFLAGS_COPY_ALPHA
f3be138: 8322809: SystemModulesMap::classNames and moduleNames arrays do not match the order
bc05893: 8323318: Remove unused Space::is_free_block
dd8ae61: 8322237: Heap dump contains duplicate thread records for mounted virtual threads
ee98d26: 8323066: gc/g1/TestSkipRebuildRemsetPhase.java fails with 'Skipping Remembered Set Rebuild.' missing
886386c: 8322890: Directly return in OldPLABSizeConstraintFunc
438ab7c: 8323284: Remove unused FilteringClosure declaration
52c7ff1: 8322330: JavadocHelperTest.java OOMEs with Parallel GC and ZGC
ff499ef: 8233443: G1 DetailedUsage class names overly generic for global namespace
37a6172: 8322936: Update blessed-modifier-order.sh for default, sealed, and non-sealed
... and 150 more: https://git.openjdk.org/jdk/compare/1fde8b868a0e40fb79de505106ef07e3dccbd1de...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.

naotoj · 2024-01-09T23:10:42Z

/integrate

openjdk · 2024-01-09T23:11:23Z

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

a5071e0: 8322817: RISC-V: Eliminate -Wparentheses warnings in riscv code
28d8149: 8323115: x86-32: Incorrect predicates for cmov instruct transforms with UseSSE
e9f7db3: 8322880: Eliminate -Wparentheses warnings in arm32 code
aba1933: 8323210: Update the usage of cmsFLAGS_COPY_ALPHA
f3be138: 8322809: SystemModulesMap::classNames and moduleNames arrays do not match the order
bc05893: 8323318: Remove unused Space::is_free_block
dd8ae61: 8322237: Heap dump contains duplicate thread records for mounted virtual threads
ee98d26: 8323066: gc/g1/TestSkipRebuildRemsetPhase.java fails with 'Skipping Remembered Set Rebuild.' missing
886386c: 8322890: Directly return in OldPLABSizeConstraintFunc
438ab7c: 8323284: Remove unused FilteringClosure declaration
... and 153 more: https://git.openjdk.org/jdk/compare/1fde8b868a0e40fb79de505106ef07e3dccbd1de...master

Your commit was automatically rebased without conflicts.

openjdk · 2024-01-09T23:11:30Z

@naotoj Pushed as commit 376051a.

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

openjdk bot added csr Pull request needs approved CSR before integration rfr Pull request is ready for review labels Dec 11, 2023

openjdk bot added core-libs core-libs-dev@openjdk.org i18n i18n-dev@openjdk.org labels Dec 11, 2023

justin-curtis-lu reviewed Dec 11, 2023

View reviewed changes

stuart-marks reviewed Dec 13, 2023

View reviewed changes

stuart-marks approved these changes Dec 13, 2023

View reviewed changes

RogerRiggs approved these changes Dec 13, 2023

View reviewed changes

RogerRiggs reviewed Dec 14, 2023

View reviewed changes

bchristi-git reviewed Dec 15, 2023

View reviewed changes

naotoj and others added 15 commits December 18, 2023 09:41

initial commt

aea5104

2nd draft

1f17437

3rd draft

66c8e39

4th draft

1b3eb03

5th draft

85eb472

6th draft

407eee9

Update src/java.base/share/classes/java/util/Locale.java

14ced1b

Co-authored-by: Justin Lu <Justin.lu@oracle.com>

Update src/java.base/share/classes/java/util/Locale.java

4cc30ab

Co-authored-by: Justin Lu <Justin.lu@oracle.com>

Update src/java.base/share/classes/java/util/Locale.java

8452a72

Co-authored-by: Justin Lu <Justin.lu@oracle.com>

Update src/java.base/share/classes/java/util/Locale.java

1e36084

Co-authored-by: Justin Lu <Justin.lu@oracle.com>

Review comments

b85f715

Reflects review comments

4309dcd

Reflects review comments

a3ea5c9

Reflects review comments

94c2e97

Reflects review comments

4a76c89

naotoj force-pushed the JDK-8320919-Locale-system-properties branch from e67585e to 4a76c89 Compare December 18, 2023 17:41

openjdk bot mentioned this pull request Jan 9, 2024

8320788: The system properties page is missing some properties #17317

Closed

4 tasks

openjdk bot added ready Pull request is ready to be integrated and removed csr Pull request needs approved CSR before integration labels Jan 9, 2024

openjdk bot added the integrated Pull request has been integrated label Jan 9, 2024

openjdk bot closed this Jan 9, 2024

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 9, 2024

	* @implNote
	* In the reference implementation, access to restricted methods can be granted to
	* specific modules using the command line option {@code --enable-native-access=M1,M2, ... Mn},
	* where {@code M1}, {@code M2}, {@code ... Mn} are module names (for the unnamed module,
	* the special value {@code ALL-UNNAMED} can be used). If this option is specified,
	* access to restricted methods are only granted to the modules listed by that option.
	* If this option is not specified, access to restricted methods is enabled for all
	* modules, but access to restricted methods will result in runtime warnings.

		* {@code language} part of the default Locale for {@link Locale.Category#DISPLAY}
		* category. In the absence of category specific system properties, the "category-less"

8320919: Clarify Locale related system properties #17065

8320919: Clarify Locale related system properties #17065

Uh oh!

Conversation

naotoj commented Dec 11, 2023 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issues

Reviewers

Reviewing

Webrev

Uh oh!

bridgekeeper bot commented Dec 11, 2023

Uh oh!

openjdk bot commented Dec 11, 2023

Uh oh!

mlbridge bot commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

justin-curtis-lu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuart-marks left a comment

Choose a reason for hiding this comment

Uh oh!

naotoj commented Dec 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Dec 18, 2023

Uh oh!

openjdk bot commented Jan 9, 2024

Uh oh!

naotoj commented Jan 9, 2024

Uh oh!

openjdk bot commented Jan 9, 2024

Uh oh!

openjdk bot commented Jan 9, 2024

Uh oh!

Uh oh!

naotoj commented Dec 11, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 11, 2023 •

edited

Loading