Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-17 Thread Naoto Sato
On Fri, 17 Mar 2023 16:56:16 GMT, Adam Sotona wrote: >> The fix to `IncludeLocalesPlugin` has been integrated. @asotona would you >> please check that the exception is not thrown with your changes? > > Yes, I can confirm the test now passes. Great! Thanks for checking - PR: https:

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-17 Thread Adam Sotona
On Fri, 17 Mar 2023 16:06:53 GMT, Naoto Sato wrote: >> I've reverted `IncludeLocalesPlugin` to previous version, plus minor fixes >> and together with #13067 the `IncludeLocalesPluginTest` is passing. >> Thank you. > > The fix to `IncludeLocalesPlugin` has been integrated. @asotona would you >

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-17 Thread Naoto Sato
On Fri, 17 Mar 2023 11:35:52 GMT, Adam Sotona wrote: >> Submitted: https://github.com/openjdk/jdk/pull/13067 > > I've reverted `IncludeLocalesPlugin` to previous version, plus minor fixes > and together with #13067 the `IncludeLocalesPluginTest` is passing. > Thank you. The fix to `IncludeLocal

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-17 Thread Adam Sotona
On Thu, 16 Mar 2023 20:45:24 GMT, Naoto Sato wrote: >> I further clarified with Naoto and I now understand the problem.The >> plugin intends to transform `.class` that implements `LocaleDataMetaInfo` in >> `jdk.localedata` module. It's a bug in the plugin `--include-locales=*` >> which w

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Naoto Sato
On Thu, 16 Mar 2023 18:40:16 GMT, Mandy Chung wrote: >> There is no common binary format for "localedata", the file in question is >> the custom format specific for `BreakIterator`, ancient code from Taligent. >> IIUC, only description of the format is in the source: >> https://github.com/open

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Mandy Chung
On Thu, 16 Mar 2023 17:00:59 GMT, Naoto Sato wrote: >> @naotoj as these localedata resource files are not `.class`, the plugin >> should not use ASM (or Classfile API) to parse it but ASM ClassReader just >> happens to work. Is the format documented anywhere? >> >> @asotona I agree that thi

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Naoto Sato
On Thu, 16 Mar 2023 16:18:46 GMT, Mandy Chung wrote: >> The file `LineBreakIteratorData_th` is not a class file, thus the exception >> is appropriate. This plugin test is trying to verify the integrity of the >> generated jimage, not only the required class files for the specified >> locale, b

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Mandy Chung
On Thu, 16 Mar 2023 16:06:15 GMT, Naoto Sato wrote: >> This plugin does not fit to the Classfile API use cases portfolio at all. >> Here it is only needed to get interfaces list out of the constant pool and >> then the Utf8 CP entries are in-place modified. That is out of the Classfile >> API

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Naoto Sato
On Thu, 16 Mar 2023 07:51:25 GMT, Adam Sotona wrote: >> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/IncludeLocalesPlugin.java >> line 160: >> >>> 158: >>> resource.type().equals(ResourcePoolEntry.Type.CLASS_OR_RESOURCE)) { >>> 159: byte[

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Adam Sotona
On Wed, 15 Mar 2023 22:31:01 GMT, Mandy Chung wrote: > What was the issue with the previous revision? The only conversion bug I found during the code reformat/revision was missing `className.replace('/', '.')` and after that the test pass. https://github.com/openjdk/jdk/blob/49c80bcd9ad6ddd85cff

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-16 Thread Adam Sotona
On Thu, 16 Mar 2023 02:43:39 GMT, Mandy Chung wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> fixed SystemModulesPlugin > > src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/IncludeLocalesPlugin.java > l

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-15 Thread Mandy Chung
On Wed, 15 Mar 2023 16:42:53 GMT, Adam Sotona wrote: >> jdk.jlink internal plugins are heavily using ASM >> >> This patch converts ASM calls to Classfile API. >> >> Please review. >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since t

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-15 Thread Mandy Chung
On Wed, 15 Mar 2023 16:42:53 GMT, Adam Sotona wrote: >> jdk.jlink internal plugins are heavily using ASM >> >> This patch converts ASM calls to Classfile API. >> >> Please review. >> Thanks, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since t

Re: RFR: 8294972: Convert jdk.jlink internal plugins to use the Classfile API [v8]

2023-03-15 Thread Adam Sotona
> jdk.jlink internal plugins are heavily using ASM > > This patch converts ASM calls to Classfile API. > > Please review. > Thanks, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: fixed SystemModulesPlugin - Chan