Review for the jigsaw module build change
Mandy.Chung at Sun.COM
Tue Feb 2 22:45:17 PST 2010
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).
> - The fix to BootLoader.c to build on Windows probably should probably
> check if path is NULL before getting the chars.
> - 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.
More information about the jigsaw-dev