Last Comment Bug 648721 - Fix linking of xpcshell and plugin-container (X libs path missing)
: Fix linking of xpcshell and plugin-container (X libs path missing)
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Landry Breuil (:gaston)
:
:
Mentors:
Depends on:
Blocks: openbsdmeta
  Show dependency treegraph
 
Reported: 2011-04-09 01:15 PDT by Landry Breuil (:gaston)
Modified: 2011-04-18 08:19 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD (528 bytes, patch)
2011-04-11 04:35 PDT, Landry Breuil (:gaston)
no flags Details | Diff | Splinter Review
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD (580 bytes, patch)
2011-04-11 05:13 PDT, Landry Breuil (:gaston)
no flags Details | Diff | Splinter Review
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD (580 bytes, patch)
2011-04-11 07:36 PDT, Landry Breuil (:gaston)
mh+mozilla: review+
Details | Diff | Splinter Review
same patch for js/src/configure.in, and ac_define(avmplus_unix) if target=openbsd (1.02 KB, patch)
2011-04-17 14:38 PDT, Landry Breuil (:gaston)
mh+mozilla: review-
Details | Diff | Splinter Review

Description Landry Breuil (:gaston) 2011-04-09 01:15:07 PDT
currently, when linking xpcshell | plugin-container, it fails with the following error :

c++ -o xpcshell -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -
Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -O2 -pipe -fno-strict-aliasing -fshort-wchar -pthread -pipe -DND
EBUG -DTRIMMED  xpcshell.o   -pthread   -Wl,-rpath,'/usr/local/lib/firefox-4.0' -Wl,-rpath-link,/usr/obj/mozilla-firefox-4.0/mozilla-2.0/di
st/bin -Wl,-rpath-link,/usr/local/lib -L/usr/obj/mozilla-firefox-4.0/mozilla-2.0/dist/bin -L../../../../dist/bin -L../../../../dist/lib ../
../../../dist/lib/libxpcomglue_s.a -L/usr/obj/mozilla-firefox-4.0/mozilla-2.0/dist/bin -lxpcom -lmozalloc -lxul -L/usr/obj/mozilla-firefox-
4.0/mozilla-2.0/dist/bin -lxpcom -lmozalloc -lxul  -L/usr/local/lib -lplds4 -lplc4 -lnspr4 -pthread -lc 
/usr/bin/ld: warning: libSM.so.8.0, needed by /usr/X11R6/lib/libXt.so.10.0, not found (try using -rpath or -rpath-link)
/usr/bin/ld: warning: libICE.so.9.0, needed by /usr/X11R6/lib/libXt.so.10.0, not found (try using -rpath or -rpath-link)
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcSaveYourselfDone'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `IceConnectionNumber'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcDeleteProperties'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `IceProcessMessages'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcRequestSaveYourselfPhase2'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcInteractDone'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcCloseConnection'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcSetProperties'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcOpenConnection'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcInteractRequest'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcModifyCallbacks'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcClientID'
/usr/X11R6/lib/libXt.so.10.0: undefined reference to `SmcGetIceConnection'

That's because libxul.so is linked against some X libs (libXt needs libSM & libICE), and has RPATH set to /usr/X11R6/lib (in ldd output), but that rpath is not set in the build config for xpcshell|plugin-container link step.

I don't know if it's a local build config issue, if XT_LIBS or XLIBS should bundle -lSM & libICE, but atm my workaround is to add -Wl,-rpath-link,/usr/X11R6/lib to LIBS in ipc/app/Makefile and js/src/xpconnect/shell/Makefile. This is definitely not a good fix so i'm looking for something better :
- a real fix in my build env that i didn't found yet ?
- add XLDFLAGS to LIBS as done in several places (browser/app/Makefile, layout/build/Makefile...) ?
- globally add that -rpath-link dir to MOZ_FIX_LINK_PATHS ?
Comment 1 Mike Hommey [:glandium] 2011-04-09 09:52:18 PDT
It looks like your problem is that libXt.so.10.0 is not linked against libSm and libICE, which would be a problem with your libXt build. FWIW, this is what Debian's libXt says:

$ readelf -d /usr/lib/libXt.so.6 | grep NEEDED
 0x0000000000000001 (NEEDED)             Shared library: [libSM.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libICE.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libX11.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]

You should see something similar. If you don't, your libXt is broken.
Comment 2 Landry Breuil (:gaston) 2011-04-09 15:31:14 PDT
$ldd /usr/X11R6/lib/libXt.so.10.0                      
/usr/X11R6/lib/libXt.so.10.0:
        Start            End              Type Open Ref GrpRef Name
        000000020c4b0000 000000020c911000 dlib 1    0   0      /usr/X11R6/lib/libXt.so.10.0
        0000000200a0e000 0000000200e16000 rlib 0    1   0      /usr/X11R6/lib/libSM.so.8.0
        0000000205741000 0000000205b5c000 rlib 0    2   0      /usr/X11R6/lib/libICE.so.9.0
        0000000205b5c000 0000000206090000 rlib 0    1   0      /usr/X11R6/lib/libX11.so.14.0
        0000000207fc4000 00000002083e0000 rlib 0    2   0      /usr/X11R6/lib/libxcb.so.2.2
        00000002077e5000 0000000207be7000 rlib 0    3   0      /usr/X11R6/lib/libpthread-stubs.so.1.0
        000000020ec96000 000000020f099000 rlib 0    3   0      /usr/X11R6/lib/libXau.so.9.0
        0000000203d58000 000000020415d000 rlib 0    3   0      /usr/X11R6/lib/libXdmcp.so.10.0

 $objdump -p /usr/X11R6/lib/libXt.so.10.0  | grep NEEDED
  NEEDED      libSM.so.8.0
  NEEDED      libICE.so.9.0
  NEEDED      libX11.so.14.0
  NEEDED      libxcb.so.2.2
  NEEDED      libpthread-stubs.so.1.0
  NEEDED      libXau.so.9.0
  NEEDED      libXdmcp.so.10.0

libXt is correct.. however, libxul is not :

$objdump -p /usr/local/lib/firefox-4.0/libxul.so.23.0  | grep NEED.*libX
  NEEDED      libXrender.so.5.0
  NEEDED      libX11.so.14.0
  NEEDED      libXext.so.11.0
  NEEDED      libXxf86vm.so.5.0
  NEEDED      libXdamage.so.3.1
  NEEDED      libXfixes.so.5.0
  NEEDED      libXau.so.9.0
  NEEDED      libXdmcp.so.10.0
  NEEDED      libXinerama.so.5.0
  NEEDED      libXi.so.11.0
  NEEDED      libXrandr.so.6.1
  NEEDED      libXcursor.so.4.0
  NEEDED      libXcomposite.so.3.0
  NEEDED      libXt.so.10.0

To me, either XT_LIBS should be '-lXt -lSM -lICE', or libxul should be explicitely linked against SM and ICE (using X_PRE_LIBS, which contains -lSM -lICE in my config.status.. ??)
Comment 3 Landry Breuil (:gaston) 2011-04-09 15:35:13 PDT
Bah, i wanted to show that SM and ICE were not in NEEDED list.. grepping for libX wasnt the right way.

$objdump -p /usr/local/lib/firefox-4.0/libxul.so.23.0  | egrep 'NEED.*(SM|ICE)'
-> that returns nothing, so the dependency on those two libs is not recorded in libxul linking. Libxul finds them (by luck) at runtime because of
$objdump -p /usr/local/lib/firefox-4.0/libxul.so.23.0 | grep RPATH
  RPATH       /usr/X11R6/lib:/usr/local/lib/firefox-4.0
Comment 4 Mike Hommey [:glandium] 2011-04-10 01:48:45 PDT
libxul doesn't directly use libSM and libICE symbols, so it doesn't need to directly link against them. If OpenBSD requires that, then its dynamic linker is broken.
Your problem boils down to this: /usr/X11R6/lib is not in the default search path for libraries, and instead of doing so, you are self inflicting pain with an half broken setup.
Your solution to the problem is to add an rpath so that libxul (and I guess anything else requiring X libraries) can find its dependencies in /usr/X11R6/lib. The secondary problem is that indirect dependencies (those that libxul DON'T need, but things it needs do, e.g. libSM through libXt) can't be found because the rpath is only on libxul, not libraries in /usr/X11R6/lib.
So, in order for libSM and libICE to be found in /usr/X11R6/lib, you choose to use this rpath from libxul and link them to it.
Well, this is bound to be a failure. Imagine one of the following (hypothetical) scenarios:
- libSM changes SO version
- libXt starts depending on /usr/X11R6/lib/libfoo.so
In both cases, your libxul (and anything depending on libXt) will be broken by an updated libXt, while it really shouldn't need to be.

So here is my advice: either add an rpath to all /usr/X11R6/lib libraries depending on other /usr/X11R6/lib libraries, or add /usr/X11R6/lib in the default search path.
Or even better, do like linux distros, get rid of /usr/X11R6 (which, btw, is wrong, since we're using X11R7, now)
Comment 5 Landry Breuil (:gaston) 2011-04-11 00:43:50 PDT
(In reply to comment #4)
> libxul doesn't directly use libSM and libICE symbols, so it doesn't need to
> directly link against them. If OpenBSD requires that, then its dynamic linker
> is broken.
> Your problem boils down to this: /usr/X11R6/lib is not in the default search
> path for libraries, and instead of doing so, you are self inflicting pain with
> an half broken setup.

No, that's called 'control of dependencies'..

> So here is my advice: either add an rpath to all /usr/X11R6/lib libraries
> depending on other /usr/X11R6/lib libraries, or add /usr/X11R6/lib in the
> default search path.

Changing system libs 'just for mozilla' is a no-go.

After discussion with some of our devs, using -Wl,-rpath-link is the cleanest fix.
So i've end up with adding '-Wl,-rpath-link,${X11BASE}/lib' to MOZ_FIX_LINK_PATHS
in configure.in, which works. X11BASE is an environment variable during the build,
and i didn't find another variable in existing XLDFLAGS,XLIBS,XCFLAGS & friends
that could be used for it. $(x_libraries) is not propagated outside of configure..

Would it be considered an acceptable fix, likely to be commited ?

> Or even better, do like linux distros, get rid of /usr/X11R6 (which, btw, is
> wrong, since we're using X11R7, now)

That's very unlikely to happen, and it's not that because all linux distros do it
that it's a good idea :)
Comment 6 Mike Hommey [:glandium] 2011-04-11 00:59:04 PDT
(In reply to comment #5)
> No, that's called 'control of dependencies'..

No, that's called adding useless dependencies, and is doomed to fail. I'm actually surprised it didn't bring you pain already in the past. A typical case is with transitions, like when libpng changes soname, at which point with your scheme, you end up needing to rebuild everything, while there's only a need to rebuild a few intermediate libraries.
 
> > So here is my advice: either add an rpath to all /usr/X11R6/lib libraries
> > depending on other /usr/X11R6/lib libraries, or add /usr/X11R6/lib in the
> > default search path.
> 
> Changing system libs 'just for mozilla' is a no-go.

It's not 'just for mozilla', that's the whole point.

> After discussion with some of our devs, using -Wl,-rpath-link is the cleanest
> fix.
> So i've end up with adding '-Wl,-rpath-link,${X11BASE}/lib' to
> MOZ_FIX_LINK_PATHS
> in configure.in, which works. X11BASE is an environment variable during the
> build,
> and i didn't find another variable in existing XLDFLAGS,XLIBS,XCFLAGS & friends
> that could be used for it. $(x_libraries) is not propagated outside of
> configure..
> 
> Would it be considered an acceptable fix, likely to be commited ?

If you put it in a openbsd specific part of configure.in, why not. OTOH, is your /usr/X11R6/lib rpath part of the mozilla build already ? If not, there's no point pushing half your "fix".
Comment 7 Landry Breuil (:gaston) 2011-04-11 01:12:55 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > No, that's called 'control of dependencies'..
> 
> No, that's called adding useless dependencies, and is doomed to fail. I'm
> actually surprised it didn't bring you pain already in the past. A typical case
> is with transitions, like when libpng changes soname, at which point with your
> scheme, you end up needing to rebuild everything, while there's only a need to
> rebuild a few intermediate libraries.

We have a different scheme : we control all sonames, be it in the basesystem or in the libraries installed by ports.
When updating packages, if an old library is still used by a package, it's kept on the system in a hidden package, and removed later when nothing uses it anymore.
And bulk builds producing packages to be distributed to users are done on the whole ports tree, so the interdependencies are correct.
 
> > After discussion with some of our devs, using -Wl,-rpath-link is the cleanest
> > fix.
> > So i've end up with adding '-Wl,-rpath-link,${X11BASE}/lib' to
> > MOZ_FIX_LINK_PATHS
> > in configure.in, which works. X11BASE is an environment variable during the
> > build,
> > and i didn't find another variable in existing XLDFLAGS,XLIBS,XCFLAGS & friends
> > that could be used for it. $(x_libraries) is not propagated outside of
> > configure..
> > 
> > Would it be considered an acceptable fix, likely to be commited ?
> 
> If you put it in a openbsd specific part of configure.in, why not. OTOH, is
> your /usr/X11R6/lib rpath part of the mozilla build already ? If not, there's
> no point pushing half your "fix".

it's part of some parts of the tree so it works for other sub libs 'by luck', through :
X_LIBS, FT2_LIBS, XLDFLAGS and MOZ_CAIRO_LIBS containing -L/usr/X11R6/lib
MOZ_GTK2_LIBS and MOZ_PANGO_LIBS containing -L/usr/X11R6/lib -R/usr/X11R6/lib
-R is an equivalent as -rpath in this case.
Comment 8 Mike Hommey [:glandium] 2011-04-11 01:24:43 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > No, that's called 'control of dependencies'..
> > 
> > No, that's called adding useless dependencies, and is doomed to fail. I'm
> > actually surprised it didn't bring you pain already in the past. A typical case
> > is with transitions, like when libpng changes soname, at which point with your
> > scheme, you end up needing to rebuild everything, while there's only a need to
> > rebuild a few intermediate libraries.
> 
> We have a different scheme : we control all sonames, be it in the basesystem or
> in the libraries installed by ports.
> When updating packages, if an old library is still used by a package, it's kept
> on the system in a hidden package, and removed later when nothing uses it
> anymore.
> And bulk builds producing packages to be distributed to users are done on the
> whole ports tree, so the interdependencies are correct.

Yay, even better, if you only rebuild half the system, you end up with 2 versions of a library in a process address space, all because one library or program that doesn't need it pulled the old version. With all the fun that comes with it.

Anyways, this got offtopic. I'll stop here.
Comment 9 Landry Breuil (:gaston) 2011-04-11 01:49:50 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > No, that's called 'control of dependencies'..
> > > 
> > > No, that's called adding useless dependencies, and is doomed to fail. I'm
> > > actually surprised it didn't bring you pain already in the past. A typical case
> > > is with transitions, like when libpng changes soname, at which point with your
> > > scheme, you end up needing to rebuild everything, while there's only a need to
> > > rebuild a few intermediate libraries.
> > 
> > We have a different scheme : we control all sonames, be it in the basesystem or
> > in the libraries installed by ports.
> > When updating packages, if an old library is still used by a package, it's kept
> > on the system in a hidden package, and removed later when nothing uses it
> > anymore.
> > And bulk builds producing packages to be distributed to users are done on the
> > whole ports tree, so the interdependencies are correct.
> 
> Yay, even better, if you only rebuild half the system, you end up with 2
> versions of a library in a process address space, all because one library or
> program that doesn't need it pulled the old version. With all the fun that
> comes with it.

Just for clarification... only the correct version of the library is loaded in the process address space, as the dependency is recorded in the binary at link time with the needed version. Oh, and we don't have/need all the crazy symlinks in usr/lib.
Like it or not, that's how OpenBSD is made, and mozilla builds/runs on it this way. I'm just trying to minimize the local changes i have.
Comment 10 Mike Hommey [:glandium] 2011-04-11 01:58:41 PDT
Just for clarification, taking the example from this bug, considering your first suggestion to add -lSM -lICE when linking libxul:
- libxul depends on libSM, libICE and libXt (but only uses symbols from the latter)
- libXt depends on libSM and libICE.

Now, imagine an upgrade of libSM's soname. You rebuild libXt but not libxul. libxul still depends on the old libSM (but doesn't use it), libXt depends on the new one. Result: both are loaded in the process address space when libxul is loaded, and symbols are very likely to resolve to the old libSM.
Comment 11 Landry Breuil (:gaston) 2011-04-11 04:35:42 PDT
Created attachment 525057 [details] [diff] [review]
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD

configure.in patch adds -Wl,-rpath-link,${X11BASE}/lib to MOZ_FIX_LINK_PATHS in the *-openbsd*) case. X11BASE is set in the environment by the portstree.
It also changes DLL_SUFFIX to obey the environment variable SO_VERSION, which permits us to control the so versionning in the portstree. Don't know if this part is acceptable.
Comment 12 Mike Hommey [:glandium] 2011-04-11 04:46:06 PDT
Comment on attachment 525057 [details] [diff] [review]
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD

>$OpenBSD: patch-configure_in,v 1.19 2011/03/30 21:39:14 landry Exp $
>https://bugzilla.mozilla.org/show_bug.cgi?id=648721
>--- configure.in.orig	Sat Mar 19 00:33:33 2011
>+++ configure.in	Mon Apr 11 10:48:46 2011
>@@ -2676,7 +2676,8 @@ ia64*-hpux*)
> 	;;
> 
> *-openbsd*)
>-    DLL_SUFFIX=".so.1.0"
>+    DLL_SUFFIX=".so.${SO_VERSION}"

Make that '.so.$(if $(SO_VERSION),$(SO_VERSION),1.0)'

>+    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,${X11BASE}/lib'

Make that depending on X11BASE being defined (though the same trick as above can't be used because of the commas)
Comment 13 Landry Breuil (:gaston) 2011-04-11 05:13:15 PDT
Created attachment 525059 [details] [diff] [review]
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD

Hah, that trick will be useful in more patches for SO_VERSION and others usages. Fixed patch, I hope that's what you meant for X11BASE defaulting to /usr/X11R6 if not defined, or rather adjust MOZ_FIX_LINK_PATHS only if ${X11BASE} is not empty ?
Comment 14 Mike Hommey [:glandium] 2011-04-11 05:42:10 PDT
I meant removing the whole -rpath-link, but fair enough. I think you need to replace the double quotes with simple quotes for DLL_SUFFIX.
Comment 15 Landry Breuil (:gaston) 2011-04-11 07:36:56 PDT
Created attachment 525070 [details] [diff] [review]
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD

This time with single quotes, tested still building fine here and SO_VERSION is correctly applied.
Comment 16 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-04-15 07:01:38 PDT
Comment on attachment 525070 [details] [diff] [review]
add -Wl,-rpath-link,X11BASE/lib to MOZ_FIX_LINK_PATHS on OpenBSD

Mike seems to know what's going on here, and he's officially a peer now, so I'm punting this over to him.
Comment 17 Mounir Lamouri (:mounir) 2011-04-17 14:12:06 PDT
Don't we want the same patch for js/src/configure.in?
Comment 18 Landry Breuil (:gaston) 2011-04-17 14:36:12 PDT
It's currently not needed as it seems nothing under js/src links with X libs, but that could be good for the sake of consistency. Testbuilding the following diff against m-c with AC_DEFINE(AVMPLUS_UNIX) in openbsd case while here.
Comment 19 Landry Breuil (:gaston) 2011-04-17 14:38:16 PDT
Created attachment 526629 [details] [diff] [review]
same patch for js/src/configure.in, and ac_define(avmplus_unix) if target=openbsd
Comment 20 Mounir Lamouri (:mounir) 2011-04-17 15:21:49 PDT
I will push the two patches when both will be reviewed. Let's remove the checkin-needed flag in the mean-time.
Comment 21 Mike Hommey [:glandium] 2011-04-17 23:52:14 PDT
Comment on attachment 526629 [details] [diff] [review]
same patch for js/src/configure.in, and ac_define(avmplus_unix) if target=openbsd

> *-openbsd*)
>-    DLL_SUFFIX=".so.1.0"
>+    DLL_SUFFIX='.so.$(if $(SO_VERSION),$(SO_VERSION),1.0)'

this is going to get in the way of the real soname that is being applied to js releases. See bug 628723.

>+    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,$(if $(X11BASE),$(X11BASE),/usr/X11R6)/lib'

you don't need that.
Comment 22 Landry Breuil (:gaston) 2011-04-18 00:11:13 PDT
(In reply to comment #21)
> Comment on attachment 526629 [details] [diff] [review]
> same patch for js/src/configure.in, and ac_define(avmplus_unix) if
> target=openbsd
> 
> > *-openbsd*)
> >-    DLL_SUFFIX=".so.1.0"
> >+    DLL_SUFFIX='.so.$(if $(SO_VERSION),$(SO_VERSION),1.0)'
> 
> this is going to get in the way of the real soname that is being applied to js
> releases. See bug 628723.

Yes, but remember, we (openbsd) override library versionning anyway, and we don't have the symlinks forest.
And besides that, that chunk only sets the 'default value' for DLL_SUFFIX, from my understanding the real soname applied for js releases come on top of that later in configure.in.

> >+    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,$(if $(X11BASE),$(X11BASE),/usr/X11R6)/lib'
> 
> you don't need that.

Mh... then mounir, can you explain what you wanted in comment #17 ?

For things to work fine for me, i only need the DLL_SUFFIX tweak to obey SO_VERSION if present in the env, and set AC_DEFINE(AVMPLUS_UNIX). (well that and https://bugzilla.mozilla.org/show_bug.cgi?id=589754 but that's a different issue)
Comment 23 Mike Hommey [:glandium] 2011-04-18 01:36:37 PDT
(In reply to comment #22)
> (In reply to comment #21)
> > Comment on attachment 526629 [details] [diff] [review]
> > same patch for js/src/configure.in, and ac_define(avmplus_unix) if
> > target=openbsd
> > 
> > > *-openbsd*)
> > >-    DLL_SUFFIX=".so.1.0"
> > >+    DLL_SUFFIX='.so.$(if $(SO_VERSION),$(SO_VERSION),1.0)'
> > 
> > this is going to get in the way of the real soname that is being applied to js
> > releases. See bug 628723.
> 
> Yes, but remember, we (openbsd) override library versionning anyway, and we
> don't have the symlinks forest.
> And besides that, that chunk only sets the 'default value' for DLL_SUFFIX, from
> my understanding the real soname applied for js releases come on top of that
> later in configure.in.

You'll end up with libmozjs185.so.1.0.1.0

> > >+    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,$(if $(X11BASE),$(X11BASE),/usr/X11R6)/lib'
> > 
> > you don't need that.
> 
> Mh... then mounir, can you explain what you wanted in comment #17 ?

He asked if it was needed. Which it isn't.
 
> For things to work fine for me, i only need the DLL_SUFFIX tweak to obey
> SO_VERSION if present in the env

Why do you want to override the so version of things that come with one ?

> and set AC_DEFINE(AVMPLUS_UNIX).

That should be a separate bug.
Comment 24 Landry Breuil (:gaston) 2011-04-18 01:50:58 PDT
(In reply to comment #23)
> (In reply to comment #22)
> > (In reply to comment #21)
> > > Comment on attachment 526629 [details] [diff] [review]
> > > same patch for js/src/configure.in, and ac_define(avmplus_unix) if
> > > target=openbsd
> > > 
> > > > *-openbsd*)
> > > >-    DLL_SUFFIX=".so.1.0"
> > > >+    DLL_SUFFIX='.so.$(if $(SO_VERSION),$(SO_VERSION),1.0)'
> > > 
> > > this is going to get in the way of the real soname that is being applied to js
> > > releases. See bug 628723.
> > 
> > Yes, but remember, we (openbsd) override library versionning anyway, and we
> > don't have the symlinks forest.
> > And besides that, that chunk only sets the 'default value' for DLL_SUFFIX, from
> > my understanding the real soname applied for js releases come on top of that
> > later in configure.in.
> 
> You'll end up with libmozjs185.so.1.0.1.0

And is it different from what we have right now ? I don't think so, since it defaults to 1.0.

> > > >+    MOZ_FIX_LINK_PATHS='-Wl,-rpath-link,$(LIBXUL_DIST)/bin -Wl,-rpath-link,$(prefix)/lib -Wl,-rpath-link,$(if $(X11BASE),$(X11BASE),/usr/X11R6)/lib'
> > > 
> > > you don't need that.
> > 
> > Mh... then mounir, can you explain what you wanted in comment #17 ?
> 
> He asked if it was needed. Which it isn't.

Fine, disregard that part then.

> > For things to work fine for me, i only need the DLL_SUFFIX tweak to obey
> > SO_VERSION if present in the env
> 
> Why do you want to override the so version of things that come with one ?

As already said, the ports tree controls the so versionning. Things are this way. Do i argue about the symlink forest linux uses ?

> > and set AC_DEFINE(AVMPLUS_UNIX).
> 
> That should be a separate bug.

https://bugzilla.mozilla.org/show_bug.cgi?id=650742
Comment 25 Mike Hommey [:glandium] 2011-04-18 02:14:23 PDT
(In reply to comment #24)
> > You'll end up with libmozjs185.so.1.0.1.0
> 
> And is it different from what we have right now ? I don't think so, since it
> defaults to 1.0.

Which is why you should set DLL_SUFFIX to .so, and either set SRCREL_VERSION in your ports, or override change Makefile.in to use SO_VERSION when there is one. You'll also need to modify Makefile.in to not create the "symlink forest".

And since that all depends on release related changes from bug 628723, which I don't think have landed yet, you should file a new bug for that and CC Wes. Anyways, if you only build the js library as part of firefox build, it should be built statically.
Comment 26 Landry Breuil (:gaston) 2011-04-18 02:38:15 PDT
(In reply to comment #25)
> (In reply to comment #24)
> > > You'll end up with libmozjs185.so.1.0.1.0
> > 
> > And is it different from what we have right now ? I don't think so, since it
> > defaults to 1.0.
> 
> Which is why you should set DLL_SUFFIX to .so, and either set SRCREL_VERSION in
> your ports, or override change Makefile.in to use SO_VERSION when there is one.
> You'll also need to modify Makefile.in to not create the "symlink forest".
> 
> And since that all depends on release related changes from bug 628723, which I
> don't think have landed yet, you should file a new bug for that and CC Wes.
> Anyways, if you only build the js library as part of firefox build, it should
> be built statically.

Right, at some point in time (during the betas ?) i had libmozjs.so.X.Y installed but it seems it's not the case anymore with firefox 4.

628723 landed in the meantime, but we're not using js185 since it is broken on sparc64. I'll fill a separate bug for it when needed.

In the meantime, readding checkin-needed for the first patch, the second patch can be discarded.
Comment 27 Mounir Lamouri (:mounir) 2011-04-18 03:18:39 PDT
Pushed in cedar:
http://hg.mozilla.org/projects/cedar/rev/ea5232835539
Comment 28 Mounir Lamouri (:mounir) 2011-04-18 08:19:12 PDT
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ea5232835539

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