Re: RFR: 8338411: Implement JEP 486: Permanently Disable the Security Manager
On Mon, 14 Oct 2024 13:52:24 GMT, Sean Mullan wrote: > This is the implementation of JEP 486: Permanently Disable the Security > Manager. See [JEP 486](https://openjdk.org/jeps/486) for more details. The > [CSR](https://bugs.openjdk.org/browse/JDK-8338412) describes in detail the > main changes in the JEP and also includes an apidiff of the specification > changes. > > NOTE: the majority (~95%) of the changes in this PR are test updates > (removal/modifications) and API specification changes, the latter mostly to > remove `@throws SecurityException`. The remaining changes are primarily the > removal of the `SecurityManager`, `Policy`, `AccessController` and other > Security Manager API implementations. There is very little new code. > > The code changes can be broken down into roughly the following categories: > > 1. Degrading the behavior of Security Manager APIs to either throw Exceptions > by default or provide an execution environment that disallows access to all > resources by default. > 2. Changing hundreds of methods and constructors to no longer throw a > `SecurityException` if a Security Manager was enabled. They will operate as > they did in JDK 23 with no Security Manager enabled. > 3. Changing the `java` command to exit with a fatal error if a Security > Manager is enabled. > 4. Removing the hotspot native code for the privileged stack walk and the > inherited access control context. The remaining hotspot code and tests > related to the Security Manager will be removed immediately after integration > - see [JDK-8341916](https://bugs.openjdk.org/browse/JDK-8341916). > 5. Removing or modifying hundreds of tests. Many tests that tested Security > Manager behavior are no longer relevant and thus have been removed or > modified. > > There are a handful of Security Manager related tests that are failing and > are at the end of the `test/jdk/ProblemList.txt`, > `test/langtools/ProblemList.txt` and `test/hotspot/jtreg/ProblemList.txt` > files - these will be removed or separate bugs will be filed before > integrating this PR. > > Inside the JDK, we have retained calls to > `SecurityManager::getSecurityManager` and `AccessController::doPrivileged` > for now, as these methods have been degraded to behave the same as they did > in JDK 23 with no Security Manager enabled. After we integrate this JEP, > those calls will be removed in each area (client-libs, core-libs, security, > etc). > > I don't expect each reviewer to review all the code changes in this JEP. > Rather, I advise that you only focus on the changes for the area > (client-libs, core-libs, net, security, etc) that you are most f... This is a great work! I looked through the following areas that relate to i18n/charset/console/javatime, and they all look good to me. src/java.base/share/classes/java/util/Locale.java src/java.base/share/classes/java/util/ResourceBundle.java src/java.base/share/classes/java/util/TimeZone.java src/java.base/share/classes/java/util/spi/LocaleServiceProvider.java test/jdk/java/io/Console/ test/jdk/java/nio/charset/spi/ test/jdk/java/time/nontestng/java/time/chrono/ test/jdk/java/util/PluggableLocale/ test/jdk/java/util/ResourceBundle/ test/jdk/java/util/TimeZone/ test/jdk/java/util/spi/ResourceBundleControlProvider/ test/jdk/sun/nio/cs/ test/jdk/sun/util/locale/provider/ - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/21498#pullrequestreview-2370292817
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v2]
On Tue, 20 Aug 2024 09:07:54 GMT, Pavel Rappo wrote: >> Justin Lu has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Replace InputStreamReader with BufferedReader > > src/jdk.jartool/share/classes/sun/tools/jar/resources/jar_pt_BR.properties > line 95: > >> 93: >> 94: main.usage.summary=Uso: jar [OPTION...] [ [--release VERSION] [-C dir] >> files] ... >> 95: main.usage.summary.try=Tente `jar --ajuda' para obter mais informações. > > I was looking for something unrelated in properties files, and found this. It > is surprising to see an option name being localised; it must be a bug. Good catch, Pavel. It is indeed a bug. This type of overtranslation l10n bug happens all the time, and hard to catch. - PR Review Comment: https://git.openjdk.org/jdk/pull/15694#discussion_r1723520963
Re: [jdk23] RFR: 8333827: JDK 23 RDP1 L10n resource files update
On Thu, 13 Jun 2024 21:38:48 GMT, Damon Nguyen wrote: > Hi all, > > This pull request contains a backport of commit > [c4702ca8](https://github.com/openjdk/jdk/commit/c4702ca8c026e2d265aca0126fd3fb7bc5bb392a) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Damon Nguyen on 13 Jun 2024 and > was reviewed by Alisen Chung, Justin Lu, Naoto Sato, Joe Wang, Pavel Rappo, > Chris Plummer, Alexey Semenyuk and Phil Race. > > Thanks! Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19713#pullrequestreview-2117085705
Re: RFR: 8333827: JDK 23 RDP1 L10n resource files update [v3]
On Mon, 10 Jun 2024 20:46:46 GMT, Naoto Sato wrote: >> Damon Nguyen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Revert updated translation > > Did not look at each for translation accuracy (did some for Japanese), but > looks good overall. > It's great to see diffs in native languages, rather than those cryptic > `\u` notation! > An interesting note after looking at Phil's comment on the > serviceui.properties change is that there is no Japanese update for that > file. The translations on > [here](https://github.com/openjdk/jdk/commit/c7d2a5c1c4e86955100f4c40170dc25222abd07f#diff-38c26a463e174224c6b9fc5007f3058482e533dc0f60f3275e21f9f2fc9acf8e) > from the output bins update look good to me but maybe @naotoj should double > check in case? I can confirm that English "Out&put trays:" to "出力トレイ(&P):" is OK - PR Comment: https://git.openjdk.org/jdk/pull/19609#issuecomment-2161765683
Re: RFR: 8333827: JDK 23 RDP1 L10n resource files update [v2]
On Mon, 10 Jun 2024 19:45:14 GMT, Damon Nguyen wrote:
>> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/msg/XMLMessages_de.properties
>> line 278:
>>
>>> 276: UndeclaredElementInContentSpec = Contentmodell des Elements
>>> "{0}" verweist auf das nicht deklarierte Element "{1}".
>>> 277: UniqueNotationName = Deklaration für die Notation "{0}" ist
>>> nicht eindeutig. Ein jeweiliger Name darf nicht in mehreren
>>> Notationsdeklarationen deklariert werden.
>>> 278: ENTITYFailedInitializeGrammar = ENTITYDatatype-Validator:
>>> Nicht erfolgreich. Initialisierungsmethode muss mit einer gültigen
>>> Grammatikreferenz aufgerufen werden. \t
>>
>> It looks like the _zh_CN_ file should also have the `\t` removed, but such
>> changes are done by the translation tool. I think in this case, we should
>> manually remove it, and then file a bug against the translation tool.
>
> Yeah, I believe you're right. Noted as something else to file against the
> translation tool.
Just out of curiosity, do we know why `\t` was attached in the first place? I
don't think it is with the original English properties.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19609#discussion_r1635231459
Re: RFR: 8333827: JDK 23 RDP1 L10n resource files update
On Fri, 7 Jun 2024 22:46:44 GMT, Damon Nguyen wrote: > This issue is responsible for updating the translations of all the > localize(able) resources in the JDK. Primarily, the changes between JDK 22 > RDP 1 and the integration of the JDK 23 RDP 1 L10n drop will be translated. > > The translation tool adjusted some definitions, which causes some changes in > localized files where the source file had no changes. This causes some words > being reverted from localized languages to English, and some had its > definitions changed. > > Alternatively, the diffs are viewable here and was generated using Jonathan > Gibbons' diff tool for l10n: > https://cr.openjdk.org/~dnguyen/output2/ Did not look at each for translation accuracy (did some for Japanese), but looks good overall. It's great to see diffs in native languages, rather than those cryptic `\u` notation! - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19609#pullrequestreview-2108722631
Re: RFR: 8325950: Make sure all files in the JDK pass jcheck [v2]
On Fri, 16 Feb 2024 12:43:25 GMT, Magnus Ihse Bursie wrote: >> Since jcheck only checks file in a commit, there is a possibility of us >> getting files in the repository that would not be accepted by jcheck. This >> can happen when extending the set of files checked by jcheck, or if jcheck >> changes how it checks files (perhaps due to bug fixes). >> >> I have now run jcheck over the entire code base, and fixed a handful of >> issues. With these changes, jcheck accept the entire code base. > > Magnus Ihse Bursie has updated the pull request incrementally with one > additional commit since the last revision: > > Replace spaces with \t in test properties file LGTM - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17871#pullrequestreview-1885586969
Re: RFR: 8325950: Make sure all files in the JDK pass jcheck
On Thu, 15 Feb 2024 17:28:52 GMT, Andy Goryachev wrote: >> Please do not replace those tabs with spaces as they are intentional for >> testing the runtime to safely ignore them. I suggest replacing them with >> Unicode escapes (`\u000b`) > > `\u000b` is VT (vertical tab) > `\u0009` or `\t` perhaps? Right. `\t` is better to avoid such a mistake. - PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1491403426
Re: RFR: 8325950: Make sure all files in the JDK pass jcheck
On Thu, 15 Feb 2024 15:48:38 GMT, Magnus Ihse Bursie wrote: >> This looks weird indeed. Luckily it's just test code. Did you run the test >> after this change? > > All the java/util/Currency tests pass. I also searched the code for "ZZ" but > could not find any references in the test. Please do not replace those tabs with spaces as they are intentional for testing the runtime to safely ignore them. I suggest replacing them with Unicode escapes (`\u000b`) - PR Review Comment: https://git.openjdk.org/jdk/pull/17871#discussion_r1491352359
Re: RFR: 8325558: Add jcheck whitespace checking for properties files
On Fri, 9 Feb 2024 13:35:55 GMT, Magnus Ihse Bursie wrote: > This is an attempt to finally implement the idea brought forward in > JDK-8295729: Properties files is essentially source code. It should have the > same whitespace checks as all other source code, so we don't get spurious > trailing whitespace changes or leading tabs instead of spaces. > > With Skara jcheck, it is possible to increase the coverage of the whitespace > checks. > > However, this turned out to be problematic, since trailing whitespace is > significant in properties files. That issue has mostly been sorted out in a > series of PRs, and this patch will finish the job with the few remaining > files, and actually enable the check in jcheck. > @naotoj Thanks! Would you care to also submit a review? My bad. I thought I approved this PR. - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17789#pullrequestreview-1875811619
Re: RFR: 8325558: Add jcheck whitespace checking for properties files
On Fri, 9 Feb 2024 13:35:55 GMT, Magnus Ihse Bursie wrote: > This is an attempt to finally implement the idea brought forward in > JDK-8295729: Properties files is essentially source code. It should have the > same whitespace checks as all other source code, so we don't get spurious > trailing whitespace changes or leading tabs instead of spaces. > > With Skara jcheck, it is possible to increase the coverage of the whitespace > checks. > > However, this turned out to be problematic, since trailing whitespace is > significant in properties files. That issue has mostly been sorted out in a > series of PRs, and this patch will finish the job with the few remaining > files, and actually enable the check in jcheck. Skimmed through the changes and all look good to me. Good to have `jcheck` detect those unneeded trailing spaces. - PR Review: https://git.openjdk.org/jdk/pull/17789#pullrequestreview-1872951198
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v2]
On Wed, 13 Sep 2023 17:38:28 GMT, Justin Lu wrote: >> JDK .properties files still use ISO-8859-1 encoding with escape sequences. >> It would improve readability to see the native characters instead of escape >> sequences (especially for the L10n process). The majority of files changed >> are localized resource files. >> >> This change converts the Unicode escape sequences in the JDK .properties >> files (both in src and test) to UTF-8 native characters. Additionally, the >> build logic is adjusted to read the .properties files in UTF-8 while >> generating the ListResourceBundle files. >> >> The only escape sequence not converted was `\u0020` as this is used to >> denote intentional trailing white space. (E.g. `key=This is the >> value:\u0020`) >> >> The conversion was done using native2ascii with options `-reverse -encoding >> UTF-8`. >> >> If this PR is integrated, the IDE default encoding for .properties files >> need to be updated to UTF-8. (IntelliJ IDEA locks .properties files as >> ISO-8859-1 unless manually changed). > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Replace InputStreamReader with BufferedReader Looks good to me, although I did not look at each l10n file, but sampled some. Thanks for tackling this conversion. - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15694#pullrequestreview-1625154951
Re: [jdk21] RFR: 8314062: ProblemList jdk/jfr/tool/TestView.java on macosx-x64
On Wed, 9 Aug 2023 19:58:25 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList jdk/jfr/tool/TestView.java on macosx-x64. Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/171#pullrequestreview-1570469956
Re: RFR: 8309632: JDK 21 RDP1 L10n resource files update [v2]
On Fri, 16 Jun 2023 04:39:42 GMT, Justin Lu wrote: >> Please review this PR which updates the JDK's localized resources since the >> previous L10n translation drop (1/26). >> >> To help with reviewing the changes, @jonathan-gibbons created a tool which >> displays the localized changes next to the original file's changes in UTF-8 >> native. Those files can be viewed >> [here](https://cr.openjdk.org/~jlu/output/) >> >> For example, >> >> > src="https://github.com/openjdk/jdk21/assets/67398801/e68a48a8-916e-4860-afb0-526924c6a3d3";> >> >> >> Please note that the HTML files only apply to .properties, and not .java >> resources. > > Justin Lu has updated the pull request incrementally with five additional > commits since the last revision: > > - Review: correct previous commit, remove 标 as well > - Review: Remove 本机 from 'javac.opt.Xlint.desc.output-file-clash' for zh_CN > - Review: Use old 'err.crash' zh_CN value > - Review: use Naoto's recommended ja values > - Revert 'main.help.opt.any.file' value for _de Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk21/pull/11#pullrequestreview-1484380340
Re: RFR: 8309632: JDK 21 RDP1 L10n resource files update [v2]
On Fri, 16 Jun 2023 15:06:30 GMT, Jonathan Gibbons wrote: >> Left some comments on the translations mainly in Japanese. It is now very >> easy to look at the l10n changes in the generated HTML. One small comment to >> the tool is that it would be nice if the order in HTML (alphabetically >> sorted currently) aligns with the order in the actual resource file. It is a >> great improvement anyways. > >> Left some comments on the translations mainly in Japanese. It is now very >> easy to look at the l10n changes in the generated HTML. One small comment to >> the tool is that it would be nice if the order in HTML (alphabetically >> sorted currently) aligns with the order in the actual resource file. It is a >> great improvement anyways. > > 1. It is not easy to recover the order in the actual resource file: the > contents are read into a `Properties` object which uses a `Hashtable` to > store the data. > 2. There is no guarantee that the order is the same in all the resource > files, so we would have to choose one, presumably the default one, to > determine the presentation order. > 3. The iteration order would be undefined when resources are removed from one > or more resource files Makes sense. Thanks for the explanation @jonathan-gibbons - PR Comment: https://git.openjdk.org/jdk/pull/14430#issuecomment-1594999086
Re: RFR: 8309632: JDK 21 RDP1 L10n resource files update [v4]
On Tue, 13 Jun 2023 22:09:40 GMT, Justin Lu wrote: >> Please review this PR which updates the JDK's localized resources since the >> previous L10n translation drop (1/26). >> >> To help with reviewing the changes, @jonathan-gibbons created a tool which >> displays the localized changes next to the original file's changes in UTF-8 >> native. Those files can be viewed >> [here](https://cr.openjdk.org/~jlu/output/) >> >> For example, >> >> > src="https://github.com/openjdk/jdk21/assets/67398801/e68a48a8-916e-4860-afb0-526924c6a3d3";> >> >> >> Please note that the HTML files only apply to .properties, and not .java >> resources. > > Justin Lu has updated the pull request incrementally with three additional > commits since the last revision: > > - Review: correct previous commit, remove 标 as well > - Review: Remove 本机 from 'javac.opt.Xlint.desc.output-file-clash' for zh_CN > - Review: Use old 'err.crash' zh_CN value Looks good, Justin - Marked as reviewed by naoto (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14430#pullrequestreview-1478236336
Re: RFR: 8309632: JDK 21 RDP1 L10n resource files update [v2]
On Mon, 12 Jun 2023 22:32:14 GMT, Justin Lu wrote:
>> Please review this PR which updates the JDK's localized resources since the
>> previous L10n translation drop (1/26).
>>
>> To help with reviewing the changes, @jonathan-gibbons created a tool which
>> displays the localized changes next to the original file's changes in UTF-8
>> native. Those files can be viewed
>> [here](https://cr.openjdk.org/~jlu/output/)
>>
>> For example,
>>
>> > src="https://github.com/openjdk/jdk21/assets/67398801/e68a48a8-916e-4860-afb0-526924c6a3d3";>
>>
>>
>> Please note that the HTML files only apply to .properties, and not .java
>> resources.
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Revert 'main.help.opt.any.file' value for _de
Left some suggestions. Should be looked at by translators later.
src/java.desktop/macosx/classes/com/apple/laf/resources/aqua_ja.properties line
63:
> 61:
> FileChooser.newFolderExistsError.textAndMnemonic=\u305D\u306E\u540D\u524D\u306F\u3059\u3067\u306B\u4F7F\u7528\u3055\u308C\u3066\u3044\u307E\u3059
> 62: FileChooser.chooseButton.textAndMnemonic=\u9078\u629E
> 63:
> FileChooser.chooseButtonToolTip.textAndMnemonic=\u9078\u629E\u3057\u305F\u30D5\u30A1\u30A4\u30EB\u3092\u8FFD\u52A0\u3057\u307E\u3059
Suggestion:
FileChooser.chooseButtonToolTip.textAndMnemonic=\u9078\u629e\u3055\u308c\u3066\u3044\u308b\u30d5\u30a1\u30a4\u30eb\u3092\u9078\u629e\u3057\u307e\u3059
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 502:
> 500:
> compiler.warn.forward.ref=\u521D\u671F\u5316\u3055\u308C\u308B\u524D\u306E\u5909\u6570''{0}''\u3092\u53C2\u7167\u3057\u3088\u3046\u3068\u3057\u307E\u3057\u305F
> 501:
> 502:
> compiler.warn.possible.this.escape=\u30B5\u30D6\u30AF\u30E9\u30B9\u524D\u306B\u6307\u5B9A\u53EF\u80FD\u306A''this''\u30A8\u30B9\u30B1\u30FC\u30D7\u306F\u5B8C\u5168\u306B\u521D\u671F\u5316\u3055\u308C\u307E\u3059
Suggestion:
compiler.warn.possible.this.escape=\u30b5\u30d6\u30af\u30e9\u30b9\u304c\u521d\u671f\u5316\u3055\u308c\u308b\u524d\u306e''this''\u30a8\u30b9\u30b1\u30fc\u30d7\u306e\u53ef\u80fd\u6027\u304c\u3042\u308a\u307e\u3059
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 504:
> 502:
> compiler.warn.possible.this.escape=\u30B5\u30D6\u30AF\u30E9\u30B9\u524D\u306B\u6307\u5B9A\u53EF\u80FD\u306A''this''\u30A8\u30B9\u30B1\u30FC\u30D7\u306F\u5B8C\u5168\u306B\u521D\u671F\u5316\u3055\u308C\u307E\u3059
> 503:
> 504:
> compiler.warn.possible.this.escape.location=\u4E8B\u524D\u306B\u6307\u5B9A\u53EF\u80FD\u306A''this''\u30A8\u30B9\u30B1\u30FC\u30D7\u306F\u547C\u51FA\u3057\u3092\u4ECB\u3057\u3066\u3053\u3053\u306B\u51FA\u73FE\u3057\u307E\u3059
Suggestion:
compiler.warn.possible.this.escape.location=\u4e8b\u524d\u306e''this''\u30a8\u30b9\u30b1\u30fc\u30d7\u306f\u3053\u306e\u547c\u3073\u51fa\u3057\u3067\u51fa\u73fe\u3057\u307e\u3059
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 956:
> 954:
> compiler.err.unclosed.text.block=\u9589\u3058\u3089\u308C\u3066\u3044\u306A\u3044\u30C6\u30AD\u30B9\u30C8\u30FB\u30D6\u30ED\u30C3\u30AF
> 955:
> 956:
> compiler.err.string.template.is.not.well.formed=\u6587\u5B57\u5217\u30C6\u30F3\u30D7\u30EC\u30FC\u30C8\u304C\u6574\u5F62\u5F0F\u3067\u306F\u3042\u308A\u307E\u305B\u3093
Suggestion:
compiler.err.string.template.is.not.well.formed=\u6587\u5b57\u5217\u30c6\u30f3\u30d7\u30ec\u30fc\u30c8\u304c\u9069\u683c\u3067\u306f\u3042\u308a\u307e\u305b\u3093
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 958:
> 956:
> compiler.err.string.template.is.not.well.formed=\u6587\u5B57\u5217\u30C6\u30F3\u30D7\u30EC\u30FC\u30C8\u304C\u6574\u5F62\u5F0F\u3067\u306F\u3042\u308A\u307E\u305B\u3093
> 957:
> 958:
> compiler.err.text.block.template.is.not.well.formed=\u30C6\u30AD\u30B9\u30C8\u30FB\u30D6\u30ED\u30C3\u30AF\u30FB\u30C6\u30F3\u30D7\u30EC\u30FC\u30C8\u304C\u6574\u5F62\u5F0F\u3067\u306F\u3042\u308A\u307E\u305B\u3093
Suggestion:
compiler.err.text.block.template.is.not.well.formed=\u30c6\u30ad\u30b9\u30c8\u30fb\u30d6\u30ed\u30c3\u30af\u30fb\u30c6\u30f3\u30d7\u30ec\u30fc\u30c8\u304c\u9069\u683c\u3067\u306f\u3042\u308a\u307e\u305b\u3093
-
Changes requested by naoto (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14430#pullrequestreview-1477963618
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228635514
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228648714
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228651272
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228653057
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228654384
Re: RFR: 8309632: JDK 21 RDP1 L10n resource files update [v2]
On Mon, 12 Jun 2023 22:32:14 GMT, Justin Lu wrote:
>> Please review this PR which updates the JDK's localized resources since the
>> previous L10n translation drop (1/26).
>>
>> To help with reviewing the changes, @jonathan-gibbons created a tool which
>> displays the localized changes next to the original file's changes in UTF-8
>> native. Those files can be viewed
>> [here](https://cr.openjdk.org/~jlu/output/)
>>
>> For example,
>>
>> > src="https://github.com/openjdk/jdk21/assets/67398801/e68a48a8-916e-4860-afb0-526924c6a3d3";>
>>
>>
>> Please note that the HTML files only apply to .properties, and not .java
>> resources.
>
> Justin Lu has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Revert 'main.help.opt.any.file' value for _de
Left some comments on the translations mainly in Japanese. It is now very easy
to look at the l10n changes in the generated HTML. One small comment to the
tool is that it would be nice if the order in HTML (alphabetically sorted
currently) aligns with the order in the actual resource file. It is a great
improvement anyways.
src/java.desktop/macosx/classes/com/apple/laf/resources/aqua_ja.properties line
63:
> 61:
> FileChooser.newFolderExistsError.textAndMnemonic=\u305D\u306E\u540D\u524D\u306F\u3059\u3067\u306B\u4F7F\u7528\u3055\u308C\u3066\u3044\u307E\u3059
> 62: FileChooser.chooseButton.textAndMnemonic=\u9078\u629E
> 63:
> FileChooser.chooseButtonToolTip.textAndMnemonic=\u9078\u629E\u3057\u305F\u30D5\u30A1\u30A4\u30EB\u3092\u8FFD\u52A0\u3057\u307E\u3059
The translation seems wrong. It means to *add* the selected file, which is not
the same meaning as in English file
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_de.properties
line 140:
> 138:
> 139: # 0: type, 1: name
> 140: compiler.err.annotation.unrecognized.attribute.name=Annotation @{0}
> weist das unbekannte Attribut "{1}" auf
Is it intentional to replace single quotes with double quotes? It is done in
several locations in this file, but other languages keep using single quotes as
in the source English resource file.
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 505:
> 503:
> 504:
> compiler.warn.possible.this.escape.location=\u4E8B\u524D\u306B\u6307\u5B9A\u53EF\u80FD\u306A''this''\u30A8\u30B9\u30B1\u30FC\u30D7\u306F\u547C\u51FA\u3057\u3092\u4ECB\u3057\u3066\u3053\u3053\u306B\u51FA\u73FE\u3057\u307E\u3059
> 505:
These translations are very cryptic. Cannot understand it at first hand.
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler_ja.properties
line 959:
> 957:
> 958:
> compiler.err.text.block.template.is.not.well.formed=\u30C6\u30AD\u30B9\u30C8\u30FB\u30D6\u30ED\u30C3\u30AF\u30FB\u30C6\u30F3\u30D7\u30EC\u30FC\u30C8\u304C\u6574\u5F62\u5F0F\u3067\u306F\u3042\u308A\u307E\u305B\u3093
> 959:
The translation of "well-formed" is incorrect. It is not 整数式
-
PR Review: https://git.openjdk.org/jdk/pull/14430#pullrequestreview-1477795175
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228530280
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228521409
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228538758
PR Review Comment: https://git.openjdk.org/jdk/pull/14430#discussion_r1228529769
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v6]
On Thu, 11 May 2023 20:21:57 GMT, Justin Lu wrote: >> This PR converts Unicode sequences to UTF-8 native in .properties file. >> (Excluding the Unicode space and tab sequence). The conversion was done >> using native2ascii. >> >> In addition, the build logic is adjusted to support reading in the >> .properties files as UTF-8 during the conversion from .properties file to >> .java ListResourceBundle file. > > Justin Lu has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains 16 commits: > > - Convert the merged master changes to UTF-8 > - Merge master and fix conflicts > - Close streams when finished loading into props > - Adjust CF test to read in with UTF-8 to fix failing test > - Reconvert CS.properties to UTF-8 > - Revert all changes to CurrencySymbols.properties > - Bug6204853 should not be converted > - Copyright year for CompileProperties > - Redo translation for CS.properties > - Spot convert CurrencySymbols.properties > - ... and 6 more: https://git.openjdk.org/jdk/compare/4386d42d...f15b373a I think this is fine, as those properties files are JDK's own. I believe the benefit of moving to UTF-8 outweighs the issue you wrote, which can be remedied by changing the encoding in the IDEs. - PR Comment: https://git.openjdk.org/jdk/pull/12726#issuecomment-1544722480
Re: RFR: 8301377: adjust timeout for JLI GetObjectSizeIntrinsicsTest.java subtest again
On Fri, 21 Apr 2023 21:35:07 GMT, Daniel D. Daugherty wrote: > Trivial fixes to increase timeouts for tests that timeout under heavy stress: > [JDK-8301377](https://bugs.openjdk.org/browse/JDK-8301377) adjust timeout for > JLI GetObjectSizeIntrinsicsTest.java subtest again > [JDK-8305502](https://bugs.openjdk.org/browse/JDK-8305502) adjust timeouts in > three more M&M tests > [JDK-8302607](https://bugs.openjdk.org/browse/JDK-8302607) increase timeout > for ContinuousCallSiteTargetChange.java Marked as reviewed by naoto (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/13593#pullrequestreview-1396444259
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v5]
On Fri, 17 Mar 2023 22:27:48 GMT, Justin Lu wrote: >> This PR converts Unicode sequences to UTF-8 native in .properties file. >> (Excluding the Unicode space and tab sequence). The conversion was done >> using native2ascii. >> >> In addition, the build logic is adjusted to support reading in the >> .properties files as UTF-8 during the conversion from .properties file to >> .java ListResourceBundle file. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > Close streams when finished loading into props Hmm, I just wonder why they are sticking to ISO-8859-1 as the default. I know j.u.Properties defaults to 8859-1, but PropertyResourceBundle, which is their primary use defaults to UTF-8 since JDK9 (https://openjdk.org/jeps/226) - PR Comment: https://git.openjdk.org/jdk/pull/12726#issuecomment-1492682703
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native [v4]
On Fri, 17 Mar 2023 20:31:27 GMT, Andy Goryachev wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Adjust CF test to read in with UTF-8 to fix failing test
>
> make/jdk/src/classes/build/tools/compileproperties/CompileProperties.java
> line 226:
>
>> 224: Properties p = new Properties();
>> 225: try {
>> 226: FileInputStream input = new FileInputStream(propertiesPath);
>
> Should this stream be closed in a finally { } block?
or better be `try-with-resources`?
-
PR: https://git.openjdk.org/jdk/pull/12726
Re: RFR: 8301991: Convert l10n properties resource bundles to UTF-8 native
On Thu, 23 Feb 2023 09:04:23 GMT, Justin Lu wrote: > This PR converts Unicode sequences to UTF-8 native in .properties file. > (Excluding the Unicode space and tab sequence). The conversion was done using > native2ascii. > > In addition, the build logic is adjusted to support reading in the > .properties files as UTF-8 during the conversion from .properties file to > .java ListResourceBundle file. test/jdk/java/text/Format/NumberFormat/CurrencySymbols.properties line 156: > 154: zh=\u00A4 > 155: zh_CN=\uFFE5 > 156: zh_HK=HK$ Why are they not encoded into UTF-8 native? - PR: https://git.openjdk.org/jdk/pull/12726
Re: RFR: 8303814: getLastErrorString should avoid charset conversions [v3]
On Mon, 13 Mar 2023 15:55:27 GMT, Daniel Jeliński wrote:
>> This patch modifies the `getLastErrorString` method to return a `jstring`.
>> Thanks to that we can avoid unnecessary back and forth conversions between
>> Unicode and other charsets on Windows.
>>
>> Other changes include:
>> - the Windows implementation of `getLastErrorString` no longer checks
>> `errno`. I verified all uses of the method and confirmed that `errno` is not
>> used anywhere.
>> - While at it, I found and fixed a few calls to
>> `JNU_ThrowIOExceptionWithLastError` that were done in context where
>> `LastError` was not set.
>> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
>> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
>> have identical behavior.
>> - zip_util was modified to return static messages instead of generated ones.
>> The generated messages were not observed anywhere, because they were
>> replaced by a static message in ZIP_Open, which is the only method used by
>> other native code.
>> - `getLastErrorString` is no longer exported by libjava.
>>
>> Tier1-3 tests continue to pass.
>>
>> No new automated regression test; testing this requires installing a
>> language pack that cannot be displayed in the current code page.
>> Tested this manually by installing Chinese language pack on English Windows
>> 11, selecting Chinese language, then checking if the message on exception
>> thrown by `InetAddress.getByName("nonexistent.local");` starts with
>> `"不知道这样的主机。"` (or
>> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
>> change, the exception message started with a row of question marks.
>
> Daniel Jeliński has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Use NULL where appropriate
Marked as reviewed by naoto (Reviewer).
-
PR: https://git.openjdk.org/jdk/pull/12922
Re: RFR: 8303814: getLastErrorString should avoid charset conversions
On Wed, 8 Mar 2023 11:30:27 GMT, Daniel Jeliński wrote:
> This patch modifies the `getLastErrorString` method to return a `jstring`.
> Thanks to that we can avoid unnecessary back and forth conversions between
> Unicode and other charsets on Windows.
>
> Other changes include:
> - the Windows implementation of `getLastErrorString` no longer checks
> `errno`. I verified all uses of the method and confirmed that `errno` is not
> used anywhere.
> - While at it, I found and fixed a few calls to
> `JNU_ThrowIOExceptionWithLastError` that were done in context where
> `LastError` was not set.
> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
> have identical behavior.
> - zip_util was modified to return static messages instead of generated ones.
> The generated messages were not observed anywhere, because they were replaced
> by a static message in ZIP_Open, which is the only method used by other
> native code.
> - `getLastErrorString` is no longer exported by libjava.
>
> Tier1-3 tests continue to pass.
>
> No new automated regression test; testing this requires installing a language
> pack that cannot be displayed in the current code page.
> Tested this manually by installing Chinese language pack on English Windows
> 11, selecting Chinese language, then checking if the message on exception
> thrown by `InetAddress.getByName("nonexistent.local");` starts with
> `"不知道这样的主机。"` (or
> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
> change, the exception message started with a row of question marks.
Looks good (w/ some minor comments)
src/java.base/share/native/libzip/zip_util.c line 767:
> 765: * or NULL if an error occurred. If a zip error occurred then *pmsg will
> 766: * be set to the error message text if pmsg != 0. Otherwise, *pmsg will
> be
> 767: * set to NULL. Caller doesn't need to free the error message.
I'd put some more context here why the caller does not need to free. (as it is
a static text)
src/java.base/windows/native/libjava/jni_util_md.c line 80:
> 78: 0,
> 79: buf,
> 80: sizeof(buf) / sizeof(WCHAR),
Maybe better to #define the size 256 so that this division is not needed.
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 208:
> 206:
> 207: if (result == 0) {
> 208: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Could be replaced with `JNU_ThrowIOException`?
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 260:
> 258:
> 259: if (result == 0) {
> 260: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Same as above
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 299:
> 297:
> 298: if (result == 0) {
> 299: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Same here
-
PR: https://git.openjdk.org/jdk/pull/12922
Re: RFR: 8303814: getLastErrorString should avoid charset conversions
On Wed, 8 Mar 2023 11:30:27 GMT, Daniel Jeliński wrote:
> This patch modifies the `getLastErrorString` method to return a `jstring`.
> Thanks to that we can avoid unnecessary back and forth conversions between
> Unicode and other charsets on Windows.
>
> Other changes include:
> - the Windows implementation of `getLastErrorString` no longer checks
> `errno`. I verified all uses of the method and confirmed that `errno` is not
> used anywhere.
> - While at it, I found and fixed a few calls to
> `JNU_ThrowIOExceptionWithLastError` that were done in context where
> `LastError` was not set.
> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
> have identical behavior.
> - zip_util was modified to return static messages instead of generated ones.
> The generated messages were not observed anywhere, because they were replaced
> by a static message in ZIP_Open, which is the only method used by other
> native code.
> - `getLastErrorString` is no longer exported by libjava.
>
> Tier1-3 tests continue to pass.
>
> No new automated regression test; testing this requires installing a language
> pack that cannot be displayed in the current code page.
> Tested this manually by installing Chinese language pack on English Windows
> 11, selecting Chinese language, then checking if the message on exception
> thrown by `InetAddress.getByName("nonexistent.local");` starts with
> `"不知道这样的主机。"` (or
> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
> change, the exception message started with a row of question marks.
Hi Daniel,
I like the idea of using `jstring` directly to avoid platform string. My
concern is that is it OK to make it not export from libjava? Would it cause any
concern on other platforms? Also, now it is not consistent with the other
sibling `getErrorString()` call.
-
PR: https://git.openjdk.org/jdk/pull/12922
Re: [jdk20] RFR: 8300719: JDK 20 RDP2 L10n resource files update [v5]
On Tue, 24 Jan 2023 23:56:23 GMT, Damon Nguyen wrote: >> Open l10n drop. Files have been updated with translated versions. Whitespace >> tool has been ran on files. >> All tests passed > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Revert culture attribute LGTM - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk20/pull/116
Re: [jdk20] RFR: 8300719: JDK 20 RDP2 L10n resource files update [v3]
On Tue, 24 Jan 2023 22:40:55 GMT, Justin Lu wrote: >> OK, I can make the manual edit for this > > This change was made because it was compared to the original > messages.properties. In that file, the respective line is given as > `MANAGE_HOTSPOT_MBEANS_IN_COLON_=Manage Hotspot MBeans in:` with no trailing > space. That's why in this file, to ensure consistency, the trailing space was > stripped. > > I think either this space should be removed, or all 4 (en, ja, de, zh) should > have the `\u0020` appended. I agree that since it is a prefix it would make > sense for a space to be there, but there is no guarantee that what comes > after, or even the code that loads the .properties file does not already > handles that necessary space. > > Should the space remain removed like in the English source file and the other > l10n files, or do you think all 4 should be changed to add `\u0020`? Confirmed that the message is for a label (`sun.tools.jconsole.LabeledComponent`), followed by a Component, so no space seems needed in each file. Sorry for the false alarm. - PR: https://git.openjdk.org/jdk20/pull/116
Re: [jdk20] RFR: 8300719: JDK 20 RDP2 L10n resource files update [v2]
On Tue, 24 Jan 2023 22:12:26 GMT, Damon Nguyen wrote: >> Open l10n drop. Files have been updated with translated versions. Whitespace >> tool has been ran on files. >> All tests passed > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Change German help of jar command Changes requested by naoto (Reviewer). src/jdk.jconsole/share/classes/sun/tools/jconsole/resources/messages_zh_CN.properties line 174: > 172: MBEANS_TAB_UNSUBSCRIBE_NOTIFICATIONS_BUTTON=\u53D6\u6D88\u8BA2\u9605(&U) > 173: > MBEANS_TAB_UNSUBSCRIBE_NOTIFICATIONS_BUTTON_TOOLTIP=\u505C\u6B62\u76D1\u542C\u901A\u77E5 > 174: > MANAGE_HOTSPOT_MBEANS_IN_COLON_=\u7BA1\u7406\u4EE5\u4E0B\u4F4D\u7F6E\u7684 > HotSpot MBean: This looks like a prefix, so instead of removing the space, it should be replaced with `\u0020` like other locations. Also, it seems that other l10n files (de, ja) do not seem to have spaces in the first place. src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_de.properties line 38: > 36: resource.post-msi-script=Auszuf\u00FChrendes Skript nach dem Erstellen > der MSI-Datei f\u00FCr das EXE-Installationsprogramm > 37: resource.wxl-file=WiX-Lokalisierungsdatei > 38: resource.wxl-file-name=MsiInstallerStrings_en.wxl Shouldn't this be `de`? src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_ja.properties line 38: > 36: > resource.post-msi-script=exe\u30A4\u30F3\u30B9\u30C8\u30FC\u30E9\u306Emsi\u30D5\u30A1\u30A4\u30EB\u304C\u4F5C\u6210\u3055\u308C\u305F\u5F8C\u306B\u5B9F\u884C\u3059\u308B\u30B9\u30AF\u30EA\u30D7\u30C8 > 37: > resource.wxl-file=WiX\u30ED\u30FC\u30AB\u30EA\u30BC\u30FC\u30B7\u30E7\u30F3\u30FB\u30D5\u30A1\u30A4\u30EB > 38: resource.wxl-file-name=MsiInstallerStrings_en.wxl Shouldn't this be `ja`? src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources_zh_CN.properties line 38: > 36: resource.post-msi-script=\u5728\u4E3A exe > \u5B89\u88C5\u7A0B\u5E8F\u521B\u5EFA msi > \u6587\u4EF6\u4E4B\u540E\u8981\u8FD0\u884C\u7684\u811A\u672C > 37: resource.wxl-file=WiX \u672C\u5730\u5316\u6587\u4EF6 > 38: resource.wxl-file-name=MsiInstallerStrings_en.wxl Shouldn't this be `zh_CN`? src/jdk.localedata/share/classes/sun/util/resources/ext/CurrencyNames_zh_CN.properties line 63: > 61: # written authorization of the copyright holder. > 62: > 63: CNY=\uffe5 Do not remove this. - PR: https://git.openjdk.org/jdk20/pull/116
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v5]
On Fri, 16 Dec 2022 03:38:54 GMT, Damon Nguyen wrote: >> Open l10n drop >> All tests passed > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Revert old translation. Fix lang codes There is another translation drop scheduled in January, so our plan is to figure out why the source extraction in this drop mistakenly used old files till then, then address it in the January l10n drop. - PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v4]
On Thu, 15 Dec 2022 23:01:35 GMT, Damon Nguyen wrote:
>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Revert double quote as well
Marked as reviewed by naoto (Reviewer).
src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/resources/jdeprscan_de.properties
line 26:
> 24: #
> 25:
> 26: main.usage=Verwendung: jdeprscan [Optionen] '{dir|jar|class}'
> ...\n\nOptionen:\n--class-path PATH\n--for-removal\n
> --full-version\n -? -h --help\n -l--list\n--release {0}\n -v
> --verbose\n--version
Good catch!
-
PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v3]
On Thu, 15 Dec 2022 20:43:42 GMT, Damon Nguyen wrote: >> Open l10n drop >> All tests passed > > Damon Nguyen has updated the pull request incrementally with one additional > commit since the last revision: > > Fix https and changed URL back Thanks for the update, Damon. Looks good for this drop. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk20/pull/35
Re: [jdk20] RFR: 8298133: JDK 20 RDP1 L10n resource files update - msgdrop 10 [v2]
On Thu, 15 Dec 2022 02:45:48 GMT, Damon Nguyen wrote:
>> Open l10n drop
>> All tests passed
>
> Damon Nguyen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Removed trailing whitespace
Changes requested by naoto (Reviewer).
src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac_de.properties
line 217:
> 215: javac.msg.usage.nonstandard.footer=Diese zus\u00E4tzlichen Optionen
> k\u00F6nnen jederzeit ohne vorherige Ank\u00FCndigung ge\u00E4ndert werden.
> 216:
> 217: javac.msg.bug=Im Compiler ({0}) ist eine Ausnahme aufgetreten. Erstellen
> Sie auf der Java-Seite zum Melden von Bugs (http://bugreport.java.com) einen
> Bugbericht, nachdem Sie die Bugdatenbank (http://bugs.java.com) auf Duplikate
> gepr\u00FCft haben. Geben Sie in Ihrem Bericht Ihr Programm, die folgende
> Diagnose und die Parameter an, die Sie dem Java-Compiler \u00FCbergeben
> haben. Vielen Dank.
How come `https` turned into `http`? I see these happening all over the places
other than this location as well.
src/jdk.jdeps/share/classes/com/sun/tools/jdeps/resources/jdeps_de.properties
line 134:
> 132: public.api.replacement.column.header=Vorgeschlagene Ersetzung
> 133: artifact.not.found=nicht gefunden
> 134:
> jdeps.wiki.url=https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool
This seems to change backward. `openjdk.java.net` has changed to `openjdk.org`
Same issues are observed in other locales.
-
PR: https://git.openjdk.org/jdk20/pull/35
Re: RFR: 8296546: Add @spec tags to API [v4]
On Thu, 1 Dec 2022 19:37:28 GMT, Jonathan Gibbons wrote: >> src/java.base/share/classes/java/lang/Character.java line 172: >> >>> 170: * occur. For example, in a future release, synchronization may fail. >>> 171: * >>> 172: * @spec https://www.unicode.org/reports/tr27 Unicode 3.1.0 >> >> This should probably be removed, as the original link (explaining `U+n` >> notation) is broken. > > @naotoj The edits are driven by a script, using info about existing links in > the same doc comment. If you don't think this reference is appropriate, it > would be better to either remove the existing link (and I'll regenerate this > patch) or else this patch goes through and you fix up both the existing link > and the `@spec` tag afterwards. Either way is fine with me. I will fix it up if you choose the latter. - PR: https://git.openjdk.org/jdk/pull/11073
Re: RFR: 8296546: Add @spec tags to API
On Thu, 10 Nov 2022 01:10:13 GMT, Jonathan Gibbons wrote: > Please review a "somewhat automated" change to insert `@spec` tags into doc > comments, as appropriate, to leverage the recent new javadoc feature to > generate a new page listing the references to all external specifications > listed in the `@spec` tags. > > "Somewhat automated" means that I wrote and used a temporary utility to scan > doc comments looking for HTML links to selected sites, such as `ietf.org`, > `unicode.org`, `w3.org`. These links may be in the main description of a doc > comment, or in `@see` tags. For each link, the URL is examined, and > "normalized", and inserted into the doc comment with a new `@spec` tag, > giving the link and tile for the spec. > > "Normalized" means... > * Use `https:` where possible (includes pretty much all cases) > * Use a single consistent host name for all URLs coming from the same spec > site (i.e. don't use different aliases for the same site) > * Point to the root page of a multi-page spec > * Use a consistent form of the spec, preferring HTML over plain text where > both are available (this mostly applies to IETF specs) > > In addition, a "standard" title is determined for all specs, determined > either from the content of the (main) spec page or from site index pages. > > The net effect is (or should be) that **all** the changes are to just **add** > new `@spec` tags, based on the links found in each doc comment. There should > be no other changes to the doc comments, or to the implementation of any > classes and interfaces. > > That being said, the utility I wrote does have additional abilities, to > update the links that it finds (e.g. changing to use `https:` etc,) but those > features are _not_ being used here, but could be used in followup PRs if > component teams so desired. I did notice while working on this overall > feature that many of our links do point to "outdated" pages, some with > eye-catching notices declaring that the spec has been superseded. Determining > how, when and where to update such links is beyond the scope of this PR. > > Going forward, it is to be hoped that component teams will maintain the > underlying links, and the URLs in `@spec` tags, such that if references to > external specifications are updated, this will include updating the `@spec` > tags. > > To see the effect of all these new `@spec` tags, see > http://cr.openjdk.java.net/~jjg/8296546/api.00/ > > In particular, see the new [External > Specifications](http://cr.openjdk.java.net/~jjg/8296546/api.00/external-specs.html) > page, which you can also find via the new link near the top of the > [Index](http://cr.openjdk.java.net/~jjg/8296546/api.00/index-files/index-1.html) > pages. src/java.base/share/classes/java/lang/Character.java line 172: > 170: * occur. For example, in a future release, synchronization may fail. > 171: * > 172: * @spec https://www.unicode.org/reports/tr27 Unicode 3.1.0 This should probably be removed, as the original link (explaining `U+n` notation) is broken. - PR: https://git.openjdk.org/jdk/pull/11073
Re: RFR: 8296822: ProblemList jdk/jfr/api/consumer/TestRecordingFileWrite.java
On Thu, 10 Nov 2022 22:54:54 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList jdk/jfr/api/consumer/TestRecordingFileWrite.java. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11095
Re: RFR: 8295729: Add jcheck whitespace checking for properties files [v3]
On Tue, 25 Oct 2022 13:43:56 GMT, Weijun Wang wrote: > I wonder if this means even if we get everything right now the tool might add > trailing spaces again later. Good catch, Max. Yes, that should be dealt with in the translation process. > I suggest we focus on the English files this time and file a bug to the > translation tool. Agree. - PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Add jcheck whitespace checking for properties files [v3]
On Mon, 24 Oct 2022 20:08:02 GMT, Andy Goryachev wrote: > Good idea! I wonder if this should be done as a unit test. go through all the > bundles and check leading/trailing whitespace. Right. Definitely not a job for `jcheck`, but it should be considered somewhere in the l10n process. - PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Add jcheck whitespace checking for properties files [v3]
On Mon, 24 Oct 2022 19:21:07 GMT, Magnus Ihse Bursie wrote: >> Properties files is essentially source code. It should have the same >> whitespace checks as all other source code, so we don't get spurious >> trailing whitespace changes. >> >> With the new Skara jcheck, it is possible to increase the coverage of the >> whitespace checks (in the old mercurial version, this was more or less >> impossible). >> >> The only manual change is to `.jcheck/conf`. All other changes were made by >> running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ >> \t]*$//'`. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Revert "Remove check for .properties from jcheck" > >This reverts commit c91fdaa19dc06351598bd1c0614e1af3bfa08ae2. > - Change trailing space and tab in values to unicode encoding The problem here is that all those (unnecessary) trailing spaces are appended by the external translators, who are not aware those spaces should not be at the end. I think what we can do is check the original English properties values that the engineers provided, and if there is no trailing spaces there, we can safely remove trailing spaces in localized bundles. - PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Remove trailing whitespace from non-value lines in properties files [v2]
On Fri, 21 Oct 2022 08:17:46 GMT, Magnus Ihse Bursie wrote: >> Properties files is essentially source code. It should have the same >> whitespace checks as all other source code, so we don't get spurious >> trailing whitespace changes. >> >> With the new Skara jcheck, it is possible to increase the coverage of the >> whitespace checks (in the old mercurial version, this was more or less >> impossible). >> >> The only manual change is to `.jcheck/conf`. All other changes were made by >> running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ >> \t]*$//'`. > > Magnus Ihse Bursie has updated the pull request incrementally with two > additional commits since the last revision: > > - Remove check for .properties from jcheck > - Restore trailing whitespace for property values One possible solution to this is to replace those dangling white spaces with explicit Unicode escapes, i.e. "\u0009" and "\u0020". This way jcheck can safely be enabled to detect future unwanted trailing spaces, and possibly existing ones can be visually recognizable by the engineers to correct them. - PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Add jcheck whitespace checking for properties files
On Thu, 20 Oct 2022 11:58:58 GMT, Magnus Ihse Bursie wrote: > Properties files is essentially source code. It should have the same > whitespace checks as all other source code, so we don't get spurious trailing > whitespace changes. > > With the new Skara jcheck, it is possible to increase the coverage of the > whitespace checks (in the old mercurial version, this was more or less > impossible). > > The only manual change is to `.jcheck/conf`. All other changes were made by > running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ > \t]*$//'`. (retracting approval) - Changes requested by naoto (Reviewer). PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Add jcheck whitespace checking for properties files
On Thu, 20 Oct 2022 11:58:58 GMT, Magnus Ihse Bursie wrote: > Properties files is essentially source code. It should have the same > whitespace checks as all other source code, so we don't get spurious trailing > whitespace changes. > > With the new Skara jcheck, it is possible to increase the coverage of the > whitespace checks (in the old mercurial version, this was more or less > impossible). > > The only manual change is to `.jcheck/conf`. All other changes were made by > running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ > \t]*$//'`. Retracting my approval too. Thanks for the catch, Andy! That sounds reasonable > Magnus - PR: https://git.openjdk.org/jdk/pull/10792
Re: RFR: 8295729: Add jcheck whitespace checking for properties files
On Thu, 20 Oct 2022 11:58:58 GMT, Magnus Ihse Bursie wrote: > Properties files is essentially source code. It should have the same > whitespace checks as all other source code, so we don't get spurious trailing > whitespace changes. > > With the new Skara jcheck, it is possible to increase the coverage of the > whitespace checks (in the old mercurial version, this was more or less > impossible). > > The only manual change is to `.jcheck/conf`. All other changes were made by > running `find . -type f -iname "*.properties" | xargs gsed -i -e 's/[ > \t]*$//'`. LGTM. Haven't looked at all the l10n files. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk/pull/10792
Re: [jdk19] RFR: 8290889: JDK 19 RDP2 L10n resource files update - msgdrop 10 [v4]
On Fri, 29 Jul 2022 18:13:25 GMT, Alisen Chung wrote: >> open l10n msg drop >> All tests passed. > > Alisen Chung has updated the pull request incrementally with one additional > commit since the last revision: > > added comments in CurrencyNames root in base, moved US CurrencyNames back > to base, readded original Chinese translation LGTM - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.org/jdk19/pull/154
Re: [jdk19] RFR: 8290889: JDK 19 RDP2 L10n resource files update - msgdrop 10 [v3]
On Tue, 26 Jul 2022 20:31:44 GMT, Alisen Chung wrote: >> open l10n msg drop >> All tests passed. > > Alisen Chung has updated the pull request incrementally with one additional > commit since the last revision: > > removed localized files from base - src/jdk.localedata/share/classes/sun/util/resources/ext/CurrencyNames_en_US.properties: this should not be moved from `java.base` module - I still need some comments explaining `XXX=XXX` in the root bundle of `CurrencyNames`. Something like These currency symbol entries are for the root bundle only to avoid throwing a MissingResourceException. Should not be copied into each localized resource bundle. src/jdk.localedata/share/classes/sun/util/resources/ext/CurrencyNames_zh_CN.properties line 63: > 61: # written authorization of the copyright holder. > 62: > 63: CNY=\uffe5 Do not remove the original translation. - Changes requested by naoto (Reviewer). PR: https://git.openjdk.org/jdk19/pull/154
Re: [jdk19] RFR: 8290889: JDK 19 RDP2 L10n resource files update - msgdrop 10
On Sat, 23 Jul 2022 03:03:25 GMT, Alisen Chung wrote: > open l10n msg drop > All tests passed. Changes requested by naoto (Reviewer). src/java.base/share/classes/sun/util/resources/CurrencyNames_de.properties line 63: > 61: # written authorization of the copyright holder. > 62: > 63: ADP=ADP These currency symbols (`XXX=XXX`) are all in the base `CurrencyNames.properties`, so not needed in each localized resources (They are in the base in order to avoid throwing exceptions). Also, the localized files should not reside in `java.base`, but to be merged with the ones in `jdk.localedata` module These comments hold for all localized resources (de, ja, zh-CN). For furure reference, it may be good to add some comment in the base `CurrencyNames.properties` about this. src/java.base/share/data/currency/CurrencyData_de.properties line 1: > 1: # `CurrencyData.properties` is not a localizable resource bundle, but a properties file containing currency data. No need to add these localized resources. - PR: https://git.openjdk.org/jdk19/pull/154
Re: [jdk19] Integrated: 8290203: ProblemList vmTestbase/nsk/jvmti/scenarios/capability/CM03/cm03t001/TestDescription.java on linux-all
On Tue, 12 Jul 2022 21:15:57 GMT, Daniel D. Daugherty wrote: > A trivial fix to ProblemList > vmTestbase/nsk/jvmti/scenarios/capability/CM03/cm03t001/TestDescription.java > on linux-all. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk19/pull/137
Re: RFR: 8289768: Clean up unused code [v2]
On Wed, 6 Jul 2022 05:32:29 GMT, Daniel Jeliński wrote: >> This patch removes many unused variables and one unused label reported by >> the compilers when relevant warnings are enabled. >> >> The unused code was found by compiling after removing `unused` from the list >> of disabled warnings for >> [gcc](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L190) >> and >> [clang](https://github.com/openjdk/jdk/blob/master/make/autoconf/flags-cflags.m4#L203), >> and enabling >> [C4189](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4189?view=msvc-170) >> MSVC warning. >> >> I only removed variables that were uninitialized or initialized without side >> effects. I verified that the removed variables were not used in any >> `#ifdef`'d code. I checked that the changed code still compiles on Windows, >> Linux and Mac, both in release and debug versions. > > Daniel Jeliński 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 four additional > commits since the last revision: > > - Merge remote-tracking branch 'origin' into unused-variables > - Remove unused variables (windows) > - Remove unused variables (macos) > - Remove unused variables (linux) Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.org/jdk/pull/9383
