RFR: JDK-8168108: lib/classlist should be packaged in java.base.jmod

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Mon Nov 7 12:40:56 UTC 2016


Hi,

The webrev haven't picked up the relationship between the LinkOpt and 
Classlist files. It seems that it is a rename + modifications. Can you 
please redo it using hg mv? And update the webrev so the changes in the 
file is visible.

The name "LinkOptData", is it supposed to be read "link optional data"? 
I'm not sure in what way it is an improvement of Classlist (even though 
this name was no good either).

Why do you pass JMODS_DIR and _TEMPDIR in Main.gmk to CreateJmods.gmk? I 
understand that you need to specify the module and that we're building 
interim jmods, but the rest seems to be redundant.

/Magnus

On 2016-11-03 13:01, Erik Joelsson wrote:
> Hello,
>
> New webrev: http://cr.openjdk.java.net/~erikj/8168108/webrev.02/
>
>
> On 2016-11-02 18:52, Mandy Chung wrote:
>>> On Nov 2, 2016, at 9:55 AM, Erik Joelsson <erik.joelsson at oracle.com> 
>>> wrote:
>>>
>>> This patch changes how the dynamically generated lib/classlist is 
>>> packaged. We already create an interim-image from java.base and 
>>> java.logging to generate the classlist during the build. We then 
>>> copy it into the jdk and jre images using makefile logic after 
>>> jlinking those images. With this patch, lib/classlist is instead 
>>> included in java.base.jmod.
>>>
>>> To achieve this, I have introduced the concept of interim jmods for 
>>> the jmods used to create the interim-image. These can be built 
>>> explicitly from the top level using targets <module>-interim-jmod. 
>>> It adds a little bit of build time, but it's hardly noticeable.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8168108
>>>
>>> Webrev: http://cr.openjdk.java.net/~erikj/8168108/webrev.01/
>> I like the interim jmods idea.  The solution is clean.
>>
>> jdk/make/GenerateClasslist.gmk
>>    62         $(call MakeDir, $(@D) $(SUPPORT_OUTPUTDIR)/classlist)
>>
>> Is the directory $(SUPPORT_OUTPUTDIR)/classlist needed?  I can’t tell 
>> why this line needs to be changed.
> Yes it was, since jli_trace.out is also generated by this target. I 
> realize this becomes a bit convoluted so renamed the output dir, the 
> makefile and the top level target to reflect that more things than 
> classlist are generated here. I fixed the make rules so that both the 
> classlist and jli_trace.out works correctly if either is removed 
> before a rebuild.
>>
>>    63         $(call LogInfo, Generating lib/classlist)
>>
>> It might be better to print $(CLASSLIST_FILE) in the log instead?
> Fixed the log messages.
>
> /Erik
>> Otherwise, looks fine.
>>
>> Mandy
>




More information about the build-dev mailing list