Review for the jigsaw module build change
Alan.Bateman at Sun.COM
Tue Feb 2 14:10:00 PST 2010
Mandy Chung wrote:
> Webrev at:
> Summary of my current set of changes:
> o modules makefile target will create 4 jigsaw images:
> jdk-base-image and jre-base-image
> jdk-module-image and jre-module-image (full JDK and JRE)
> o modularize the jdk to generate jdk7 base module and
> a number of other modules defined in modules.config and
> The granularity of the modules will be sorted out in the future.
> The current list is all fine-grained modules.
> o All jdk tools except javac and jmod are now running in module
> mode. javac will be changed to run in module mode when Jon's
> change is in. I'll have to look into the jmod bootstrap
> issue to determine if we can make it run in module mode.
> o ClassAnalyzer to generate the module-info.java that's
> where most of my time was spent in generating the proper
> module dependencies so as to get the jdk modules installed
> o jprt modules build successfully on all platforms
> I fixed a couple issues in the
> jdk/make/java/java/mapfile-vers, and org/openjdk/jigsaw/BootLoader.c.
> o Ran test/jdk/java/* jtreg tests on solaris-i586 and linux-amd64
> in legacy mode. Will run jtreg tests on windows.
> o Sync'ed jigsaw repo with jdk 7 b81
> I'd like to push this change to the jigsaw repo tomorrow.
> Alan, do you have cycle to take a look at this webrev?
> There are more to do as listed in make/common/Modules.gmk but
> I think it's good to push this change out so that all of us can
> play with the modular jdk.
This is good work - you deserve a medal!
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.
- 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 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. 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?
That's all I have for now.
More information about the jigsaw-dev