Integrated: 8330542: Template for Creating Strict JAXP Configuration File
On Wed, 17 Apr 2024 23:24:06 GMT, Joe Wang wrote: > Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. > > Updated on 5/23/2024 > > Provide a template `jaxp-strict.template` instead of a properties file. This > template can be used to create custom configuration files. This pull request has now been integrated. Changeset: 91caec07 Author:Joe Wang URL: https://git.openjdk.org/jdk/commit/91caec07cb2e4d98d4366f5627f55834282caa94 Stats: 287 lines in 5 files changed: 263 ins; 7 del; 17 mod 8330542: Template for Creating Strict JAXP Configuration File Reviewed-by: lancea, erikj, alanb, ihse, mullan, naoto - PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v15]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. > > Updated on 5/23/2024 > > Provide a template `jaxp-strict.template` instead of a properties file. This > template can be used to create custom configuration files. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: update properties files with wording suggestions; move summary to after the test tag - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/714095d1..abc1e88b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=14 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=13-14 Stats: 21 lines in 3 files changed: 2 ins; 2 del; 17 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v14]
On Fri, 24 May 2024 16:36:32 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. >> >> Updated on 5/23/2024 >> >> Provide a template `jaxp-strict.template` instead of a properties file. This >> template can be used to create custom configuration files. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > rename the template to jaxp-strict.properties.template Thanks Sean, Naoto! Updated accordingly. - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2135662540
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v14]
On Fri, 24 May 2024 16:36:32 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. >> >> Updated on 5/23/2024 >> >> Provide a template `jaxp-strict.template` instead of a properties file. This >> template can be used to create custom configuration files. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > rename the template to jaxp-strict.properties.template Thanks Magnus, Alan. Renamed the template to jaxp-strict.properties.template. - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2129975324
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v14]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. > > Updated on 5/23/2024 > > Provide a template `jaxp-strict.template` instead of a properties file. This > template can be used to create custom configuration files. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: rename the template to jaxp-strict.properties.template - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/0de8ad69..714095d1 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=13 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=12-13 Stats: 10 lines in 4 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v12]
On Wed, 22 May 2024 08:52:42 GMT, Magnus Ihse Bursie wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> add a note to module-info > > make/modules/java.xml/Copy.gmk line 35: > >> 33: $(eval $(call SetupCopyFiles, COPY_XML_MODULE_CONF, \ >> 34: DEST := $(CONF_DST_DIR), \ >> 35: FILES := $(wildcard >> $(TOPDIR)/src/java.xml/share/conf/jaxp*.properties*), \ > > I don't think you need, nor should have, the asterisk after the extension. > You are only copying `.properties` files. > > Suggestion: > > FILES := $(wildcard $(TOPDIR)/src/java.xml/share/conf/jaxp*.properties), \ Thanks Magnus! - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1612802098
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v13]
On Fri, 24 May 2024 05:26:40 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated on 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. >> >> Updated on 5/18/2024 >> >> Withdraw changes to jaxp.properties. The original idea was to match >> jaxp-strict.properties with regard to the properties. However, that change >> impact the configuration process, resulting in tests that verify the process >> to fail. >> >> Updated on 5/23/2024 >> >> Provide a template `jaxp-strict.template` instead of a properties file. This >> template can be used to create custom configuration files. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > add a template instead of a property file; remove implNote; update test and > make script accordingly. Update: the plan has changed from providing a configuration file to a template. Module description, test and make file are updated accordingly. Thanks Erik for the new make file! - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2128568806
Re: RFR: 8330542: Template for Creating Strict JAXP Configuration File [v13]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: add a template instead of a property file; remove implNote; update test and make script accordingly. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/dd7f6239..0de8ad69 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=12 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=11-12 Stats: 299 lines in 6 files changed: 133 ins; 155 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a stricter default configuration [v12]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: add a note to module-info - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/55a86db3..dd7f6239 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=11 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=10-11 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 13:03:36 GMT, Sean Mullan wrote: >> In XML parlance, a "processor" is an aggregation of parsers, serializers, >> and other things that contribute to the processing. So I think it could be >> either here, but you have a point and if it stays as "processor" then it >> should link #FacPro where the term is defined. > > It's the wording that doesn't sound right, before this you have "making" > which doesn't sound right with "process". Maybe it needs two sentences. Thanks! Same as Alan, I thought you were talking about "processor" as well on the first glance, then realized you're referring to the parallel statements :-) - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606995925
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v11]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated on 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. > > Updated on 5/18/2024 > > Withdraw changes to jaxp.properties. The original idea was to match > jaxp-strict.properties with regard to the properties. However, that change > impact the configuration process, resulting in tests that verify the process > to fail. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: updated jaxp-strict; fixed typo in module-info. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/dfc965c6..55a86db3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=10 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=09-10 Stats: 5 lines in 2 files changed: 0 ins; 2 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
On Mon, 20 May 2024 07:13:02 GMT, Alan Bateman wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> withdraw changes to jaxp.properties. The configuration process has not >> changed, changing the default configuration would result in many failures >> that test the process. > > src/java.xml/share/conf/jaxp-strict.properties line 17: > >> 15: # java -Djava.xml.config.file=$JAVA_HOME/conf/jaxp-strict.properties >> 16: # >> 17: # The pathname to the configuration file must be valid. If it is not >> absolute, > > I think it would be better to drop this paragraph or else just replace it > with a sentence to say that the java.xml module description specifies the > system property. Thanks Alan. Replaced the paragraph with a reference to the module description. - PR Review Comment: https://git.openjdk.org/jdk/pull/18831#discussion_r1606993156
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v10]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: withdraw changes to jaxp.properties. The configuration process has not changed, changing the default configuration would result in many failures that test the process. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/2ee2c7ca..dfc965c6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=09 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=08-09 Stats: 10 lines in 2 files changed: 0 ins; 0 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v8]
On Thu, 16 May 2024 22:20:39 GMT, Joe Wang wrote: >> Add two sample configuration files: >> >> jaxp-strict.properties: used to set strict configuration, stricter than >> jaxp.properties in previous versions such as JDK 22 >> >>> jaxp-compat.properties: used to regain compatibility from any more >>> restricted configuration than previous versions such as JDK 22 >> >> Updated 5/16/2024 >> >> Design change: >> The design is changed to include in the JDK two configuration files that are >> the default jaxp.properties and jaxp-strict.properties, instead of three, >> dropping jaxp-compat.properties. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > remove jaxp-compat.properties from the list Thanks Alan, Erik! Updated accordingly. - PR Comment: https://git.openjdk.org/jdk/pull/18831#issuecomment-2118424649
Re: RFR: 8330542: Add jaxp-strict.properties in preparation for a secure by default configuration [v9]
> Add two sample configuration files: > > jaxp-strict.properties: used to set strict configuration, stricter than > jaxp.properties in previous versions such as JDK 22 > >> jaxp-compat.properties: used to regain compatibility from any more >> restricted configuration than previous versions such as JDK 22 > > Updated 5/16/2024 > > Design change: > The design is changed to include in the JDK two configuration files that are > the default jaxp.properties and jaxp-strict.properties, instead of three, > dropping jaxp-compat.properties. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: modernize make copy; update module summary and jaxp-strict. - Changes: - all: https://git.openjdk.org/jdk/pull/18831/files - new: https://git.openjdk.org/jdk/pull/18831/files/cf4df792..2ee2c7ca Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=18831=08 - incr: https://webrevs.openjdk.org/?repo=jdk=18831=07-08 Stats: 24 lines in 3 files changed: 1 ins; 12 del; 11 mod Patch: https://git.openjdk.org/jdk/pull/18831.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/18831/head:pull/18831 PR: https://git.openjdk.org/jdk/pull/18831
Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v7]
On Tue, 5 Mar 2024 17:50:09 GMT, Naoto Sato wrote: >> This PR intends to remove the legacy `COMPAT` locale data from the JDK. The >> `COMPAT` locale data was introduced for applications' migratory purposes >> transitioning to `CLDR`. It is becoming a technical debt and now is the time >> to remove it (we've been emitting a warning at JVM startup since JDK21, if >> the app is using `COMPAT`). A corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflects review comments Marked as reviewed by joehw (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/17991#pullrequestreview-1917852026
Re: RFR: 8174269: Remove COMPAT locale data provider from JDK [v6]
On Mon, 4 Mar 2024 19:07:44 GMT, Naoto Sato wrote: >> This PR intends to remove the legacy `COMPAT` locale data from the JDK. The >> `COMPAT` locale data was introduced for applications' migratory purposes >> transitioning to `CLDR`. It is becoming a technical debt and now is the time >> to remove it (we've been emitting a warning at JVM startup since JDK21, if >> the app is using `COMPAT`). A corresponding CSR has also been drafted. > > Naoto Sato 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 44 additional commits since > the last revision: > > - Merge branch 'master' into JDK-8174269-COMPAT-Removal > - Addressing review comments > - Update test/jdk/java/text/Format/DateFormat/Bug6683975.java > >Co-authored-by: Justin Lu > - Remove `GensrcLocaleData.gmk` > - Update make/jdk/src/classes/build/tools/cldrconverter/CLDRConverter.java > >Co-authored-by: Andrey Turbanov > - cleanup > - BreakIteratorProvider available locales fix > - clean-up > - test fixes > - makeZoneData.pl fix > - ... and 34 more: https://git.openjdk.org/jdk/compare/9da59104...b771e303 LGTM. This is a lot of work. Looking through the files alone takes hours. Kudos to the great work! I kind of like some of the date formats in COMPACT to be honest :-) make/jdk/src/classes/build/tools/cldrconverter/CLDRConverter.java line 1320: > 1318: * "USdst" -> "D" > 1319: * > 1320: * `tzdbLinks` retains `Link`s of time zones. if the value nit, "if the value" seems to be an unfinished sentence. src/java.base/share/classes/sun/util/locale/provider/BaseLocaleDataMetaInfo.java line 31: > 29: * It is used to avoid loading non-existent localized resources so that > 30: * jar files won't be opened unnecessary to look up them. > 31: */ nit: move the class description to right above the class? "unnecessary" -> "unnecessarily" - Marked as reviewed by joehw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17991#pullrequestreview-1915980264 PR Comment: https://git.openjdk.org/jdk/pull/17991#issuecomment-1978071713 PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1512206671 PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1512207766
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. java.xml changes look fine to me. - Marked as reviewed by joehw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17789#pullrequestreview-1876181019
Re: RFR: 8325558: Add jcheck whitespace checking for properties files
On Fri, 9 Feb 2024 13:46:06 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. > > src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/msg/XPointerMessages.properties > line 24: > >> 22: # Messages for message reporting >> 23: BadMessageKey = The error message corresponding to the message key can >> not be found. >> 24: FormatFailed = An internal error occurred while formatting the following >> message:\n > > Same here with `:\n`... It's done with the code (that is, a key is appended with the code). In fact, the whole Xerces stack was implemented this way. I'd leave it as is. - PR Review Comment: https://git.openjdk.org/jdk/pull/17789#discussion_r1486731927
Re: RFR: JDK-8325189: Enable this-escape javac warning in java.base
On Fri, 2 Feb 2024 23:36:41 GMT, Joe Darcy wrote: > After the "this-escape" lint warning was added to javac (JDK-8015831), the > base module was not updated to be able to compile with this warning enabled. > This PR makes the necessary changes to allow the base module to build with > the warning enabled. The two XML changes look good to me. There would be a lot of warnings in the java.xml module as well, if we had to do it in the future. - Marked as reviewed by joehw (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/17692#pullrequestreview-1866004351
Re: [jdk22] RFR: 8322647: Short name for the `Europe/Lisbon` time zone is incorrect
On Thu, 4 Jan 2024 17:26:37 GMT, Naoto Sato wrote: > 8322647: Short name for the `Europe/Lisbon` time zone is incorrect Marked as reviewed by joehw (Reviewer). - PR Review: https://git.openjdk.org/jdk22/pull/30#pullrequestreview-1804685583
Integrated: 8306055: Add a built-in Catalog to JDK XML module
On Fri, 17 Nov 2023 20:22:40 GMT, Joe Wang wrote: > Implement the built-in Catalog. This pull request has now been integrated. Changeset: 93bdc2a6 Author: Joe Wang URL: https://git.openjdk.org/jdk/commit/93bdc2a6db91a95d6ee52ec92080e586c694dad5 Stats: 1647 lines in 50 files changed: 1434 ins; 118 del; 95 mod 8306055: Add a built-in Catalog to JDK XML module Reviewed-by: ihse, lancea, alanb - PR: https://git.openjdk.org/jdk/pull/16719
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v3]
On Wed, 22 Nov 2023 08:58:41 GMT, Alan Bateman wrote: > I'm happy with the addition of the JDK built-in catalog, the inclusion of the > DTD defined by Java SE, and the docs updates. Thanks again, Alan, I really appreciate it! Finally got all pieces together, a nice foundation for the next. - PR Comment: https://git.openjdk.org/jdk/pull/16719#issuecomment-1823167935
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v4]
> Implement the built-in Catalog. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: fix whitespace - Changes: - all: https://git.openjdk.org/jdk/pull/16719/files - new: https://git.openjdk.org/jdk/pull/16719/files/4914278a..d7a7c9a0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16719=03 - incr: https://webrevs.openjdk.org/?repo=jdk=16719=02-03 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16719.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16719/head:pull/16719 PR: https://git.openjdk.org/jdk/pull/16719
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v3]
On Wed, 22 Nov 2023 08:08:39 GMT, Andrey Turbanov wrote:
>> Joe Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add a note; fix alignment
>
> test/jaxp/javax/xml/jaxp/unittest/common/catalog/CatalogTestBase.java line
> 299:
>
>> 297: Properties config, Properties[] sysProp, Properties[] apiProp,
>> CustomCatalog cc,
>> 298: boolean expectError, String error) throws Exception {
>> 299: testSchema1(filename, xsd, fsp, state, config, sysProp,
>> apiProp, cc, expectError, error);
>
> Suggestion:
>
> testSchema1(filename, xsd, fsp, state, config, sysProp, apiProp, cc,
> expectError, error);
Thanks. Fixed whitespace.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/16719#discussion_r1402418756
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
On Mon, 20 Nov 2023 16:48:51 GMT, Lance Andersen wrote:
> Hi Joe,
>
> Thank you for all of your hard work and perseverance to drive this project as
> I realize it was a lot of work and iterations based on feedback from the team.
>
> Overall looks good. A couple of minor comments below
Thanks Lance for your help with all that iterations and feedbacks. I really
appreciate it!
I'm glad we were able to split the project into smaller/more manageable tasks.
Changeset this size feels a lot easier to handle.
> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
> line 1084:
>
>> 1082: String publicId, String systemId) {
>> 1083: InputSource is = resolveWithCatalog(cr, cFile, publicId,
>> systemId);
>> 1084: //if (is != null && !is.isEmpty()) {
>
> Probably can delete this
Changed to a note, as a reminder that empty source isn't considered resolved in
this case.
> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/XMLEntityManager.java
> line 1101:
>
>> 1099: return cr.resolveEntity(publicId, systemId);
>> 1100: } catch (CatalogException e) {
>> 1101:
>> fErrorReporter.reportError(XMLMessageFormatter.XML_DOMAIN,"CatalogException",
>
> Alignment looks like it needs to be sanity checked here
Fixed.
-
PR Comment: https://git.openjdk.org/jdk/pull/16719#issuecomment-1819538297
PR Review Comment: https://git.openjdk.org/jdk/pull/16719#discussion_r1399549063
PR Review Comment: https://git.openjdk.org/jdk/pull/16719#discussion_r1399549587
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
On Mon, 20 Nov 2023 16:38:54 GMT, Magnus Ihse Bursie wrote: > Build change trivially okay Thanks Magnus! - PR Comment: https://git.openjdk.org/jdk/pull/16719#issuecomment-1819529895
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v3]
> Implement the built-in Catalog. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: add a note; fix alignment - Changes: - all: https://git.openjdk.org/jdk/pull/16719/files - new: https://git.openjdk.org/jdk/pull/16719/files/c1059a84..4914278a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16719=02 - incr: https://webrevs.openjdk.org/?repo=jdk=16719=01-02 Stats: 4 lines in 1 file changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/16719.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16719/head:pull/16719 PR: https://git.openjdk.org/jdk/pull/16719
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
On Sun, 19 Nov 2023 23:36:16 GMT, Joe Wang wrote: >> Implement the built-in Catalog. > > Joe Wang has updated the pull request incrementally with one additional > commit since the last revision: > > remove J2SE directory; add note explaining how DTDs are resolved Tests: Tier 1-3 and JCK passed. SPECjvm2008: no regression - PR Comment: https://git.openjdk.org/jdk/pull/16719#issuecomment-1818021731
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
On Sun, 19 Nov 2023 08:46:30 GMT, Alan Bateman wrote: >> Joe Wang has updated the pull request incrementally with one additional >> commit since the last revision: >> >> remove J2SE directory; add note explaining how DTDs are resolved > > src/java.xml/share/classes/module-info.java line 910: > >> 908: * Method 1 >> 909: * 22 >> 910: * > > I went through a number of iterations with Joe on this implNote so happy with > this version. Thanks Alan! I really appreciate your allocating so much time to this project, esp. the APIs and CSRs. So this is the last piece to finally complete the project. - PR Review Comment: https://git.openjdk.org/jdk/pull/16719#discussion_r1398521238
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
On Sun, 19 Nov 2023 08:45:29 GMT, Alan Bateman wrote: >> src/java.xml/share/classes/jdk/xml/internal/jdkcatalog/JDKCatalog.xml line >> 34: >> >>> 32: >>> 33: http://java.sun.com/dtd/preferences.dtd; >>> uri="J2SE/preferences.dtd"/> >>> 34: http://java.sun.com/dtd/properties.dtd; >>> uri="J2SE/properties.dtd"/> >> >> Would it be possible to provide a summary on how the relative uri in the >> "uri" attribute is handled? Asking as it's not clear to me how the decoding >> is handling, meaning this is a relative URI, not a relative file path. > > Can we move DTD in the JDK's catalog to a "Java" or "JavaSE" directory, only > because "J2SE" feels a bit yesterday. Thanks Alan. Removed “J2SE”. Added a note explaining how the DTDs are resolved. In early prototypes, the Catalog contained 7 sub-groups in the java/dtd group, including J2SE and J2EE, names came from their original places where these DTDs were defined. You're right, now that the built-in catalog contains only DTDs defined in the Java platform, it makes sense to keep just the java/dtd group. - PR Review Comment: https://git.openjdk.org/jdk/pull/16719#discussion_r1398519536
Re: RFR: 8306055: Add a built-in Catalog to JDK XML module [v2]
> Implement the built-in Catalog. Joe Wang has updated the pull request incrementally with one additional commit since the last revision: remove J2SE directory; add note explaining how DTDs are resolved - Changes: - all: https://git.openjdk.org/jdk/pull/16719/files - new: https://git.openjdk.org/jdk/pull/16719/files/f52f8ad6..c1059a84 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16719=01 - incr: https://webrevs.openjdk.org/?repo=jdk=16719=00-01 Stats: 8 lines in 4 files changed: 5 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16719.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16719/head:pull/16719 PR: https://git.openjdk.org/jdk/pull/16719
RFR: 8306055: Add a built-in Catalog to JDK XML module
Implement the built-in Catalog. - Commit messages: - fix whitespace error - 8306055: Add a built-in Catalog to JDK XML module Changes: https://git.openjdk.org/jdk/pull/16719/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16719=00 Issue: https://bugs.openjdk.org/browse/JDK-8306055 Stats: 1641 lines in 50 files changed: 1429 ins; 118 del; 94 mod Patch: https://git.openjdk.org/jdk/pull/16719.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16719/head:pull/16719 PR: https://git.openjdk.org/jdk/pull/16719
Re: RFR: 8317979: Use TZ database style abbreviations in the CLDR locale provider [v3]
On Tue, 17 Oct 2023 16:52:12 GMT, Naoto Sato wrote: >> CLDR provides very few short names for time zones, such as PST/PDT. This >> will typically end up substituting names from the COMPAT provider. Once the >> COMPAT is removed, they will be displayed in the GMT format, i.e., >> GMT+XX:YY. Although some of the short names in the COMPAT provider are >> somewhat questionable (less common ones are simply made up from the long >> names by taking the initials), it would not be desirable for them to fall >> back to the GMT format. >> To mitigate the situation, CLDR can use the abbreviated names from the TZ >> database, which contains legacy (major) short names as FORMAT. The CLDR >> provider can use them instead of the GMT offset style. This enhancement is a >> precursor to the future removal of the COMPAT provider. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Delay populating GMT format at runtime. Reflecting review comments. Marked as reviewed by joehw (Reviewer). Thanks Naoto for the update. Sounds like a good plan to me. - PR Review: https://git.openjdk.org/jdk/pull/16206#pullrequestreview-1683235181 PR Comment: https://git.openjdk.org/jdk/pull/16206#issuecomment-1767061468
Re: RFR: 8317979: Use TZ database style abbreviations in the CLDR locale provider [v2]
On Mon, 16 Oct 2023 23:37:51 GMT, Naoto Sato wrote: >> CLDR provides very few short names for time zones, such as PST/PDT. This >> will typically end up substituting names from the COMPAT provider. Once the >> COMPAT is removed, they will be displayed in the GMT format, i.e., >> GMT+XX:YY. Although some of the short names in the COMPAT provider are >> somewhat questionable (less common ones are simply made up from the long >> names by taking the initials), it would not be desirable for them to fall >> back to the GMT format. >> To mitigate the situation, CLDR can use the abbreviated names from the TZ >> database, which contains legacy (major) short names as FORMAT. The CLDR >> provider can use them instead of the GMT offset style. This enhancement is a >> precursor to the future removal of the COMPAT provider. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Use ZoneOffset to parse offset Marked as reviewed by joehw (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/16206#pullrequestreview-1681329184
Re: RFR: 8296246: Update Unicode Data Files to Version 15.1.0 [v5]
On Tue, 19 Sep 2023 18:55:28 GMT, Naoto Sato wrote:
>> This PR is to incorporate the latest Unicode 15.1, which was released
>> yesterday. Besides the usual character data update, an upgraded
>> implementation of RegEx which reflects the Indic Conjunct Break specified in
>> the latest [Unicode Annex #29 ("Unicode Text
>> Segmentation")](https://unicode.org/reports/tr29/) is included. A
>> corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review comments
Marked as reviewed by joehw (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/15728#pullrequestreview-1634146771
Re: RFR: 8296246: Update Unicode Data Files to Version 15.1.0 [v5]
On Tue, 19 Sep 2023 18:55:28 GMT, Naoto Sato wrote:
>> This PR is to incorporate the latest Unicode 15.1, which was released
>> yesterday. Besides the usual character data update, an upgraded
>> implementation of RegEx which reflects the Indic Conjunct Break specified in
>> the latest [Unicode Annex #29 ("Unicode Text
>> Segmentation")](https://unicode.org/reports/tr29/) is included. A
>> corresponding CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reflecting review comments
Marked as reviewed by joehw (Reviewer).
-
PR Review: https://git.openjdk.org/jdk/pull/15728#pullrequestreview-1634146771
Re: RFR: 8296246: Update Unicode Data Files to Version 15.1.0 [v3]
On Mon, 18 Sep 2023 18:33:20 GMT, Naoto Sato wrote:
>> This PR is to incorporate the latest Unicode 15.1, which was released
>> yesterday. Besides the usual character data update, an upgraded
>> implementation of RegEx which reflects the Indic Conjunct Break specified in
>> the latest [Unicode Annex #29 ("Unicode Text
>> Segmentation")](https://unicode.org/reports/tr29/) is included. A
>> corresponding CSR has also been drafted.
>
> Naoto Sato 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 10 additional commits since
> the last revision:
>
> - Fix GensrcRegex.gmk
> - Merge branch 'master' into JDK-8296246-Unicode15.1
> - Update
> make/jdk/src/classes/build/tools/generateextraproperties/GenerateExtraProperties.java
>
>Co-authored-by: Andrey Turbanov
> - Update
> make/jdk/src/classes/build/tools/generateextraproperties/GenerateExtraProperties.java
>
>Co-authored-by: Andrey Turbanov
> - TR29 final version
> - .md file update
> - Final 8/28
> - Draft 8/11
> - GenerateExtraProperties tool
> - initial commit
LGTM. A couple of minor suggestions.
make/jdk/src/classes/build/tools/generateextraproperties/GenerateExtraProperties.java
line 41:
> 39: /**
> 40: * Parses extra properties files of UCD, and replaces the placeholders in
> 41: * the given template source file with the generated conditions, then emit
s/emit/emits?
make/jdk/src/classes/build/tools/generateextraproperties/GenerateExtraProperties.java
line 51:
> 49: *
> 50: * %%%Type(=Value)%%%
> 51: *
Nice example to explain how it works!
I wonder if it might be helpful to clarify that the () indicate optional. It
could be slightly confused with being a part of the template.
make/modules/java.base/gensrc/GensrcRegex.gmk line 35:
> 33: INDICCONJUNCTBREAKTEMP :=
> $(MODULE_SRC)/share/classes/jdk/internal/util/regex/IndicConjunctBreak.java.template
> 34: INDICCONJUNCTBREAKPROPS :=
> $(MODULE_SRC)/share/data/unicodedata/DerivedCoreProperties.txt
> 35: INDICCONJUNCTBREAKPARAMS := InCB=Linker InCB=Extend InCB=Consonant
DerivedCoreProperties.txt is very large. Would it be helpful to reference the
section, that is "Derived Property: Indic_Conjunct_Break", in this file or the
template?
-
Marked as reviewed by joehw (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15728#pullrequestreview-1633706039
PR Review Comment: https://git.openjdk.org/jdk/pull/15728#discussion_r1330390702
PR Review Comment: https://git.openjdk.org/jdk/pull/15728#discussion_r1330408135
PR Review Comment: https://git.openjdk.org/jdk/pull/15728#discussion_r1330410613
Re: RFR: 8303039: Utilize `coverageLevels.txt` [v2]
On Thu, 2 Mar 2023 01:03:20 GMT, Naoto Sato wrote: >> This is a pre-requisite for supporting CLDR v43, where they combine `seeds` >> locales with `common` locales >> (https://cldr.unicode.org/index/downloads/cldr-43#h.7s25aqdv767e). In order >> to have the same coverage level of locales, CLDRConverter tool needs to comb >> through the locale files based on the `coverageLevels.txt` file, (and the >> ones we already included as of v42). Confirmed the same set of locales is >> generated before and after this modification. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Explicitly filter coverage levels Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12812
Re: RFR: 8298808: Check `script` code on detecting the base locales [v2]
On Wed, 14 Dec 2022 23:47:43 GMT, Naoto Sato wrote: >> Fixing `CLDRConverter` build tool to not regard [new >> locales](https://github.com/unicode-org/cldr/pull/2508/files#diff-94cbefde02914335da884f768063a06a84ad651ee4e56cdb1cb90625d7776731) >> introduced in CLDR 43 as one of the base locales. Otherwise they would >> incorrectly go into `java.base` module, then a test case fails. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Allow Latin script by default Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.org/jdk/pull/11684
Re: RFR: 8284840: Update CLDR to Version 42.0
On Fri, 21 Oct 2022 16:55:28 GMT, Naoto Sato wrote:
> This is to update the CLDR data from version 41 to version 42. The vast
> majority of the changes are basically replacing the CLDR data, along with
> tools/testcase alignments to those upstream changes:
>
> https://unicode-org.atlassian.net/browse/CLDR-14032 (" at " is no longer used
> for standard date/time format)
> https://unicode-org.atlassian.net/browse/CLDR-14831 (NBSP prefixed to `a`,
> instead of a normal space )
> https://unicode-org.atlassian.net/browse/CLDR-11510 (Fix first day of week
> info for China (CN))
> https://unicode-org.atlassian.net/browse/CLDR-15966 (Japanese: Support
> numbers up to 京)
>
> Here is the link to CLDR v42's release notes:
> https://cldr.unicode.org/index/downloads/cldr-42
Release notes look good to me.
-
Marked as reviewed by joehw (Reviewer).
PR: https://git.openjdk.org/jdk/pull/10820
Re: RFR: 8284840: Update CLDR to Version 42.0
On Fri, 21 Oct 2022 16:55:28 GMT, Naoto Sato wrote:
> This is to update the CLDR data from version 41 to version 42. The vast
> majority of the changes are basically replacing the CLDR data, along with
> tools/testcase alignments to those upstream changes:
>
> https://unicode-org.atlassian.net/browse/CLDR-14032 (" at " is no longer used
> for standard date/time format)
> https://unicode-org.atlassian.net/browse/CLDR-14831 (NBSP prefixed to `a`,
> instead of a normal space )
> https://unicode-org.atlassian.net/browse/CLDR-11510 (Fix first day of week
> info for China (CN))
> https://unicode-org.atlassian.net/browse/CLDR-15966 (Japanese: Support
> numbers up to 京)
>
> Here is the link to CLDR v42's release notes:
> https://cldr.unicode.org/index/downloads/cldr-42
Are the first two items (CLDR-14032, CLDR-14831) considered a behavior change
(e.g. the format string will be different) that could use a CSR or the release
notes to document it? I see the later points to the CLDR release notes, but
maybe a statement in our release notes could be clearer?
-
PR: https://git.openjdk.org/jdk/pull/10820
Re: RFR: JDK-8294618: Update openjdk.java.net => openjdk.org [v4]
On Fri, 30 Sep 2022 01:11:45 GMT, Joe Darcy wrote: >> With the domain change from openjdk.java.net to openjdk.org, references to >> URLs in the sources should be updated. >> >> Updates were made using a shell script. I"ll run a copyright updater before >> any push. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > http -> https The ones in java.xml look good to me. - Marked as reviewed by joehw (Reviewer). PR: https://git.openjdk.org/jdk/pull/10501
Re: RFR: 8294317: Insufficient build rules for tzdb.dat
On Fri, 23 Sep 2022 22:10:03 GMT, Naoto Sato wrote: > The current makefile for `tzdb.dat` won't recompile if some dependent files > (e.g. `VERSION`) are updated. Adding all files under the tzdb directory to > the dependency will fix this issue. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.org/jdk/pull/10415
