Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

Thanks. Created an issue for the follow-on issue:

https://bugs.openjdk.java.net/browse/JDK-8165804

Naoto

On 9/9/16 3:34 PM, Mandy Chung wrote:



On Sep 9, 2016, at 2:58 PM, Naoto Sato  wrote:

While at it, I noticed two build issues and updated the webrev. They are not 
directly related to the split package issue per se, but related to Thai 
breakiterator:

1) BreakIteratorRules_th.class slipped into the product image, which shouldn't.

2) Removed BreakIteratorRulesProvider.java which is not needed, as the class 
above is only used at the build time and not through module.

Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.03/


Looks fine.

It’d be good to file a follow-on issue and investigate what it takes to 
implement Masayoshi’s suggestion - have BreakIterators::getObject to return 
dictionary data instead of a resource name.

Mandy

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Mandy Chung]

> On Sep 9, 2016, at 2:58 PM, Naoto Sato  wrote:
> 
> While at it, I noticed two build issues and updated the webrev. They are not 
> directly related to the split package issue per se, but related to Thai 
> breakiterator:
> 
> 1) BreakIteratorRules_th.class slipped into the product image, which 
> shouldn't.
> 
> 2) Removed BreakIteratorRulesProvider.java which is not needed, as the class 
> above is only used at the build time and not through module.
> 
> Here is the updated webrev:
> 
> http://cr.openjdk.java.net/~naoto/8165605/webrev.03/

Looks fine.  

It’d be good to file a follow-on issue and investigate what it takes to 
implement Masayoshi’s suggestion - have BreakIterators::getObject to return 
dictionary data instead of a resource name.

Mandy

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

While at it, I noticed two build issues and updated the webrev. They are 
not directly related to the split package issue per se, but related to 
Thai breakiterator:


1) BreakIteratorRules_th.class slipped into the product image, which 
shouldn't.


2) Removed BreakIteratorRulesProvider.java which is not needed, as the 
class above is only used at the build time and not through module.


Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.03/

Naoto

On 9/9/16 7:15 AM, Erik Joelsson wrote:

Build change looks ok.

/Erik


On 2016-09-08 17:37, Naoto Sato wrote:

Updated the webrev wrt the latter comment:

http://cr.openjdk.java.net/~naoto/8165605/webrev.02/

Naoto

On 9/7/16 6:37 PM, Mandy Chung wrote:



On Sep 7, 2016, at 6:29 PM, Naoto Sato  wrote:

Hi Mandy,

Although avoiding the hardcoded pathname is good, it is specific to
the BreakIterator implementation of the COMPAT provider. So I am not
sure making a generic SPI would be needed here.


I was thinking of one of the internal services that jdk.localedata
currently provides.



Anyway, this split package issue is blocking Alan's push, so I'd
like to push the change as it is. We can get back to this later.


I agree this can be cleaned up as a separate issue.

 152 InputStream is = module.getResourceAsStream(
 153 ("jdk.localedata".equals(module.getName()) ?
 154  "sun/text/resources/ext/" :
"sun/text/resources/") + dictionaryName);

It may be easier to read if line 153-154 are moved and assign to a
separate variable.  Otherwise, looks fine.

Mandy



Naoto

On 9/7/16 5:17 PM, Mandy Chung wrote:

Hi Naoto,

Is there an alternative to get back the pathname of the resource
e.g. adding a method in existing internal SPI to avoid hardcoding
the module name and the resource pathname.

Mandy


On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:

Forgot to include jlink plugin changes. Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.01/

Naoto

On 9/7/16 3:03 PM, Naoto Sato wrote:

Please review the changes to the subject bug:

https://bugs.openjdk.java.net/browse/JDK-8165605

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8165605/webrev.00/

The change is simply to move those 3 resources under
sun.text.resources.ext package so that it won't cause the split
package
issue.

Naoto

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

The build tool is getting the output directory as a parameter and that 
also need to be tweaked. That output directory path is specified in the 
makefile which I wanted to avoid.


Naoto

On 9/8/16 4:31 PM, Masayoshi Okutsu wrote:

Is it just a matter of an extra step, new File(path).getName()?

Masayoshi


On 9/9/2016 12:00 AM, Naoto Sato wrote:

Well, actually I tried this approach first, then found out that the
data files are also used by the GenerateBreakIteratorData build tool
which has some implicit assumption of the value being the file name
w/o path. So I ended up with the fix.

Naoto

On 9/8/16 5:46 AM, Alan Bateman wrote:



On 08/09/2016 03:51, Masayoshi Okutsu wrote:

I thought Mandy suggested that the dictionary names in a
ResourceBundle contain path names rather than base names, something
like this:

In BreakIteratorInfo_th.java:

{"WordDictionary", "thai_dict"},
to
{"WordDictionary", "sun/text/resources/ext/thai_dict"},

I haven't checked any side effects of this change, though.

That would be cleaner, assuming it doesn't cause issues.

-Alan

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Erik Joelsson]

Build change looks ok.

/Erik


On 2016-09-08 17:37, Naoto Sato wrote:

Updated the webrev wrt the latter comment:

http://cr.openjdk.java.net/~naoto/8165605/webrev.02/

Naoto

On 9/7/16 6:37 PM, Mandy Chung wrote:



On Sep 7, 2016, at 6:29 PM, Naoto Sato  wrote:

Hi Mandy,

Although avoiding the hardcoded pathname is good, it is specific to 
the BreakIterator implementation of the COMPAT provider. So I am not 
sure making a generic SPI would be needed here.


I was thinking of one of the internal services that jdk.localedata 
currently provides.




Anyway, this split package issue is blocking Alan's push, so I'd 
like to push the change as it is. We can get back to this later.


I agree this can be cleaned up as a separate issue.

 152 InputStream is = module.getResourceAsStream(
 153 ("jdk.localedata".equals(module.getName()) ?
 154  "sun/text/resources/ext/" : 
"sun/text/resources/") + dictionaryName);


It may be easier to read if line 153-154 are moved and assign to a 
separate variable.  Otherwise, looks fine.


Mandy



Naoto

On 9/7/16 5:17 PM, Mandy Chung wrote:

Hi Naoto,

Is there an alternative to get back the pathname of the resource 
e.g. adding a method in existing internal SPI to avoid hardcoding 
the module name and the resource pathname.


Mandy


On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:

Forgot to include jlink plugin changes. Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.01/

Naoto

On 9/7/16 3:03 PM, Naoto Sato wrote:

Please review the changes to the subject bug:

https://bugs.openjdk.java.net/browse/JDK-8165605

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8165605/webrev.00/

The change is simply to move those 3 resources under
sun.text.resources.ext package so that it won't cause the split 
package

issue.

Naoto

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Masayoshi Okutsu]

Is it just a matter of an extra step, new File(path).getName()?

Masayoshi


On 9/9/2016 12:00 AM, Naoto Sato wrote:
Well, actually I tried this approach first, then found out that the 
data files are also used by the GenerateBreakIteratorData build tool 
which has some implicit assumption of the value being the file name 
w/o path. So I ended up with the fix.


Naoto

On 9/8/16 5:46 AM, Alan Bateman wrote:



On 08/09/2016 03:51, Masayoshi Okutsu wrote:

I thought Mandy suggested that the dictionary names in a
ResourceBundle contain path names rather than base names, something
like this:

In BreakIteratorInfo_th.java:

{"WordDictionary", "thai_dict"},
to
{"WordDictionary", "sun/text/resources/ext/thai_dict"},

I haven't checked any side effects of this change, though.

That would be cleaner, assuming it doesn't cause issues.

-Alan

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

Updated the webrev wrt the latter comment:

http://cr.openjdk.java.net/~naoto/8165605/webrev.02/

Naoto

On 9/7/16 6:37 PM, Mandy Chung wrote:



On Sep 7, 2016, at 6:29 PM, Naoto Sato  wrote:

Hi Mandy,

Although avoiding the hardcoded pathname is good, it is specific to the 
BreakIterator implementation of the COMPAT provider. So I am not sure making a 
generic SPI would be needed here.


I was thinking of one of the internal services that jdk.localedata currently 
provides.



Anyway, this split package issue is blocking Alan's push, so I'd like to push 
the change as it is. We can get back to this later.


I agree this can be cleaned up as a separate issue.

 152 InputStream is = module.getResourceAsStream(
 153 ("jdk.localedata".equals(module.getName()) ?
 154  "sun/text/resources/ext/" : "sun/text/resources/") + 
dictionaryName);

It may be easier to read if line 153-154 are moved and assign to a separate 
variable.  Otherwise, looks fine.

Mandy



Naoto

On 9/7/16 5:17 PM, Mandy Chung wrote:

Hi Naoto,

Is there an alternative to get back the pathname of the resource e.g. adding a 
method in existing internal SPI to avoid hardcoding the module name and the 
resource pathname.

Mandy


On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:

Forgot to include jlink plugin changes. Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.01/

Naoto

On 9/7/16 3:03 PM, Naoto Sato wrote:

Please review the changes to the subject bug:

https://bugs.openjdk.java.net/browse/JDK-8165605

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8165605/webrev.00/

The change is simply to move those 3 resources under
sun.text.resources.ext package so that it won't cause the split package
issue.

Naoto

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

Well, actually I tried this approach first, then found out that the data 
files are also used by the GenerateBreakIteratorData build tool which 
has some implicit assumption of the value being the file name w/o path. 
So I ended up with the fix.


Naoto

On 9/8/16 5:46 AM, Alan Bateman wrote:



On 08/09/2016 03:51, Masayoshi Okutsu wrote:

I thought Mandy suggested that the dictionary names in a
ResourceBundle contain path names rather than base names, something
like this:

In BreakIteratorInfo_th.java:

{"WordDictionary", "thai_dict"},
to
{"WordDictionary", "sun/text/resources/ext/thai_dict"},

I haven't checked any side effects of this change, though.

That would be cleaner, assuming it doesn't cause issues.

-Alan

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Alan Bateman]

On 08/09/2016 03:51, Masayoshi Okutsu wrote:
I thought Mandy suggested that the dictionary names in a 
ResourceBundle contain path names rather than base names, something 
like this:


In BreakIteratorInfo_th.java:

{"WordDictionary", "thai_dict"},
to
{"WordDictionary", "sun/text/resources/ext/thai_dict"},

I haven't checked any side effects of this change, though.

That would be cleaner, assuming it doesn't cause issues.

-Alan

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Mandy Chung]

> On Sep 7, 2016, at 6:29 PM, Naoto Sato  wrote:
> 
> Hi Mandy,
> 
> Although avoiding the hardcoded pathname is good, it is specific to the 
> BreakIterator implementation of the COMPAT provider. So I am not sure making 
> a generic SPI would be needed here.

I was thinking of one of the internal services that jdk.localedata currently 
provides.

> 
> Anyway, this split package issue is blocking Alan's push, so I'd like to push 
> the change as it is. We can get back to this later.

I agree this can be cleaned up as a separate issue.

 152 InputStream is = module.getResourceAsStream(
 153 ("jdk.localedata".equals(module.getName()) ? 
 154  "sun/text/resources/ext/" : "sun/text/resources/") + 
dictionaryName);

It may be easier to read if line 153-154 are moved and assign to a separate 
variable.  Otherwise, looks fine.

Mandy

> 
> Naoto
> 
> On 9/7/16 5:17 PM, Mandy Chung wrote:
>> Hi Naoto,
>> 
>> Is there an alternative to get back the pathname of the resource e.g. adding 
>> a method in existing internal SPI to avoid hardcoding the module name and 
>> the resource pathname.
>> 
>> Mandy
>> 
>>> On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:
>>> 
>>> Forgot to include jlink plugin changes. Here is the updated webrev:
>>> 
>>> http://cr.openjdk.java.net/~naoto/8165605/webrev.01/
>>> 
>>> Naoto
>>> 
>>> On 9/7/16 3:03 PM, Naoto Sato wrote:
 Please review the changes to the subject bug:
 
 https://bugs.openjdk.java.net/browse/JDK-8165605
 
 The proposed fix is located at:
 
 http://cr.openjdk.java.net/~naoto/8165605/webrev.00/
 
 The change is simply to move those 3 resources under
 sun.text.resources.ext package so that it won't cause the split package
 issue.
 
 Naoto
>> 

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

Hi Mandy,

Although avoiding the hardcoded pathname is good, it is specific to the 
BreakIterator implementation of the COMPAT provider. So I am not sure 
making a generic SPI would be needed here.


Anyway, this split package issue is blocking Alan's push, so I'd like to 
push the change as it is. We can get back to this later.


Naoto

On 9/7/16 5:17 PM, Mandy Chung wrote:

Hi Naoto,

Is there an alternative to get back the pathname of the resource e.g. adding a 
method in existing internal SPI to avoid hardcoding the module name and the 
resource pathname.

Mandy


On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:

Forgot to include jlink plugin changes. Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.01/

Naoto

On 9/7/16 3:03 PM, Naoto Sato wrote:

Please review the changes to the subject bug:

https://bugs.openjdk.java.net/browse/JDK-8165605

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8165605/webrev.00/

The change is simply to move those 3 resources under
sun.text.resources.ext package so that it won't cause the split package
issue.

Naoto

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Mandy Chung]

Hi Naoto,

Is there an alternative to get back the pathname of the resource e.g. adding a 
method in existing internal SPI to avoid hardcoding the module name and the 
resource pathname.

Mandy

> On Sep 7, 2016, at 3:56 PM, Naoto Sato  wrote:
> 
> Forgot to include jlink plugin changes. Here is the updated webrev:
> 
> http://cr.openjdk.java.net/~naoto/8165605/webrev.01/
> 
> Naoto
> 
> On 9/7/16 3:03 PM, Naoto Sato wrote:
>> Please review the changes to the subject bug:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8165605
>> 
>> The proposed fix is located at:
>> 
>> http://cr.openjdk.java.net/~naoto/8165605/webrev.00/
>> 
>> The change is simply to move those 3 resources under
>> sun.text.resources.ext package so that it won't cause the split package
>> issue.
>> 
>> Naoto

Re: [9] RFR: 8165605: Thai resources in jdk.localedata cause split package issue with java.base

[Naoto Sato]

Forgot to include jlink plugin changes. Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8165605/webrev.01/

Naoto

On 9/7/16 3:03 PM, Naoto Sato wrote:

Please review the changes to the subject bug:

https://bugs.openjdk.java.net/browse/JDK-8165605

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8165605/webrev.00/

The change is simply to move those 3 resources under
sun.text.resources.ext package so that it won't cause the split package
issue.

Naoto