RFR: JDK-8189094: Change required boot jdk to JDK 9

Erik Joelsson erik.joelsson at oracle.com
Tue Oct 17 13:59:06 UTC 2017


New webrev:


On 2017-10-17 13:10, Magnus Ihse Bursie wrote:
>
> This is a quite complex change. It's a bit unfortunate that we have to 
> make these kinds of changes to use JDK 9 as a boot JDK. Anyway, that's 
> the way it is.
>
> A couple of remarks:
>
> * In boot-jdk.m4, please update the comment as well:
>     # When compiling code to be executed by the Boot JDK, force jdk8 compatibility.
> - BOOT_JDK_SOURCETARGET="-source 8 -target 8"
> + BOOT_JDK_SOURCETARGET="-source 9 -target 9"
Fixed
> * In JavaCompilation.gmk:
> + # exist yet, is in it.
> + $$(foreach d,$$($1_SRC), \
Fixed
> You can add a space after the comma with no ill effects.
> - $$(eval $$(call FillCacheFind,$$($1_SRC)))
> + $$(eval $$(call FillCacheFind, $$(wildcard $$($1_SRC))))
> Should not the $(wildcard) filtering be done in the FillCacheFind 
> function? It seems reasonable that it should not complain with an 
> error if you give it a non-existing directory.
> - $1_ALL_SRCS += $$(foreach s, $$($1_SRC), $$(call CacheFind, $$(s)))
> + $1_ALL_SRCS += $$($1_EXTRA_FILES) $$(foreach s, $$($1_SRC), $$(if 
> $$(wildcard $$s), \
> + $$(call CacheFind, $$s)))
>
> The same goes here. Any wildcard filtering should be done in 
> CacheFind, I think.
>
I looked at CacheFind and figured it didn't already do wildcard 
consistently so opted to change the call site. In the new patch I added 
wildcard checking in all instances of CacheFind instead. This means we 
likely have places where we can remove redundant wildcard calls in other 
places. Removing those in this change is starting to feel like too much 
however. I can file a followup.
> * In spec.gmk.in, the code for INTERIM_LANGTOOLS_MODULES_COMMA is 
> basically repeating the pre-existing macro CommaList. However, that is 
> only available in MakeBase.gmk. I'm not sure it's possible to use, but 
> maybe. It looks like there's a lot of hard-coded stuff in spec.gmk.in, 
> and maybe that is not the right place for it. Where are  
> INTERIM_LANGTOOLS_ARGS used?
The SPEC is always included first, so the CommaList macro is not 
available. I agree that the spec is not necessarily the optimal place to 
put static things like this, but we don't really have a Common or Defs 
to put such things in. The args are used whenever we need to run the 
interim langtools, including compiling java, running javadoc and some 
gensrc/gendata. Some of the other variables are needed when compiling 
the interim modules.

In my defense I'm not changing the location of these settings in the patch.
>
> * In jib-profiles.js:
> You have no "var" declaration for the new boot_jdk_version etc. I'm 
> not too well-versed in javascript and it is probably quite legal not 
> to, but I think it's better style at least to keep it.
Fixed
>
> * Finally, $(BUILDTOOLS_OUTPUTDIR)/override_modules should be renamed 
> "interim_modules".
>
Fixed

/Erik



More information about the build-dev mailing list