RFR: JDK-8031767 Support system or alternative implementations of zlib

Erik Joelsson erik.joelsson at oracle.com
Fri Mar 25 17:42:26 UTC 2016


Looks good to me.

/Erik

On 2016-03-24 23:31, Xueming Shen wrote:
> Thanks Erik!
>
> Webrev has been updated accordingly.
>
> http://cr.openjdk.java.net/~sherman/8031767/webrev/
>
> -Sherman
>
> On 03/24/2016 01:57 PM, Erik Joelsson wrote:
>> Hello,
>>
>> Yes, I believe we still need to, the reason being that configure 
>> automatically falls back to bundled if the headers are not available. 
>> This is what happened when you initially tried your patch in JPRT. I 
>> still think this is a good way for configure to behave in the general 
>> case. In the scenario of building our official builds we should 
>> instead fail fast if the intended configuration is not possible. It's 
>> true this has not been needed with Macosx so far. It seems zlib 
>> headers are generally available on that platform. I still think it's 
>> better to be explicit with these things so that the intention is 
>> obvious. Solving this kind of problem, of specifying stricter 
>> requirements for Oracle builds is one of the main features of using Jib.
>>
>> /Erik
>>
>> On 2016-03-24 18:21, Xueming Shen wrote:
>>> Erik,
>>>
>>> I'm not familiar with the jib-profiles.js. So just want to confirm 
>>> before putting into
>>> the webrev. The proposal is to build with system zlib by default for 
>>> non-windows
>>> platforms, without the need of specifying the configuration/build opton
>>> --with-zlib=system. Do we still need to update this with explicit 
>>> option, as we are
>>> not doing that for macos, which is using the system zlib for a while.
>>>
>>> Thanks,
>>> Sherman
>>>
>>> On 03/24/2016 06:22 AM, Erik Joelsson wrote:
>>>> Hello again. Here is my suggested patch for the jib-profiles.js 
>>>> file. This will enforce system zlib for Oracle builds on Linux, 
>>>> Solaris and Macosx.
>>>>
>>>> /Erik
>>>>
>>>> diff -r 6da9e0c79eac common/conf/jib-profiles.js
>>>> --- a/common/conf/jib-profiles.js
>>>> +++ b/common/conf/jib-profiles.js
>>>> @@ -241,7 +241,7 @@
>>>>              target_os: "linux",
>>>>              target_cpu: "x64",
>>>>              dependencies: concat(common.dependencies, "devkit"),
>>>> -            configure_args: common.configure_args,
>>>> +            configure_args: concat(common.configure_args, 
>>>> "--with-zlib=system"),
>>>>              make_args: common.make_args
>>>>          },
>>>>
>>>> @@ -250,7 +250,8 @@
>>>>              target_cpu: "x86",
>>>>              build_cpu: "x64",
>>>>              dependencies: concat(common.dependencies, "devkit"),
>>>> -            configure_args: concat(common.configure_args, 
>>>> common.configure_args_32bit),
>>>> +            configure_args: concat(common.configure_args, 
>>>> common.configure_args_32bit,
>>>> +                "--with-zlib=system"),
>>>>              make_args: common.make_args
>>>>          },
>>>>
>>>> @@ -258,7 +259,7 @@
>>>>              target_os: "macosx",
>>>>              target_cpu: "x64",
>>>>              dependencies: concat(common.dependencies, "devkit"),
>>>> -            configure_args: common.configure_args,
>>>> +            configure_args: concat(common.configure_args, 
>>>> "--with-zlib=system"),
>>>>              make_args: common.make_args
>>>>          },
>>>>
>>>> @@ -266,7 +267,7 @@
>>>>              target_os: "solaris",
>>>>              target_cpu: "x64",
>>>>              dependencies: concat(common.dependencies, "devkit", 
>>>> "cups"),
>>>> -            configure_args: common.configure_args,
>>>> +            configure_args: concat(common.configure_args, 
>>>> "--with-zlib=system"),
>>>>              make_args: common.make_args
>>>>          },
>>>>
>>>> @@ -274,7 +275,7 @@
>>>>              target_os: "solaris",
>>>>              target_cpu: "sparcv9",
>>>>              dependencies: concat(common.dependencies, "devkit", 
>>>> "cups"),
>>>> -            configure_args: common.configure_args,
>>>> +            configure_args: concat(common.configure_args, 
>>>> "--with-zlib=system"),
>>>>              make_args: common.make_args
>>>>          },
>>>>
>>>>
>>>> On 2016-03-24 09:05, Erik Joelsson wrote:
>>>>> Hello,
>>>>>
>>>>> As I wrote in the bug, jdk9/dev currently fails when using 
>>>>> --with-zlib=system with the new devkit on Linux. That will need to 
>>>>> be fixed first.
>>>>>
>>>>> If the intention of this change is to enforce --with-zlib=system 
>>>>> on OracleJDK builds, we should also update the Jib profile 
>>>>> definitions for Linux and Solaris. Configure currently falls back 
>>>>> to internal if external isn't available. That's a bit too brittle 
>>>>> for our official builds. Enforcing it from Jib is the best way for 
>>>>> this kind of change IMO.
>>>>>
>>>>> Please don't post diffs of internal files on the open list. In 
>>>>> this case, nothing secret was in the diff but it could easily have 
>>>>> been. There is no need for posting review on the closed 
>>>>> generated-configure as long as that's the only thing that changed 
>>>>> in the closed repo, as long as the open changes causing it are 
>>>>> properly reviewed.
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2016-03-23 19:37, Xueming Shen wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This one was discussed back to Feb, and have been waiting for the 
>>>>>> devkit clearance
>>>>>> from the build-dev, which has just been resolved [1]. So here is 
>>>>>> webrev again.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~sherman/8031767/webrev
>>>>>>
>>>>>> thanks!
>>>>>> Sherman
>>>>>>
>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8149545
>>>>>>
>>>>>> btw, here is the similar change in the corresponding "closed"
>>>>>> autoconf/generated-configure.sh for convenience.
>>>>>>
>>>>>> @@ -5353,11 +5353,11 @@
>>>>>>
>>>>>> # Do not change or remove the following line, it is needed for 
>>>>>> consistency checks:
>>>>>> -DATE_WHEN_GENERATED=1458558778
>>>>>> +DATE_WHEN_GENERATED=1458755892
>>>>>>
>>>>>>  ############################################################################### 
>>>>>>
>>>>>>  #
>>>>>>  # Initialization / Boot-strapping
>>>>>>  #
>>>>>>
>>>>>> @@ -62528,14 +62528,14 @@
>>>>>>
>>>>>>
>>>>>>    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for which 
>>>>>> zlib to use" >&5
>>>>>>  $as_echo_n "checking for which zlib to use... " >&6; }
>>>>>>
>>>>>> -  DEFAULT_ZLIB=bundled
>>>>>> -  if test "x$OPENJDK_TARGET_OS" = xmacosx; then
>>>>>> -    # On macosx default is system...on others default is bundled
>>>>>>      DEFAULT_ZLIB=system
>>>>>> +  if test "x$OPENJDK_TARGET_OS" = xwindows; then
>>>>>> +    # On windows default is bundled...on others default is system
>>>>>> +    DEFAULT_ZLIB=bundled
>>>>>>    fi
>>>>>>
>>>>>>    if test "x${ZLIB_FOUND}" != "xyes"; then
>>>>>>      # If we don't find any system...set default to bundled
>>>>>>      DEFAULT_ZLIB=bundled
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 02/05/2016 10:55 AM, Xueming Shen wrote:
>>>>>>> Hi
>>>>>>>
>>>>>>> Please help codereview the change to build the jdk9 runtime to 
>>>>>>> use the system zlib on
>>>>>>> Solaris and Linux platforms by default.
>>>>>>>
>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8031767
>>>>>>> Webrev: http://cr.openjdk.java.net/~sherman/8031767/webrev/
>>>>>>>
>>>>>>> Background info:
>>>>>>>
>>>>>>> Compression is heavily used in Java based big data/middle-ware 
>>>>>>> applications.
>>>>>>> There are many products in market today that help compression 
>>>>>>> performance
>>>>>>> either through software or hardware acceleration and most likely 
>>>>>>> these products
>>>>>>> support the zlib interface as API, for example Intel's IPP 
>>>>>>> library has a faster
>>>>>>> version of compression libraries. To configure the Java runtime 
>>>>>>> to use the system
>>>>>>> zlib would make these acceleration capabilities available to 
>>>>>>> java users through
>>>>>>> java.util.zip package directly. The jdk already has a build 
>>>>>>> configuration option
>>>>>>> to build the jdk to use the system zlib via "--with-zlib=system" 
>>>>>>> and the OSX is
>>>>>>> by default built to use the system zlib. This proposal is to 
>>>>>>> propose to build
>>>>>>> the jdk to use the system zlib library (the zlib bundled by the 
>>>>>>> underlying Solaris/
>>>>>>> Linuxplatforms), instead of the binary  built from source code 
>>>>>>> jdk repository
>>>>>>> (current 1.2.8 from the open source zlib.org)
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Sherman
>>>>>>>
>>>>>>>
>>>>>>> btw, attached is the similar change in the closed repo: 
>>>>>>> autoconf/generated-configure.sh
>>>>>>> -------------------------------------------------------------
>>>>>>>
>>>>>>> # Do not change or remove the following line, it is needed for 
>>>>>>> consistency checks:
>>>>>>> -DATE_WHEN_GENERATED=1454436146
>>>>>>> +DATE_WHEN_GENERATED=1454626552
>>>>>>>
>>>>>>>  ############################################################################### 
>>>>>>>
>>>>>>>  #
>>>>>>>  # Initialization / Boot-strapping
>>>>>>>  #
>>>>>>>
>>>>>>> ------------------------------------------------------------------------ 
>>>>>>>
>>>>>>>
>>>>>>> @@ -58839,14 +58839,14 @@
>>>>>>>
>>>>>>>
>>>>>>>    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for which 
>>>>>>> zlib to use" >&5
>>>>>>>  $as_echo_n "checking for which zlib to use... " >&6; }
>>>>>>>
>>>>>>> - DEFAULT_ZLIB=bundled
>>>>>>> - if test "x$OPENJDK_TARGET_OS" = xmacosx; then
>>>>>>> - # On macosx default is system...on others default is bundled
>>>>>>>      DEFAULT_ZLIB=system
>>>>>>> + if test "x$OPENJDK_TARGET_OS" = xwindows; then
>>>>>>> + # On windows default is bundled...on others default is system
>>>>>>> + DEFAULT_ZLIB=bundled
>>>>>>>    fi
>>>>>>>
>>>>>>>    if test "x${ZLIB_FOUND}" != "xyes"; then
>>>>>>>      # If we don't find any system...set default to bundled
>>>>>>>      DEFAULT_ZLIB=bundled
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the core-libs-dev mailing list