1375859 - Use the zlib in mozglue instead of the system provided one

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement)

Description

[Gian-Carlo Pascutto [:gcp]]

As discovered in bug 1323277, using the system provided zlib may mean we are stuck with bugs (including security bugs) that are fixed in later zlib versions.

We currently force usage of the system zlib on Android:
https://dxr.mozilla.org/mozilla-central/rev/b1b9129838ade91684574f42219b2010928d7db4/mobile/android/config/mozconfigs/common#42

According to glandium, we use our own zlib in mozglue.

He suggested we try building with ZLIB_IN_MOZGLUE=1 and see if it works. esawin confirmed it does.

So it looks like we might be able to get rid of the system zlib without a size penalty.

Comment 1

[Mike Hommey [:glandium]]

I'll hook that up properly in the build... next week.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

The linker uses zlib. The linker is in mozglue, zlib is in libxul by
default, and if not building with --with-system-zlib, this can't work
(mozglue can't depend on libxul).

So enable the setup where zlib is built in mozglue when we need it
there.

Review commit: https://reviewboard.mozilla.org/r/152640/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152640/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1375859 - Build fennec with in-tree zlib.

Review commit: https://reviewboard.mozilla.org/r/152642/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/152642/

Comment 4

[Michael Shal [:mshal]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

https://reviewboard.mozilla.org/r/152640/#review157912

::: old-configure.in:4560
(Diff revision 1)
>  dnl =
>  dnl ========================================================
>  MOZ_ARG_HEADER(Static build options)
>  
>  if test -z "$MOZ_SYSTEM_ZLIB"; then
> -if test -n "$JS_SHARED_LIBRARY"; then
> +if test -n "$JS_SHARED_LIBRARY" -o -n "$MOZ_LINKER"; then

Isn't MOZ_LINKER guaranteed to be empty here because of this check? https://dxr.mozilla.org/mozilla-central/rev/53477d584130945864c4491632f88da437353356/old-configure.in#2038

IOW, if MOZ_SYSTEM_ZLIB isn't set, we can't have MOZ_LINKER set because if we did then we'd hit that error message. If that's true, then I think you can just remove this conditional entirely.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152640/diff/1-2/

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152642/diff/1-2/

Comment 7

[Michael Shal [:mshal]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

https://reviewboard.mozilla.org/r/152640/#review157940

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152642/diff/2-3/

Comment 9

[Mike Hommey [:glandium]]

Note this breaks the build on building szip... which we actually don't need since bug 1307886. Back then, I wanted to keep the code around, but months later, I think we can remove it. I'll file a separate bug for that, which this bug will depend on.

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152640/diff/2-3/

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152642/diff/3-4/

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152640/diff/3-4/

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/152642/diff/4-5/

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

There are a couple additional changes that should get some review. I also rebased on top of bug 1376704 and will trigger a try once more.

Comment 15

[Michael Shal [:mshal]]

Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

https://reviewboard.mozilla.org/r/152640/#review158486

LGTM!

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

https://reviewboard.mozilla.org/r/152642/#review158488

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1de11ea129cd
Build zlib in libmozglue when the linker in enabled. r=mshal
https://hg.mozilla.org/integration/autoland/rev/a9e6437e0e60
Build fennec with in-tree zlib. r=snorp

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1de11ea129cd
https://hg.mozilla.org/mozilla-central/rev/a9e6437e0e60