RFR: JDK-8189094: Change required boot jdk to JDK 9
erik.joelsson at oracle.com
Tue Oct 17 13:59:06 UTC 2017
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"
> * In JavaCompilation.gmk:
> + # exist yet, is in it.
> + $$(foreach d,$$($1_SRC), \
> 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
> to, but I think it's better style at least to keep it.
> * Finally, $(BUILDTOOLS_OUTPUTDIR)/override_modules should be renamed
More information about the build-dev