PING: [PATCH FOR REVIEW] System Zlib Support

[Andrew Hughes]

----- Original Message -----
> Hi Andrew,
> 
> I meant if we are going to put SYSTEM_ZLIB=true as default for linux
> as
> Alan suggested,
> we might need to update the build document as well to include
> zlib-dev
> as the "necessary"
> package to build jdk on linux.

Ok.  I think that's a separate issue which warrants a separate bug &
changeset.  Let's have this one work on fixing the build so SYSTEM_ZLIB
actually works on GNU/Linux first.  At the moment, enabling SYSTEM_ZLIB=true
breaks the build on anything other than MacOS/X, which seems like a bug to me.

> 
> Currently the SYSTEM_ZLIB=true is set in make/common/Defs-macosx.gmk.

Ok, so it's the default there already.  That explains the defines.h logic.

> ZLIB_VERSION = 1.2.5 is setin make/common/Defs.gmk. 

What's the relevance of this version?  My system install is 1.2.7.

> I'm not good at
> Makfile structure,
> just wonder why not put the ZLIB_LIBS setting into same place as
> well,
> it might help the
> future maintenance. I'm not sure in Defs.gmk or three copies in
> Defs-<os>.gmk, though.
> Personally, I would just put it in Defs.gmk, together with the
> ZLIB_VERSION.

There's more than just three Defs-<os>.gmk files. There's also
Windows and embedded, neither of which I can build and I doubt
"-lz" works on Windows for linking zlib.  I've used MacOS X/*BSD,
Solaris and GNU/Linux enough to know it will work there.

> 
> The rest looks fine to me.
> 

Thanks :-)

> -Sherman
> 
> On 08/06/2012 05:16 AM, Andrew Hughes wrote:
> >
> > ----- Original Message -----
> >> On 8/5/2012 2:00 PM, Alan Bateman wrote:
> >>> On 03/08/2012 19:33, Andrew Hughes wrote:
> >>>> :
> >>>> http://cr.openjdk.java.net/~andrew/syslibs/zlib/webrev.02/
> >>>>
> >>>> is an updated version which checks if ZLIB_LIBS is set on
> >>>> Solaris,
> >>>> GNU/Linux
> >>>> and MacOS X and sets it to -lz if not.
> >>>>
> >>>> I wasn't sure what to do with Windows but something can be added
> >>>> there if necessary.
> >>> Thanks for the update, it looks right to me now. To double check
> >>> I
> >>> did
> >>> a quick build+test on all platforms with latest jdk8/tl + your
> >>> patch
> >>> and I don't see any issues.
> >>>
> >>> Now I'm wondering whether we should just bite the bullet and
> >>> default
> >>> SYSTEM_ZLIB to true on Linux, maybe Solaris too (Sherman - can
> >>> you
> >>> think of any reasons not to do this? It would avoid needing to
> >>> put
> >>> in
> >>> a means to switch zlib at startup as it could be done simply with
> >>> LD_LIBRARY_PATH).
> >>>
> >>> -Alan.
> >> I'm still on a very old ubuntu (9.1) so I might be wrong.  Does
> >> the
> >> pkg-config --cflags/libs assume the zlib-dev or
> >> some similar dev package to be installed? pkg-config says I don't
> >> have
> >> it installed, so the cflags does not get
> >> set correctly.
> > You'll need zlib-dev both for pkg-config and the actual build, as
> > you'll
> > need the zlib headers.  CFLAGS is usually empty anyway but
> > pkgconfig
> > will also provide the "-lz" for ZLIB_LIBS.
> >
> >> It appears at least one ubuntu12 machine has the same
> >> situation. So I guess at least we will have
> >> to add something into the "build readme" to add this package, if
> >> it
> >> is
> >> not installed by default.
> >>
> > Probably.  This is pretty standard for building anything on a
> > binary distribution,
> > as binaries are split away from development headers, so it's not
> > anything out of the
> > ordinary.  Headers for other libraries are already a requirement.
> >
> >> I don't have a Solaris machine for a while, so just wonder if the
> >> zlib
> >> always get installed by default installation
> >> these days?
> >>
> > I'll defer to those better informed on this one :-)
> >
> >> -Sherman
> >>
> 
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07