Status

defect
RESOLVED FIXED
7 years ago
Last year

People

(Reporter: Benjamin, Assigned: glandium)

Tracking

unspecified
mozilla16
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 9 obsolete attachments)

Reporter

Description

7 years ago
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?
Reporter

Comment 3

7 years ago
Posted 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.
Assignee

Comment 4

7 years ago
(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.
Reporter

Comment 5

7 years ago
(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.
Assignee

Comment 6

7 years ago
(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).
Reporter

Comment 7

7 years ago
Posted 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
Assignee

Comment 8

7 years ago
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.
Assignee

Comment 9

7 years ago
(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.
Reporter

Comment 10

7 years ago
(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.
Assignee

Comment 11

7 years ago
(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.
Reporter

Comment 12

7 years ago
Attachment #633410 - Attachment is obsolete: true
Attachment #633863 - Flags: review?(mh+mozilla)
Assignee

Comment 13

7 years ago
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-
Assignee

Comment 14

7 years ago
Please note that I landed bug 763987 and bug 764046, which conflicts with some parts of your patch.
Reporter

Comment 15

7 years ago
(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.
Assignee

Comment 16

7 years ago
(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.
Reporter

Comment 17

7 years ago
Posted patch address review comments (obsolete) — Splinter Review
Attachment #633863 - Attachment is obsolete: true
Reporter

Comment 18

7 years ago
(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.
Reporter

Updated

7 years ago
Attachment #635205 - Flags: review?(mh+mozilla)
Assignee

Comment 19

7 years ago
(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.
Assignee

Comment 20

7 years ago
Posted patch PoC (obsolete) — Splinter Review
This should do what you want. It would need some testing, though. (I pushed it to try, for starters)
Assignee

Comment 21

7 years ago
Posted patch PoC (obsolete) — Splinter Review
It's better with the new files.
Assignee

Updated

7 years ago
Attachment #635292 - Attachment is obsolete: true
Reporter

Comment 22

7 years ago
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
Reporter

Comment 23

7 years ago
It also fails for standalone js builds.
Assignee

Comment 24

7 years ago
(In reply to Benjamin Peterson from comment #23)
> It also fails for standalone js builds.

On what platform, with what error?
Reporter

Comment 25

7 years ago
(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.)
Assignee

Comment 26

7 years ago
Posted 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)
Assignee

Updated

7 years ago
Attachment #635300 - Attachment is obsolete: true
Assignee

Comment 27

7 years ago
Attachment #640579 - Flags: review?(khuey)
Assignee

Updated

7 years ago
Attachment #638413 - Attachment is obsolete: true
Assignee

Comment 28

7 years ago
(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.
Reporter

Updated

7 years ago
Attachment #635205 - Flags: review?(mh+mozilla)
Reporter

Comment 29

7 years ago
Posted patch make standalone builds work (obsolete) — Splinter Review
This one-liner is needed to make JS standalone builds work.
Assignee

Comment 30

7 years ago
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.
Assignee

Comment 31

7 years ago
Comment on attachment 640579 [details] [diff] [review]
Link the javascript engine against zlib

Needs a fix.
Attachment #640579 - Flags: review?(khuey)
Assignee

Updated

7 years ago
Attachment #640579 - Attachment is obsolete: true
Assignee

Updated

7 years ago
Attachment #640654 - Attachment is obsolete: true
Assignee

Updated

7 years ago
Attachment #635205 - Attachment is obsolete: true
Assignee

Updated

7 years ago
Assignee: nobody → mh+mozilla
https://hg.mozilla.org/mozilla-central/rev/4be97839a566
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee

Updated

7 years ago
Depends on: 776305

Comment 35

7 years ago
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

Comment 36

7 years ago
Please ignore. I see this is fixed in Bug 776305.
Depends on: 776704
Depends on: 773967
Depends on: 780976
Assignee

Updated

7 years ago
Depends on: 825915

Updated

Last year
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.