RFR: JDK-8189094: Change required boot jdk to JDK 9
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Wed Oct 18 08:34:10 UTC 2017
On 2017-10-17 15:59, Erik Joelsson wrote:
> 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"
>> * 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.
Please do. I made a quick check, and it looks like it's at least 10
places we can simplify.
>> * 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
Fair enough. Maybe we should have a make/common/Constants.gmk or
something like that which is included first in the spec?
>> * 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
Your fixed webrev looks good to me.
More information about the build-dev