Thanks for the reviews - the fix is now in the JPRT queue.

/Staffan

> On 4 maj 2015, at 11:11, Erik Joelsson <erik.joels...@oracle.com> wrote:
> 
> 
> On 2015-05-04 10:25, Staffan Larsen wrote:
>>> On 4 maj 2015, at 09:54, Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 
>>> wrote:
>>> 
>>> 
>>>> 4 maj 2015 kl. 09:27 skrev David Holmes <david.hol...@oracle.com>:
>>>> 
>>>> Hi Staffan,
>>>> 
>>>> Seems fine as a spot fix but I'm wondering if this shouldn't be a common 
>>>> option for all the dlls now we are building with VS2013?
>>> Or maybe as a define in the source code before the include section for the 
>>> specific source code that needs a legacy version of GetProcessMemoryInfo? 
>>> That seems more prudent to me.
>> In this case the function is called the same and works the same in all 
>> versions of Windows (there is no “legacy version”). What differs is that it 
>> is linked against different names in different libs for different versions 
>> of Windows. So from a source code perspective there is no difference, but 
>> from a build perspective there is.
> I think the placement suggested here is fine. It could be moved to configure 
> for global application, but I doubt it will ever be needed. As soon as we 
> drop support for Windows versions older than 7, we can remove this option.
> 
> /Erik
>> /Staffan
>> 
>>> /Magnus
>>> 
>>>> Thanks,
>>>> David
>>>> 
>>>>> On 4/05/2015 4:52 PM, Staffan Larsen wrote:
>>>>> This is a P1 bug that surfaced when changes from jdk9/dev and jdk9/hs-rt 
>>>>> met in jdk9/hs. In this case the windows compiler upgrades in jdk9/dev 
>>>>> met changes in jdk9/hs-rt that moved a call to GetProcessMemoryInfo from 
>>>>> management.dll to management_ext.dll. With the compiler upgrades 
>>>>> PSAPI_VERSION=1 is needed when compiling the library calling 
>>>>> GetProcessMemoryInfo. This fix simply moves that patch from 
>>>>> make/lib/Lib-java.management.gmk to make/lib/Lib-jdk.management.gmk. The 
>>>>> patch was introduced in JDK-8076557.
>>>>> 
>>>>> I will push the change below directly to jdk9/hs.
>>>>> 
>>>>> Thanks,
>>>>> /Staffan
>>>>> 
>>>>> 
>>>>> diff --git a/make/lib/Lib-java.management.gmk 
>>>>> b/make/lib/Lib-java.management.gmk
>>>>> --- a/make/lib/Lib-java.management.gmk
>>>>> +++ b/make/lib/Lib-java.management.gmk
>>>>> @@ -38,11 +38,6 @@
>>>>>     $(LIBJAVA_HEADER_FLAGS) \
>>>>>     #
>>>>> 
>>>>> -# In (at least) VS2013 and later, -DPSAPI_VERSION=1 is needed to generate
>>>>> -# a binary that is compatible with windows versions older than 7/2008R2.
>>>>> -# See MSDN documentation for GetProcessMemoryInfo for more information.
>>>>> -BUILD_LIBMANAGEMENT_CFLAGS += -DPSAPI_VERSION=1
>>>>> -
>>>>> LIBMANAGEMENT_OPTIMIZATION := HIGH
>>>>> ifneq ($(findstring $(OPENJDK_TARGET_OS), solaris linux), )
>>>>>   ifeq ($(ENABLE_DEBUG_SYMBOLS), true)
>>>>> diff --git a/make/lib/Lib-jdk.management.gmk 
>>>>> b/make/lib/Lib-jdk.management.gmk
>>>>> --- a/make/lib/Lib-jdk.management.gmk
>>>>> +++ b/make/lib/Lib-jdk.management.gmk
>>>>> @@ -39,6 +39,11 @@
>>>>>     $(LIBJAVA_HEADER_FLAGS) \
>>>>>     #
>>>>> 
>>>>> +# In (at least) VS2013 and later, -DPSAPI_VERSION=1 is needed to generate
>>>>> +# a binary that is compatible with windows versions older than 7/2008R2.
>>>>> +# See MSDN documentation for GetProcessMemoryInfo for more information.
>>>>> +BUILD_LIBMANAGEMENT_EXT_CFLAGS += -DPSAPI_VERSION=1
>>>>> +
>>>>> LIBMANAGEMENT_EXT_OPTIMIZATION := HIGH
>>>>> ifneq ($(findstring $(OPENJDK_TARGET_OS), solaris linux), )
>>>>>   ifeq ($(ENABLE_DEBUG_SYMBOLS), true)

Reply via email to