In bug #624422, we're creating various libraries with the same SONAME but different name, overriding -Wl,-h in LDFLAGS for that. In comment #67 there, Mike proposed: > Ideally, we would avoid the original -Wl,-h to be passed on the command line > from MKSHLIB. Ideally, we'd define a SONAME here (or moz.build), that > defaults to $(notdir $@) in config/rules.mk, and change the various MKSHLIB > definitions in configure.in. That could probably be done in a followup bug.

Moving to good-next - this looks heavy for a starter bug. Mike, thanks for your ongoing awesomeness flagging and tagging these.

I'd like to work on this bug. Can someone please assign it to me?

We don't usually assign bugs to new bugzilla users (in the sense of the "Assigned To" field), but feel free to work on it. If you need assistance, please hop on irc, in the #build channel, or ask your questions here (but it's usually better on irc)

This is first patch in a series of two. It allows specifying SONAME in moz.build file. If no SONAME is specified, gnu ld by defaults uses the filename of this library as the SONAME in the binaries that link to this library. Also, only linux platform is supported for now. I haven't looked into how other *nix platforms deal with SONAME.

This is the second patch and final patch in the series. The current implementation of the solution outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=624422#c39 uses LDFLAGS in Makefile.in to specify the SONAME. This patch uses the new SONAME option in moz.build to do the same, and removes the LDFLAGS manipulation from Makefile.in. Note, however, that the Makefile.in is still needed to specify the --no-as-needed flag. Should this be moved to moz.build as well?

I realized that the Makefile.in of the stub library is empty a. This patch essentially deletes Makefile.in from widget/gtk/mozgtk/stub directory

Fixed an incorrect key lookup. Also, I will add the tests once the basic design is approved

Comment on attachment 8450159 [details] [diff] [review] 0001-Bug-1000994-Allow-specifying-SONAME-for-shared-libs-.patch Review of attachment 8450159 [details] [diff] [review]: ----------------------------------------------------------------- The approach is sound, but you'll need some changes before landing this, see below. Note for the future, please avoid the [PATCH] headers in the patch summary. (they're not stripped when importing in mercurial) ::: config/rules.mk @@ +495,5 @@ > EXTRA_DSO_LDOPTS += -Wl,--version-script,$(LD_VERSION_SCRIPT) > EXTRA_DEPS += $(LD_VERSION_SCRIPT) > endif > +ifdef SONAME > +EXTRA_DSO_LDOPTS += -Wl,-soname,$(DLL_PREFIX)$(SONAME)$(DLL_SUFFIX) This needs to be done in configure.in and js/src/configure.in instead. Check all the $(notdir $@) things in the various MKSHLIB/MKCSHLIB values. ::: python/mozbuild/mozbuild/frontend/emitter.py @@ +386,5 @@ > > + soname = sandbox.get('SONAME') > + if soname: > + if final_lib or sandbox.get('FORCE_STATIC_LIB'): > + raise SandboxValidationError('SONAME applicable only for shared libaries') The error message should be different for the final_lib case, because the final_lib might actually be a shared library. ::: python/mozbuild/mozbuild/frontend/sandbox_symbols.py @@ +409,5 @@ > + """The soname of the shared object currently being linked > + > + soname is the "logical name" of a shared object, often used to provide > + version backwards compatibility. This variable only makes sense for > + shared objects, and currently supported only on Linux. In practice, that's Linux, BSDs and probably others.

Comment on attachment 8450034 [details] [diff] [review] 0002-Bug-1000994-Use-SONAME-to-implement-solution-propose.patch Review of attachment 8450034 [details] [diff] [review]: ----------------------------------------------------------------- Please change the patch summary to be self-describing (instead of referring to something written in comments in a bug).

Thanks for the review Mike. I've made the changes as per your feedback.

I have changed the patch summary to include more details.

Comment on attachment 8452116 [details] [diff] [review] Rev2 - Allow SONAME in moz.build files Review of attachment 8452116 [details] [diff] [review]: ----------------------------------------------------------------- You're missing the config.mk changes to avoid SONAME being defined in Makefile.in files. ::: config/rules.mk @@ +180,5 @@ > +ifdef SONAME > +DSO_SONAME = $(DLL_PREFIX)$(SONAME)$(DLL_SUFFIX) > +else > +DSO_SONAME = $(notdir $@) > +endif You have a trailing whitespace here. ::: python/mozbuild/mozbuild/frontend/sandbox_symbols.py @@ +409,5 @@ > + """The soname of the shared object currently being linked > + > + soname is the "logical name" of a shared object, often used to provide > + version backwards compatibility. This variable makes sense only for > + shared objects, and is supported only on Linux, NetBSD and Solaris. There's actually more than that list of OSes. You should just say something like "some unix platforms".

Thanks for the review Mike. I've added the missing changes to config.mk. I forgot to include them in the previous patch. I've also fixed the other issues you pointed out.

https://tbpl.mozilla.org/?tree=Try&rev=3ce9526b0207 https://tbpl.mozilla.org/?tree=Try&rev=f81e8a567f8d (Note this one is expected to go orange, because it's a Gtk+3 build, see Elm)

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e565152b016 https://hg.mozilla.org/integration/mozilla-inbound/rev/c23d678b51c8

https://hg.mozilla.org/mozilla-central/rev/5e565152b016 https://hg.mozilla.org/mozilla-central/rev/c23d678b51c8

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #17) > https://hg.mozilla.org/mozilla-central/rev/5e565152b016 This one caused a bustage. Bug 1036747