Closed Bug 763651 Opened 12 years ago Closed 12 years ago

link JS to zlib

Categories

(Firefox Build System :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla16

People

(Reporter: Benjamin, Assigned: glandium)

References

Details

Attachments

(1 file, 9 obsolete files)

As part of bug 761723, I'm compressing JS source from the parser. I'm using zlib for this, so I need the appropriate zlib (system or built-in) to the built and linked to the JS library.
This is going to suck a little bit on Windows because mozjs is a separate DLL, so we'll have to link zlib into both mozjs and xul.
Can't we just stick zlib in mozglue?
Attached patch first try (obsolete) — Splinter Review
Here is a patch which links libmozz into mozglue. It works fine when mozglue is a static library on Linux. When it's shared, the zlib symbols are not kept (or exported?), so building fails. I'd appreciate any advice on how to deal with this.
(In reply to Benjamin Peterson from comment #3)
> Created attachment 633194 [details] [diff] [review]
> first try
> 
> Here is a patch which links libmozz into mozglue. It works fine when mozglue
> is a static library on Linux.

Are you sure?

> When it's shared, the zlib symbols are not
> kept (or exported?), so building fails. I'd appreciate any advice on how to
> deal with this.

You probably just need to set VISIBILITY_FLAGS to nothing in zlib's makefile.
(In reply to Mike Hommey [:glandium] from comment #4)
> (In reply to Benjamin Peterson from comment #3)
> > Created attachment 633194 [details] [diff] [review]
> > first try
> > 
> > Here is a patch which links libmozz into mozglue. It works fine when mozglue
> > is a static library on Linux.
> 
> Are you sure?

Yes. For example https://tbpl.mozilla.org/php/getParsedLog.php?id=12650568&tree=Try&full=1

> 
> > When it's shared, the zlib symbols are not
> > kept (or exported?), so building fails. I'd appreciate any advice on how to
> > deal with this.
> 
> You probably just need to set VISIBILITY_FLAGS to nothing in zlib's makefile.

I wonder if the problem is the linker is not retaining the zlib symbols in the shared library since no one uses them.
(In reply to Benjamin Peterson from comment #5)
> (In reply to Mike Hommey [:glandium] from comment #4)
> > (In reply to Benjamin Peterson from comment #3)
> > > Created attachment 633194 [details] [diff] [review]
> > > first try
> > > 
> > > Here is a patch which links libmozz into mozglue. It works fine when mozglue
> > > is a static library on Linux.
> > 
> > Are you sure?
> 
> Yes. For example
> https://tbpl.mozilla.org/php/getParsedLog.php?id=12650568&tree=Try&full=1

It's not because it builds that it does the right thing. I'll check.

> > > When it's shared, the zlib symbols are not
> > > kept (or exported?), so building fails. I'd appreciate any advice on how to
> > > deal with this.
> > 
> > You probably just need to set VISIBILITY_FLAGS to nothing in zlib's makefile.
> 
> I wonder if the problem is the linker is not retaining the zlib symbols in
> the shared library since no one uses them.

They are probably not exported (thus resetting VISIBILITY_FLAGS).
Attached patch second try (obsolete) — Splinter Review
Here's a second iteration. It works if WRAP_SYSTEM_INCLUDES is on (thanks khuey), but fails when -fvisibility=hidden is being used everywhere. I'm not sure what needs to be flipped now.
Attachment #633194 - Attachment is obsolete: true
Comment on attachment 633410 [details] [diff] [review]
second try

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

::: js/src/configure.in
@@ +3494,5 @@
> +     [ZLIB_CFLAGS=$withval
> +      MOZ_NATIVE_ZLIB=])
> +
> +if test -n "$MOZ_NATIVE_ZLIB"; then
> +   PKG_CHECK_MODULES(ZLIB, zlib)

The main configure doesn't do this check with pkg-config. It would be better if both did the same thing. Depending on how old the pkg-config file is for zlib, that could mean changing the main configure.in. However, if it's rather new, I'd go for using the same thing as the main configure.

::: mozglue/build/Makefile.in
@@ +18,5 @@
>  # If this is ever changed, update MOZ_SHARED_MOZGLUE in browser/installer/Makefile.in
>  ifneq (,$(filter WINNT Darwin Android,$(OS_TARGET)))
>  FORCE_SHARED_LIB = 1
>  else
> +FORCE_SHARED_LIB = 1

You can't do that.

@@ +30,1 @@
>  CPPSRCS = dummy.cpp

You can remove this and the comment. As well as the dummy.cpp file.
(In reply to Mike Hommey [:glandium] from comment #6)
> (In reply to Benjamin Peterson from comment #5)
> > (In reply to Mike Hommey [:glandium] from comment #4)
> > > (In reply to Benjamin Peterson from comment #3)
> > > > Created attachment 633194 [details] [diff] [review]
> > > > first try
> > > > 
> > > > Here is a patch which links libmozz into mozglue. It works fine when mozglue
> > > > is a static library on Linux.
> > > 
> > > Are you sure?
> > 
> > Yes. For example
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=12650568&tree=Try&full=1
> 
> It's not because it builds that it does the right thing. I'll check.

I see why this works: you statically link zlib in mozglue *and* libxul.

Here's how I see it: you only really need to make it part of mozglue when js is built as a shared library. Leave it linked to libxul when it's not.
(In reply to Mike Hommey [:glandium] from comment #9)
> Here's how I see it: you only really need to make it part of mozglue when js
> is built as a shared library. Leave it linked to libxul when it's not.

The problem is we still need to link the js shell.
(In reply to Benjamin Peterson from comment #10)
> (In reply to Mike Hommey [:glandium] from comment #9)
> > Here's how I see it: you only really need to make it part of mozglue when js
> > is built as a shared library. Leave it linked to libxul when it's not.
> 
> The problem is we still need to link the js shell.

Add it to the shell, then. You already have to add something in case you use native zlib.
Attached patch link into mozglue when possible (obsolete) — Splinter Review
Attachment #633410 - Attachment is obsolete: true
Attachment #633863 - Flags: review?(mh+mozilla)
Comment on attachment 633863 [details] [diff] [review]
link into mozglue when possible

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

::: js/src/configure.in
@@ +3491,5 @@
> +
> +MOZ_ARG_ENABLE_BOOL(moz-zlib,
> +[  --enable-moz-zlib=FLAGS   INTERNAL build with builtin Mozilla zlib],
> +     [ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,../../$(DEPTH)/modules/zlib/src)'
> +      ZLIB_DIR=])

I think it would be better to define these in the main configure.in, and export them from there, where they can be picked up without an option. The check below would then pick these values, provided they're not reset above.

@@ +3507,5 @@
> +fi
> +if test -z "$ZLIB_DIR" -o "$ZLIB_DIR" = no; then
> +    SYSTEM_ZLIB=
> +elif test "$ZLIB_DIR" != yes; then
> +    PKG_CHECK_MODULES(ZLIB, zlib)

If you're doing that, please do the same in the main configure.

@@ +3536,5 @@
> +fi
> +
> +ZLIB_PROG_LIBS="${ZLIB_LIBS}"
> +
> +if test "$MOZ_GLUE_PROGRAM_LDFLAGS" -a "$OS_ARCH" != "WINNT"; then

This should be test -z "$MOZ_GLUE_PROGRAM_LDFLAGS", shouldn't it? Or, on the contrary, just set ZLIB_PROG_LIBS when MOZ_GLUE_PROGRAM_LDFLAGS is set.
I also don't think you need to check OS_ARCH.

::: modules/zlib/src/Makefile.in
@@ -14,5 @@
>  MODULE		= zlib
>  LIBRARY_NAME	= mozz
>  GRE_MODULE	= 1
>  LIBXUL_LIBRARY = 1
> -DIST_INSTALL = 1

Is there a particular reason why you remove this?

::: tools/profiler/libunwind/src/Makefile.in
@@ +16,5 @@
>  @SET_MAKE@
>  
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.

You don't want to make these libunwind changes part of this patch :)
Attachment #633863 - Flags: review?(mh+mozilla) → review-
Please note that I landed bug 763987 and bug 764046, which conflicts with some parts of your patch.
(In reply to Mike Hommey [:glandium] from comment #13)
> Comment on attachment 633863 [details] [diff] [review]
> link into mozglue when possible
> 
> Review of attachment 633863 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/configure.in
> @@ +3491,5 @@
> > +
> > +MOZ_ARG_ENABLE_BOOL(moz-zlib,
> > +[  --enable-moz-zlib=FLAGS   INTERNAL build with builtin Mozilla zlib],
> > +     [ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,../../$(DEPTH)/modules/zlib/src)'
> > +      ZLIB_DIR=])
> 
> I think it would be better to define these in the main configure.in, and
> export them from there, where they can be picked up without an option. The
> check below would then pick these values, provided they're not reset above.
> 
> @@ +3507,5 @@
> > +fi
> > +if test -z "$ZLIB_DIR" -o "$ZLIB_DIR" = no; then
> > +    SYSTEM_ZLIB=
> > +elif test "$ZLIB_DIR" != yes; then
> > +    PKG_CHECK_MODULES(ZLIB, zlib)
> 
> If you're doing that, please do the same in the main configure.

The reason I did this, is so is you build js standalone, you get system zlib by default. This doesn't seem to be what the main configure is interested in.

> 
> @@ +3536,5 @@
> > +fi
> > +
> > +ZLIB_PROG_LIBS="${ZLIB_LIBS}"
> > +
> > +if test "$MOZ_GLUE_PROGRAM_LDFLAGS" -a "$OS_ARCH" != "WINNT"; then
> 
> This should be test -z "$MOZ_GLUE_PROGRAM_LDFLAGS", shouldn't it? Or, on the
> contrary, just set ZLIB_PROG_LIBS when MOZ_GLUE_PROGRAM_LDFLAGS is set.
> I also don't think you need to check OS_ARCH.

We only want to link zlib in the js programs if mozglue is not being linked into them.

Checking OS_ARCH was the only way I could convince it to build on Windows.

> 
> ::: modules/zlib/src/Makefile.in
> @@ -14,5 @@
> >  MODULE		= zlib
> >  LIBRARY_NAME	= mozz
> >  GRE_MODULE	= 1
> >  LIBXUL_LIBRARY = 1
> > -DIST_INSTALL = 1
> 
> Is there a particular reason why you remove this?

It somehow allows the zlib symbols to be exported from the mozglue library.

> 
> ::: tools/profiler/libunwind/src/Makefile.in
> @@ +16,5 @@
> >  @SET_MAKE@
> >  
> > +# This Source Code Form is subject to the terms of the Mozilla Public
> > +# License, v. 2.0. If a copy of the MPL was not distributed with this
> > +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> 
> You don't want to make these libunwind changes part of this patch :)

Curiously, running autoreconf-2.13 causes this.
(In reply to Benjamin Peterson from comment #15)
> (In reply to Mike Hommey [:glandium] from comment #13)
> > Comment on attachment 633863 [details] [diff] [review]
> > link into mozglue when possible
> > 
> > Review of attachment 633863 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: js/src/configure.in
> > @@ +3491,5 @@
> > > +
> > > +MOZ_ARG_ENABLE_BOOL(moz-zlib,
> > > +[  --enable-moz-zlib=FLAGS   INTERNAL build with builtin Mozilla zlib],
> > > +     [ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,../../$(DEPTH)/modules/zlib/src)'
> > > +      ZLIB_DIR=])
> > 
> > I think it would be better to define these in the main configure.in, and
> > export them from there, where they can be picked up without an option. The
> > check below would then pick these values, provided they're not reset above.
> > 
> > @@ +3507,5 @@
> > > +fi
> > > +if test -z "$ZLIB_DIR" -o "$ZLIB_DIR" = no; then
> > > +    SYSTEM_ZLIB=
> > > +elif test "$ZLIB_DIR" != yes; then
> > > +    PKG_CHECK_MODULES(ZLIB, zlib)
> > 
> > If you're doing that, please do the same in the main configure.
> 
> The reason I did this, is so is you build js standalone, you get system zlib
> by default. This doesn't seem to be what the main configure is interested in.

I meant the PKG_CHECK_MODULES part. The main configure doesn't use that.

> > @@ +3536,5 @@
> > > +fi
> > > +
> > > +ZLIB_PROG_LIBS="${ZLIB_LIBS}"
> > > +
> > > +if test "$MOZ_GLUE_PROGRAM_LDFLAGS" -a "$OS_ARCH" != "WINNT"; then
> > 
> > This should be test -z "$MOZ_GLUE_PROGRAM_LDFLAGS", shouldn't it? Or, on the
> > contrary, just set ZLIB_PROG_LIBS when MOZ_GLUE_PROGRAM_LDFLAGS is set.
> > I also don't think you need to check OS_ARCH.
> 
> We only want to link zlib in the js programs if mozglue is not being linked
> into them.

Actually, you do want to link zlib in the js programs if mozglue *is* being linked (statically), and only link zlib in mozglue if mozglue is a shared library.
 
> Checking OS_ARCH was the only way I could convince it to build on Windows.
> 
> > 
> > ::: modules/zlib/src/Makefile.in
> > @@ -14,5 @@
> > >  MODULE		= zlib
> > >  LIBRARY_NAME	= mozz
> > >  GRE_MODULE	= 1
> > >  LIBXUL_LIBRARY = 1
> > > -DIST_INSTALL = 1
> > 
> > Is there a particular reason why you remove this?
> 
> It somehow allows the zlib symbols to be exported from the mozglue library.

That sounds unexpected.

> > ::: tools/profiler/libunwind/src/Makefile.in
> > @@ +16,5 @@
> > >  @SET_MAKE@
> > >  
> > > +# This Source Code Form is subject to the terms of the Mozilla Public
> > > +# License, v. 2.0. If a copy of the MPL was not distributed with this
> > > +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> > 
> > You don't want to make these libunwind changes part of this patch :)
> 
> Curiously, running autoreconf-2.13 causes this.

You should just use make -f client.mk, it will update configure.
Attached patch address review comments (obsolete) — Splinter Review
Attachment #633863 - Attachment is obsolete: true
(In reply to Mike Hommey [:glandium] from comment #16)
> (In reply to Benjamin Peterson from comment #15)
> > (In reply to Mike Hommey [:glandium] from comment #13)
> > > Comment on attachment 633863 [details] [diff] [review]
> > > link into mozglue when possible
> > > 
> > > Review of attachment 633863 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > ::: js/src/configure.in
> > > @@ +3491,5 @@
> > > > +
> > > > +MOZ_ARG_ENABLE_BOOL(moz-zlib,
> > > > +[  --enable-moz-zlib=FLAGS   INTERNAL build with builtin Mozilla zlib],
> > > > +     [ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,../../$(DEPTH)/modules/zlib/src)'
> > > > +      ZLIB_DIR=])
> > > 
> > > I think it would be better to define these in the main configure.in, and
> > > export them from there, where they can be picked up without an option. The
> > > check below would then pick these values, provided they're not reset above.
> > > 
> > > @@ +3507,5 @@
> > > > +fi
> > > > +if test -z "$ZLIB_DIR" -o "$ZLIB_DIR" = no; then
> > > > +    SYSTEM_ZLIB=
> > > > +elif test "$ZLIB_DIR" != yes; then
> > > > +    PKG_CHECK_MODULES(ZLIB, zlib)
> > > 
> > > If you're doing that, please do the same in the main configure.
> > 
> > The reason I did this, is so is you build js standalone, you get system zlib
> > by default. This doesn't seem to be what the main configure is interested in.
> 
> I meant the PKG_CHECK_MODULES part. The main configure doesn't use that.

Yes, I was also talking about that. I didn't think this was a useful behaviour for the main configure to have.

> 
> > > @@ +3536,5 @@
> > > > +fi
> > > > +
> > > > +ZLIB_PROG_LIBS="${ZLIB_LIBS}"
> > > > +
> > > > +if test "$MOZ_GLUE_PROGRAM_LDFLAGS" -a "$OS_ARCH" != "WINNT"; then
> > > 
> > > This should be test -z "$MOZ_GLUE_PROGRAM_LDFLAGS", shouldn't it? Or, on the
> > > contrary, just set ZLIB_PROG_LIBS when MOZ_GLUE_PROGRAM_LDFLAGS is set.
> > > I also don't think you need to check OS_ARCH.
> > 
> > We only want to link zlib in the js programs if mozglue is not being linked
> > into them.
> 
> Actually, you do want to link zlib in the js programs if mozglue *is* being
> linked (statically), and only link zlib in mozglue if mozglue is a shared
> library.

Why not if it's static?

>  
> > Checking OS_ARCH was the only way I could convince it to build on Windows.
> > 
> > > 
> > > ::: modules/zlib/src/Makefile.in
> > > @@ -14,5 @@
> > > >  MODULE		= zlib
> > > >  LIBRARY_NAME	= mozz
> > > >  GRE_MODULE	= 1
> > > >  LIBXUL_LIBRARY = 1
> > > > -DIST_INSTALL = 1
> > > 
> > > Is there a particular reason why you remove this?
> > 
> > It somehow allows the zlib symbols to be exported from the mozglue library.
> 
> That sounds unexpected.

It was on advice from khuey.
Attachment #635205 - Flags: review?(mh+mozilla)
(In reply to Benjamin Peterson from comment #18)
> > > > > +MOZ_ARG_ENABLE_BOOL(moz-zlib,
> > > > > +[  --enable-moz-zlib=FLAGS   INTERNAL build with builtin Mozilla zlib],
> > > > > +     [ZLIB_LIBS='$(call EXPAND_LIBNAME_PATH,mozz,../../$(DEPTH)/modules/zlib/src)'
> > > > > +      ZLIB_DIR=])
> > > > 
> > > > I think it would be better to define these in the main configure.in, and
> > > > export them from there, where they can be picked up without an option. The
> > > > check below would then pick these values, provided they're not reset above.

You didn't address this in your patch.

> > > > @@ +3507,5 @@
> > > > > +fi
> > > > > +if test -z "$ZLIB_DIR" -o "$ZLIB_DIR" = no; then
> > > > > +    SYSTEM_ZLIB=
> > > > > +elif test "$ZLIB_DIR" != yes; then
> > > > > +    PKG_CHECK_MODULES(ZLIB, zlib)
> > > > 
> > > > If you're doing that, please do the same in the main configure.
> > > 
> > > The reason I did this, is so is you build js standalone, you get system zlib
> > > by default. This doesn't seem to be what the main configure is interested in.
> > 
> > I meant the PKG_CHECK_MODULES part. The main configure doesn't use that.
> 
> Yes, I was also talking about that. I didn't think this was a useful
> behaviour for the main configure to have.

Having native zlib enabled by default surely isn't wanted in the main configure at this point (although i do think we could), but using pkg-config when it is enabled surely is.

> > Actually, you do want to link zlib in the js programs if mozglue *is* being
> > linked (statically), and only link zlib in mozglue if mozglue is a shared
> > library.
> 
> Why not if it's static?

See comment 9. Linking zlib in static mozglue and only there, on unix, opens a can of worms you don't want to open.

> > > Checking OS_ARCH was the only way I could convince it to build on Windows.
> > > 
> > > > 
> > > > ::: modules/zlib/src/Makefile.in
> > > > @@ -14,5 @@
> > > > >  MODULE		= zlib
> > > > >  LIBRARY_NAME	= mozz
> > > > >  GRE_MODULE	= 1
> > > > >  LIBXUL_LIBRARY = 1
> > > > > -DIST_INSTALL = 1
> > > > 
> > > > Is there a particular reason why you remove this?
> > > 
> > > It somehow allows the zlib symbols to be exported from the mozglue library.
> > 
> > That sounds unexpected.
> 
> It was on advice from khuey.

The advice was probably about VISIBILITY_FLAGS, not DIST_INSTALL. It shouldn't matter much that DIST_INSTALL is removed, anyways, but it would be better to avoid unrelated changes.
Attached patch PoC (obsolete) — Splinter Review
This should do what you want. It would need some testing, though. (I pushed it to try, for starters)
Attached patch PoC (obsolete) — Splinter Review
It's better with the new files.
Attachment #635292 - Attachment is obsolete: true
The latest patch(In reply to Mike Hommey [:glandium] from comment #20)
> Created attachment 635292 [details] [diff] [review]
> PoC
> 
> This should do what you want. It would need some testing, though. (I pushed
> it to try, for starters)

Thank you very much. It's broken on Windows, though: https://tbpl.mozilla.org/?tree=Try&rev=c6c9b86590f3
It also fails for standalone js builds.
(In reply to Benjamin Peterson from comment #23)
> It also fails for standalone js builds.

On what platform, with what error?
(In reply to Mike Hommey [:glandium] from comment #24)
> (In reply to Benjamin Peterson from comment #23)
> > It also fails for standalone js builds.
> 
> On what platform, with what error?

Linux 64:

$ mkdir _DBG.OBJ && cd _DBG.OBJ
$ ../configure --enable-debug --disable-optimize && make -j9
...
/usr/bin/python2.7 ../config/pythonpath.py -I./config ../config/expandlibs_exec.py --depend .deps/libmozjs.pp --target libmozjs.so --uselist --  c++  -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -Wno-long-long -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -pthread -pipe  -DDEBUG -D_DEBUG -DTRACING -g -fno-omit-frame-pointer  -DUSE_SYSTEM_MALLOC=1 -DENABLE_ASSEMBLER=1 -DENABLE_JIT=1 -fPIC -shared -Wl,-z,defs -Wl,--gc-sections -Wl,-h,libmozjs.so -o libmozjs.so  bignum-dtoa.o bignum.o cached-powers.o diy-fp.o double-conversion.o fast-dtoa.o fixed-dtoa.o strtod.o jsalloc.o jsanalyze.o jsapi.o jsarray.o jsatom.o jsbool.o jsclone.o jscntxt.o jscompartment.o jsdate.o jsdbgapi.o jsdhash.o jsdtoa.o jsexn.o jsfriendapi.o jsfun.o jsgc.o jscrashreport.o jshash.o jsinfer.o jsinterp.o jsiter.o jslog2.o jsmath.o jsnativestack.o jsnum.o jsobj.o json.o jsonparser.o jsopcode.o jsproxy.o jsprf.o jsprobes.o jspropertycache.o jspropertytree.o jsreflect.o jsscope.o jsscript.o jsstr.o jstypedarray.o jsutil.o jswatchpoint.o jsweakmap.o jswrapper.o jsxml.o prmjtime.o sharkctl.o ArgumentsObject.o ScopeObject.o Debugger.o GlobalObject.o MethodGuard.o ObjectImpl.o Stack.o String.o BytecodeCompiler.o BytecodeEmitter.o FoldConstants.o ParseMaps.o ParseNode.o Parser.o SemanticAnalysis.o TokenStream.o TreeContext.o TestingFunctions.o LifoAlloc.o MapObject.o MemoryMetrics.o RegExpObject.o RegExpStatics.o RegExp.o Marking.o Memory.o Statistics.o StringBuffer.o Unicode.o Xdr.o MethodJIT.o StubCalls.o Compiler.o FrameState.o FastArithmetic.o FastBuiltins.o FastOps.o LoopState.o StubCompiler.o MonoIC.o PolyIC.o ImmutableSync.o InvokeHelpers.o Retcon.o TrampolineCompiler.o ExecutableAllocator.o PageBlock.o YarrInterpreter.o YarrPattern.o YarrSyntaxChecker.o Logging.o ExecutableAllocatorPosix.o OSAllocatorPosix.o ARMAssembler.o MacroAssemblerARM.o MacroAssemblerX86Common.o YarrJIT.o jsperf.o pm_linux.o HashFunctions.o    -lpthread    -Wl,-rpath-link,./dist/bin -Wl,-rpath-link,/usr/local/lib     -ldl  -lm -ldl  
jsutil.o: In function `js::TryCompressString(unsigned char const*, unsigned long, unsigned char*, unsigned long*)':
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:59: undefined reference to `deflateInit_'
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:64: undefined reference to `deflate'
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:65: undefined reference to `deflateEnd'
jsutil.o: In function `js::DecompressString(unsigned char const*, unsigned long, unsigned char*, unsigned long)':
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:86: undefined reference to `inflateInit_'
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:96: undefined reference to `inflate'
/home/benjamin/dev/repos/mozilla/central/js/src/jsutil.cpp:98: undefined reference to `inflateEnd'
collect2: ld returned 1 exit status
make[1]: *** [libmozjs.so] Error 1
make[1]: Leaving directory `/home/benjamin/dev/repos/mozilla/central/js/src/_DBG.OBJ'
make: *** [default] Error 2


(Note this is with the patch for bug 761723, which actually uses zlib, applied.)
Attached patch PoC (obsolete) — Splinter Review
This works on win32 (but fails on win64 because mozjs is not built as a separate lib there, and that makes the assumptions from this patch fail)
Attachment #635300 - Attachment is obsolete: true
Attachment #640579 - Flags: review?(khuey)
Attachment #638413 - Attachment is obsolete: true
(In reply to Mike Hommey [:glandium] from comment #27)
> Created attachment 640579 [details] [diff] [review]
> Link the javascript engine against zlib

This passed try: https://tbpl.mozilla.org/?tree=Try&rev=d78cdbe17c10
I also tested Benjamin's standalone case from comment 25.
Attachment #635205 - Flags: review?(mh+mozilla)
Attached patch make standalone builds work (obsolete) — Splinter Review
This one-liner is needed to make JS standalone builds work.
Ah damn, my test was useless because there's nothing in js actually using zlib, so i didn't get the failure. Unfortunately, your patch breaks windows.
Comment on attachment 640579 [details] [diff] [review]
Link the javascript engine against zlib

Needs a fix.
Attachment #640579 - Flags: review?(khuey)
Attachment #640579 - Attachment is obsolete: true
Attachment #640654 - Attachment is obsolete: true
Attachment #635205 - Attachment is obsolete: true
Assignee: nobody → mh+mozilla
https://hg.mozilla.org/mozilla-central/rev/4be97839a566
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Depends on: 776305
Profiled build on Linux is still broken:

 % make -f client.mk profiledbuild
...
/usr/bin/python2.7 /var/tmp/mozilla-central/js/src/config/pythonpath.py -I../config /var/tmp/mozilla-central/js/src/config/expandlibs_exec.py --depend .deps/js.pp --target js --uselist --  c++ -o js  -pedantic -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -Wno-long-long -march=native -Wno-delete-non-virtual-dtor -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -pthread -pipe  -DNDEBUG -DTRIMMED -fprofile-generate -O3 -fomit-frame-pointer js.i_o jsoptparse.i_o jsheaptools.i_o   -lpthread -fuse-linker-plugin -Wl,-O1,--hash-style=gnu,--no-keep-memory -Wl,--icf=safe  -fprofile-generate -Wl,-rpath-link,../../../dist/bin -Wl,-rpath-link,/var/tmp/moz-build-dir/dist/lib   -L../../../dist/bin -L../../../dist/lib -L/var/tmp/moz-build-dir/dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl ../editline/libeditline.a ../libjs_static.a -lz -Wl,--whole-archive ../../../dist/lib/libmozglue.a ../../../dist/lib/libmemory.a -Wl,--no-whole-archive -rdynamic -ldl    
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'inflateInit_' is not defined locally
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'inflate' is not defined locally
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'inflateEnd' is not defined locally
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'deflateInit_' is not defined locally
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'deflate' is not defined locally
/usr/lib/gcc/x86_64-pc-linux-gnu/4.8.0/../../../../x86_64-pc-linux-gnu/bin/ld: error: hidden symbol 'deflateEnd' is not defined locally
collect2: error: ld returned 1 exit status
make[6]: *** [js] Error 1
Please ignore. I see this is fixed in Bug 776305.
Depends on: 776704
Depends on: 773967
Depends on: 780976
Depends on: 825915
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.