Hello Mandy,
Looks good. Just noticed another thing in CheckModules.gmk. You can
remove the explicit FIXPATH as the variable JAVA already contains it. In
general, only spec.gmk should need to worry about FIXPATH.
If there are no other changes, I don't need to see another round before
you push.
/Erik
On 2014-08-28 19:21, Mandy Chung wrote:
On 8/27/14 11:38 PM, Erik Joelsson wrote:
Hello Mandy,
That certainly looks better. A couple of more thoughts, and sorry for
not pointing this out earlier, but the new structure is still new to
me too.
* The rmic targets also generate classes, so for modules.xml to be
correct, I suspect you need to depend on that too. Simply add "rmic"
after java on the dependency line. I assume the verification doesn't
care about resources? If it does, then you would also need to depend
on the rest of gendata, something like $(filter-out jdk.dev-gendata,
$(GENDATA_TARGETS)).
Good catch. rmic needs to be added in the dependency. jdeps verifies
class files only and doesn't care about resources.
* In Gendata-jdk.dev.gmk, there is an ifndef OPENJDK. We are trying
to move away from that construct when possible. It's a bit cumbersome
but to avoid it. To do it in the current model, create a closed
version of Gendata-jdk.dev.gmk. Add "$(eval $(call
IncludeCustomExtension, jdk, gendata/Gendata-jdk.dev.gmk))" after
"include GendataCommon.gmk". Change the first assignment of
METADATA_FILES to += and move the closed addition to the closed
version of the file. There is no need for ifndef OPENJDK in the
closed file. It only gets included when we build closed.
That's another good change in the build.
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.02/
I also moved jdk/make/CheckModules.gmk to top/make/CheckModules.gmk
per Magnus's suggestion.
Mandy
/Erik
On 2014-08-27 18:00, Mandy Chung wrote:
Erik, Magnus,
This is much easier than I have thought. I really like this new build.
I have separated out Gendata-jdk.dev.gmk and removed the modules-xml
target completely.
Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.01/
Mandy
