Last Comment Bug 803811 - --with-system-zlib version detection broken after bug 781446
: --with-system-zlib version detection broken after bug 781446
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Build Config (show other bugs)
: unspecified
: x86 OpenBSD
: -- normal (vote)
: Thunderbird 19.0
Assigned To: Landry Breuil (:gaston)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-20 03:01 PDT by Landry Breuil (:gaston)
Modified: 2012-11-29 02:18 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
use MOZ_ZLIB_CHECK (port part of #763651) (5.42 KB, patch)
2012-10-20 04:06 PDT, Landry Breuil (:gaston)
mh+mozilla: feedback+
Details | Diff | Splinter Review
use MOZ_ZLIB_CHECK (port part of #763651) (5.29 KB, patch)
2012-10-20 06:41 PDT, Landry Breuil (:gaston)
standard8: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description Landry Breuil (:gaston) 2012-10-20 03:01:25 PDT
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] 2012-10-20 03:38:44 PDT
Use the MOZ_ZLIB_CHECK macro. You can include m-c's m4 directly.
Comment 2 Landry Breuil (:gaston) 2012-10-20 04:06:13 PDT
Created attachment 673546 [details] [diff] [review]
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] 2012-10-20 04:18:15 PDT
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) 2012-10-20 04:47:18 PDT
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] 2012-10-20 06:06:53 PDT
(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) 2012-10-20 06:41:53 PDT
Created attachment 673561 [details] [diff] [review]
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) 2012-10-25 12:10:57 PDT
https://hg.mozilla.org/comm-central/rev/9d72569694f7
Comment 8 Landry Breuil (:gaston) 2012-10-25 12:14:34 PDT
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, afk until Dec) 2012-10-29 02:20:29 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.