Review for the jigsaw module build change

[Mandy Chung]

Alan Bateman wrote:
> This is good work - you deserve a medal!
>

Thanks.  I'm happy to see the jdk modules running on jigsaw!

> I've skimmed through all the changes and it looks good enough to push 
> to the jigsaw/jdk repo. We will need to do a more complete review when 
> it comes to pushing this to jdk7.
>
> Some minor comments/questions:
>
> - The launcher changes hard code "7-ea". No problem for now but I'm 
> sure we can pass in the version number during the build.
>

Yes, we should fix this.

> - In Defs.gmk and launchers/Makefile it would be good to document the 
> new MODULE argument (the other arguments are documented).
>

Will do.

> - The fix to BootLoader.c to build on Windows probably should probably 
> check if path is NULL before getting the chars.
>

Fixed.

> - The updates to the class analyzer are okay for now but we might want 
> to re-visit this soon as it hard codes the names of some of the 
> platform modules and has other logic to generate aggregate modules for 
> each of the sun modules that I'm sure we will need to change.
>
> - In modules.group the definitions of the boot and base modules 
> include org.openjdk.jigsaw.Hi that assume need to be removed before 
> you push this.
>

I leave it there for testing purpose.   We will remove it in the future.
> - I didn't understand the dependency issue with pack200's command line 
> interface - this seems to be more than changing the Driver class to 
> public so that the tool can become a module. 

javac doesn't allow a package-private class referenced in the 
module-info.java but defined in another module (I guess maybe the 
dependency is local, javac might allow it - I haven't tried).  Making it 
public is just a temporary solution for now.  We have to revisit this 
issue to determine the right solution for it.

> On the tools, I was surprised to see a module per tool. If a module 
> support multiple entry points then does this need go away?

I think it's fine to have one module per tool.   I presume that we don't 
need one module per tool when a module support multiple entry points.  
We'll revisit this at that time.
>
> That's all I have for now.

Thanks
Mandy