We have a flag set on all Linkables, cxx_link, denoting whether there's anything being linked into them that requires C++. We do this even when we link against shared libraries that required C++. But if these libraries don't export C++ interfaces, there's no reason that the things linking against them should require C++. Therefore, ignore shared libraries when making the determination of whether an object requires C++ or not. I think we *could* have problems here if the C code referenced C++ symbols through inline asm or similar. But I think this is *really* unlikely.

We currently only honor LIB_IS_C_ONLY for cases where we set a LIBRARY (and, implicitly, REAL_LIBRARY) and FORCE_SHARED_LIB. For many libraries, such as the libraries from NSS, we never set REAL_LIBRARY, which leads to not setting REAL_LIBRARY, which leads to not honoring LIB_IS_C_ONLY. This practice has not been harmful thus far (except perhaps linking in more things than necessary to our NSS shared libraries), but on some platforms, linking with the C++ compiler will drag in more things than we would like. Consulting LIBRARY first should not be necessary; checking FORCE_SHARED_LIB should be enough to tell us if we're building a shared library for the purposes of honoring LIB_IS_C_ONLY.

Comment on attachment 8883110 [details] [diff] [review] part 2 - honor LIB_IS_C_ONLY in more cases Review of attachment 8883110 [details] [diff] [review]: ----------------------------------------------------------------- "we never set REAL_LIBRARY, which leads to not setting REAL_LIBRARY," One of those is probably meant to be LIBRARY.

Both of these libraries call into libm for various reasons, but by linking with the C++ compiler on most platforms, they never had to declare their dependency on libm. Future changes will make these libraries link with the C compiler, which won't automatically link with libm, so we need to make the dependency explicit prior to that change. (I have seen the other two patches on the bug fail on try without this change. They also fail in a local x86-64 Linux build, but I have not seen the same failure on try, which seems peculiar to me. Nonetheless, linking to libm everywhere seems like the correct thing to do.)

Comment on attachment 8883578 [details] [diff] [review] part 1a - link libavutil and libavcodec with libm Review of attachment 8883578 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/ffvpx/libavutil/moz.build @@ +54,5 @@ > NO_VISIBILITY_FLAGS = True > > OS_LIBS += CONFIG['REALTIME_LIBS'] > +OS_LIBS += ['m'] > + Please remove the trailing whitespace here.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1f41d6e14484 part 1 - make C++ linking for Linkable ignore shared libraries; r=glandium https://hg.mozilla.org/integration/mozilla-inbound/rev/de188948af11 part 2 - link libavutil and libavcodec with libm; r=rillian https://hg.mozilla.org/integration/mozilla-inbound/rev/b06c107c096b part 3 - honor LIB_IS_C_ONLY in more cases; r=glandium

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fd48ef4aea6d followup - don't explicitly link to libm on Windows; r=bustage

(In reply to Nathan Froyd [:froydnj] from comment #4) > (I have seen the other two patches on the bug fail on try without this > change. > They also fail in a local x86-64 Linux build, but I have not seen the same > failure on try, which seems peculiar to me. Nonetheless, linking to libm > everywhere seems like the correct thing to do.) Did your local build have `--enable-stdcxx-compat`? per bug 1256642 comment 2 that was the thing that made a mess of things in my original work. Thanks for fixing this!

https://hg.mozilla.org/mozilla-central/rev/1f41d6e14484 https://hg.mozilla.org/mozilla-central/rev/de188948af11 https://hg.mozilla.org/mozilla-central/rev/b06c107c096b https://hg.mozilla.org/mozilla-central/rev/fd48ef4aea6d