803811 - --with-system-zlib version detection broken after bug 781446

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect)

Description

[Landry Breuil (:gaston)]

After http://hg.mozilla.org/comm-central/rev/00c391e2f48c (now in tb 17.0b1), build with --system-zlib fails to correctly detect the version.

$grep VERNUM /usr/include/zlib.h                                                     
#define ZLIB_VERNUM 0x1230

MOZZLIB is also 0x1230.

the MOZZLIBNUM check does 
$echo 0x1230 | awk -F. '{printf "0x%x\n", ((($1 * 16 + $2) * 16) + $3) * 16 + $4}'
0x1230000

which fails the configure test - if the version is MOZZLIB=1.2.3, the test succeeds

$echo 1.2.3 | awk -F. '{printf "0x%x\n", ((($1 * 16 + $2) * 16) + $3) * 16 + $4}'  
0x1230

So, either MOZZLIB should be fixed in configure.in (like it was in mozilla/configure.in for a short period of time) or bug #763651 should be ported (since it adds a MOZ_ZLIB_CHECK macro)

So, which way should it be fixed (in c-c, c-a, c-b) ?

Comment 1

[Mike Hommey [:glandium]]

Use the MOZ_ZLIB_CHECK macro. You can include m-c's m4 directly.

Comment 2

[Landry Breuil (:gaston)]

Attachment: use MOZ_ZLIB_CHECK (port part of #763651)

First shot, seems to correctly set the MOZ_ZLIB vars after configuring with and without --system-zlib set. I'm not 100% fluent with the configure maze, so i didnt take all parts of the mozilla bug, this probably needs more love.

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 673546 [details] [diff] [review]
use MOZ_ZLIB_CHECK (port part of #763651)

Review of attachment 673546 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +3966,5 @@
> +    MOZ_ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,$(DEPTH)/mozilla/modules/zlib/src)'
> +fi
> +
> +if test "$MOZ_LINKER" = 1 -a "$MOZ_NATIVE_ZLIB" != 1; then
> +    AC_MSG_ERROR([Custom dynamic linker requires --with-system-zlib])

You don't need this part.

Comment 4

[Landry Breuil (:gaston)]

What about this m-c part at the bottom of configure.in then ? wasnt sure if it was needed for c-c either ir it was just for the link js to zlib part :

if test "$MOZ_NATIVE_ZLIB" != 1 -a "$OS_ARCH" = "WINNT"; then
   MOZ_ZLIB_LIBS=
fi
export MOZ_NATIVE_ZLIB
export MOZ_ZLIB_CFLAGS
export MOZ_ZLIB_LIBS

Comment 5

[Mike Hommey [:glandium]]

(In reply to Landry Breuil (:gaston) from comment #4)
> What about this m-c part at the bottom of configure.in then ? wasnt sure if
> it was needed for c-c either ir it was just for the link js to zlib part :
> 
> if test "$MOZ_NATIVE_ZLIB" != 1 -a "$OS_ARCH" = "WINNT"; then
>    MOZ_ZLIB_LIBS=
> fi
> export MOZ_NATIVE_ZLIB
> export MOZ_ZLIB_CFLAGS
> export MOZ_ZLIB_LIBS

Yeah, you don't need that.

Comment 6

[Landry Breuil (:gaston)]

Attachment: use MOZ_ZLIB_CHECK (port part of #763651)

So let's port only the needed parts. i'll see if the same patch applies to aurora/beta or if MOZZLIB needs to be fixed there instead, as id like to get it fixed in next betas..

Comment 7

[Landry Breuil (:gaston)]

https://hg.mozilla.org/comm-central/rev/9d72569694f7

Comment 8

[Landry Breuil (:gaston)]

Comment on attachment 673561 [details] [diff] [review]
use MOZ_ZLIB_CHECK (port part of #763651)

Nominating for aurora/beta, since they're affected too (the beta patch doesnt apply as-is but i have it locally)
[Approval Request Comment]
Regression caused by (bug #): Bug 781446 
User impact if declined: failure to build with --with-system-zlib
Testing completed (on c-c, etc.): in progress
Risk to taking this patch (and alternatives if risky): none

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 673561 [details] [diff] [review]
use MOZ_ZLIB_CHECK (port part of #763651)

Yep, I think we can port this safely. a=me.

Comment 10

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/6f55d3140a2d
https://hg.mozilla.org/releases/comm-beta/rev/86cf289595ef