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)