RFR  Modular Source Code
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Wed Aug 20 10:14:53 UTC 2014
On 2014-08-15 10:52, Magnus Ihse Bursie wrote:
> These are indeed the single most significant changes to the build
> system since the "new" build system was introduced.
> Here follows a partial review of the changes in the jdk repo. Even
> though it's long, I'm not done. ;-)
While this has turned into a post-factum code review, I continue
The fix from 8026773 was inadvertently reverted in the following line:
$(ECHO) Compiling $(words $(PROPSOURCES) v1 v2 v3) properties into
This might implicate a merge error at some point, so maybe the file
should be more thoroughly studied if there are more parts of the fix for
8026773 that has been lost.
The following stanza is incorrectly indented:
# Yep, there are files to be copied!
$$(foreach i,$$($1_ALL_COPIES),$$(eval $$(call
# Now we can depend on $$($1_ALL_COPY_TARGETS) to copy all files!
## Service types are required in the classpath when compiing
The ListPathsSafely support files have changed, replacing the first
three strings with:
While the old replacements did not do anything useful ("shortening"
three-letter strings into another three-letter string), this change
breaks the old pattern of having the strings sorted in length order. At
first, I thought it was just a matter of style (I still think this is a
matter of style, and that it should be fixed) but then I realized there
also was a reason for this:
The strings are matched from longest to shortest (given, of course,
that the list is correctly ordered) and thus the string "com/sun" will
match and be replaced by X07 before the new string "com/sun/org" will be
given the chance.
In corba/make and langtools/make:
For clarity and consistency, the file corba/make/CompileCorba.gmk
should be renamed CompileInterimCorba.gmk and
langtools/make/CompileInterim.gmk should be renamed
I'm not entirely happy with the way the following dependencies are
# Declare dependencies from all other <module>-lib to java.base-lib
$(foreach t, $(filter-out java.base-libs, $(LIB_TARGETS)), \
$(eval $t: java.base-libs))
# Declare the special case dependency for jdk.deploy.osx where libosx
# links against libosxapp.
# This dependency needs to be explicitly declared. jdk.jdi-gensrc
# header file used by jdk.jdwp libs.
I have discussed this off-line with Erik and concluded that this
probably has to do for now, but I believe there should be a better way
of describing this kinds of relationships.
Alright, now I'm done code reviewing! :-) Lots of kudos to Erik for
managing to re-create the build system for the modular source code with
such high quality.
More information about the build-dev