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

[David Holmes]

Magnus,

It seems these changes have broken all Windows builds:

Copying support/modules_include/jdk.accessibility/win32/bridge
/usr/bin/cp -fP 
'/cygdrive/c/jprt/T/P1/042744.daholme/s/open/src/jdk.accessibility/windows/native/include/bridge' 
'/cygdrive/c/jprt/T/P1/042744.daholme/s/build/windows-x64-debug/support/modules_include/jdk.accessibility/win32/bridge'
/usr/bin/cp: omitting directory 
'/cygdrive/c/jprt/T/P1/042744.daholme/s/open/src/jdk.accessibility/windows/native/include/bridge'
CopyCommon.gmk:62: recipe for target 
'/cygdrive/c/jprt/T/P1/042744.daholme/s/build/windows-x64-debug/support/modules_include/jdk.accessibility/win32/bridge' 
failed
make[3]: *** 
[/cygdrive/c/jprt/T/P1/042744.daholme/s/build/windows-x64-debug/support/modules_include/jdk.accessibility/win32/bridge] 
Error 1

David
-----

On 5/12/2017 7:26 AM, Erik Joelsson wrote:
> Looks good.
> 
> /Erik
> 
> 
> On 2017-12-04 12:40, 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
>>>>
>>>
>>
>