Review Request: 8173381: osName/osArch/osVersion is missing in ModuleDescriptor created by SystemModules

Mandy Chung mandy.chung at oracle.com
Sat Jan 28 20:50:44 UTC 2017


Updated webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173381/webrev.02

> On Jan 28, 2017, at 10:01 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 27/01/2017 06:21, Mandy Chung wrote:
> 
>> :
>> Updated webrev:
>>   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8173381/webrev.01/
>> 
> This mostly looks good.
> 
> I think it would cleaner if the plugin used rewriter.targetPlatform("", "", "") rather than adding filtering to the extender.
> 

I agree that would be cleaner and made the change. That’s what I started with but that’d leave ModuleTarget attribute in the class file. 

> For the secret option for testing then I assume it should be "retainModuleTarget" rather than "retainTargetPlatform". There are a couple of places that use the old name in method names and maybe we should rename those too. One other rename is the plugin has "PackagesAttribute" when it should be "ModulePackages" attribute.

I fixed up a few places in ModuleInfoExtender as well.

Mandy



More information about the jigsaw-dev mailing list