RFR: JDK-8190484 Move jvm.h, jmm.h et al to hotspot/*/include

[Magnus Ihse Bursie]

On 2017-12-05 00:28, David Holmes wrote:
> Hi Magnus,
>
> There may be a further wart here to resolve. 
But of course...

> From the "bump the classfile version to 54" thread on jdk-dev:
>
> ---
>
> >>>> Don't you also need to update:
> >>>>
> >>>> jdk/src/java.base/share/native/include/classfile_constants.h
> >>>>
> >>>> #define JVM_CLASSFILE_MAJOR_VERSION 53
> >>>>
> >>> I cannot find any usages for this constant, nor 
> JVM_CLASSFILE_MINOR_VERSION. I will remove them.
> >>
> >> Okay. I don't know the history or use of this file, other than it 
> gets included into jvm.h to export the jvm interface to the jdk.
> >>
> >
>> And classfile_constants.h is also distributed with the image. I am
>> unsure of the intent/history. To play it safe i will just bump the
>> number.
> Hmmm - that seems wrong. jvm.h is not an exported external interface 
> AFAIK. And we just moved it so I don't think it will get distributed 
> any more. Hmm that also suggests that classfile_constants.h may be in 
> the wrong place ... I'll take this up elsewhere.
>
> ---
>
> So is classfile_constants.h also in the wrong place? And should it be 
> in the image ??

It sounds like it. :( It's only included from jvm.h (and 
src/java.base/share/native/libverify/check_code.c) as far as I can tell. 
So it should probably move with jvm.h.

The bad news is that I just pushed JDK-8190484. (To jdk/hs; Jesper 
promised me to make sure it ended up in jdk/jdk before RDP1). The good 
news is that just moving classfile_constants.h is probably simple, I 
assume all include paths are already correct.

Can you open a new bug (and/or handle it all by yourself)?

/Magnus

>
> David
>
> On 5/12/2017 6:40 AM, Magnus Ihse Bursie wrote:
>> On 2017-12-04 19:17, mandy chung wrote:
>>>
>>>
>>> On 12/4/17 9:33 AM, Erik Joelsson wrote:
>>>> Hello Magnus,
>>>>
>>>> The <module>-copy targets are currently only being generated for 
>>>> modules that have make/copy/Copy-<module>.gmk makefiles present. By 
>>>> removing make/copy/Copy-jdk.accessibility.gmk and 
>>>> make/copy/Copy-jdk.jdwp.agent.gmk, those targets are no longer 
>>>> created so the logic in CopyCommon will not be executed.
>>>>
>>>> This can be solved in two ways. Either generate <module>-copy rules 
>>>> for all modules or leave the files there with just include 
>>>> CopyCommon.gmk as the only contents. I would recommend the latter 
>>>> for now. Most modules do not need to copy anything.
>>>
>>> Is it possible to generate <module>-copy rules for module where 
>>> src/<module>/{share, $OS}/include directory or 
>>> make/copy/Copy-<module>.gmk is present?
>> Technically, it's of course possible. But it does not fit very well 
>> with the current DeclareRecipesForPhase. I agree with Erik, that for 
>> now the reasonable approach is to have files that only include 
>> CopyCommon. We can consider for future updates if it's worth 
>> generalizing this.
>>
>> Updated webrev that restores the removed Copy-$MODULE.gmk files:
>> http://cr.openjdk.java.net/~ihse/JDK-8190484-move-hotspot-exported-includes/webrev.02 
>>
>>
>>>>
>>>> Another minor note, when ordering include directories, I usually 
>>>> put the most specific dir first, so that any platform specific 
>>>> header file with the same name would override a more general one. 
>>>> We don't have that situation in this case, but I still think it's 
>>>> good practice.
>>>>
>>>> Regarding where to push this. IMO, if it depends on a change 
>>>> currently in hs, push it to hs. If it ends up in JDK 10 or 11 
>>>> doesn't really matter that much.
>>>>
>>>
>>> I would love this in JDK 10 if time permits and I am happy to see 
>>> Coleen retarget it to 10.  This is a really nice clean up that shows 
>>> the benefit from JEP 201 w.r.t. exported native header files.   But 
>>> this is not a must for JDK 10 and if it can't make 10, it's okay for 
>>> 11.
>>
>> Ok. I'll try to get it into jdk 10. Will push this to jdk/jdk as soon 
>> as the needed fixes are integrated from jdk/hs.
>>
>> /Magnus
>>
>>>
>>> Mandy
>>>
>>>
>>>> /Erik
>>>>
>>>>
>>>> On 2017-12-04 03:06, Magnus Ihse Bursie wrote:
>>>>> JDK-8190484 was created as a follow-up bug to the unification of 
>>>>> the duplicated jvm.h, jvm_md.h and jmm.h, to determine the proper 
>>>>> location of these files. This has now been decided to be 
>>>>> hotspot/share/include and hotspot/os/$OS/include, respectively.
>>>>>
>>>>> This patch moves the relevant files there, but since this also 
>>>>> frees up the src/$MODULE/native/include directories for the 
>>>>> original purpose, it also unifies and simplifies the build logic 
>>>>> for these directories, so that common code is executed for all 
>>>>> modules to just copy any exported header files from these 
>>>>> directories, should they exist.
>>>>>
>>>>> I'm intending to push this to jdk-hs.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8190484
>>>>> WebRev: 
>>>>> http://cr.openjdk.java.net/~ihse/JDK-8190484-move-hotspot-exported-includes/webrev.01 
>>>>>
>>>>>
>>>>> /Magnus
>>>>
>>>
>>