Closed Bug 1375859 Opened 5 years ago Closed 5 years ago

Use the zlib in mozglue instead of the system provided one

Categories

(Firefox for Android Graveyard :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: gcp, Assigned: glandium)

References

Details

Attachments

(2 files)

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.
Blocks: 1323277
I'll hook that up properly in the build... next week.
Assignee: nobody → mh+mozilla
Flags: needinfo?(mh+mozilla)
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.
Attachment #8881492 - Flags: review?(mshal)
Comment on attachment 8881492 [details]
Bug 1375859 - Build zlib in libmozglue when the linker in enabled.

https://reviewboard.mozilla.org/r/152640/#review157940
Attachment #8881492 - Flags: review?(mshal) → review+
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.
Depends on: 1376704
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.
Flags: needinfo?(mh+mozilla)
Attachment #8881492 - Flags: review+ → review?(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!
Attachment #8881492 - Flags: review?(mshal) → review+
Comment on attachment 8881493 [details]
Bug 1375859 - Build fennec with in-tree zlib.

https://reviewboard.mozilla.org/r/152642/#review158488
Attachment #8881493 - Flags: review?(snorp) → review+
Pushed by mh@glandium.org:
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
https://hg.mozilla.org/mozilla-central/rev/1de11ea129cd
https://hg.mozilla.org/mozilla-central/rev/a9e6437e0e60
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.