Closed Bug 1256642 Opened 9 years ago Closed 8 years ago

Make the build system infer LIB_IS_C_ONLY

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: ted, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #0) > This Makefile only contains `LIB_IS_C_ONLY = 1`: > https://hg.mozilla.org/mozilla-central/file/ > 5e14887312d4523ab59c3f6c6c94a679cf42b496/config/external/sqlite/Makefile.in > > The only function of that is to use the C compiler instead of the C++ > compiler for linking the library: > https://dxr.mozilla.org/mozilla-central/rev/ > d6ee82b9a74155b6bfd544166f036fc572ae8c56/config/rules.mk#151 > > But note: > https://dxr.mozilla.org/mozilla-central/rev/ > d6ee82b9a74155b6bfd544166f036fc572ae8c56/build/templates.mozbuild#15 But that's just for the explicit stuff. We'd be adding a dependency on libstdc++ from sqlite on the platforms where it's a separate shared library (which I think is just linux nowaways). BTW, this makes me think that since we're now building nspr with the gecko build system, the nspr libraries are linked to libstdc++ too...
I have a candidate patch. It does correctly add this for the NSPR libs, as well as a few other things I hadn't considered: ``` luser@eye7:/build/mozilla-central$ find /build/debug-mozilla-central/ -name backend.mk | xargs grep LIB_IS_C_ONLY /build/debug-mozilla-central/media/ffvpx/libavcodec/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/media/ffvpx/libavutil/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/config/external/nspr/ds/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/config/external/nspr/pr/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/config/external/nspr/libc/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/config/external/sqlite/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/widget/gtk/mozgtk/gtk3/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/widget/gtk/mozgtk/gtk2/backend.mk:LIB_IS_C_ONLY := 1 /build/debug-mozilla-central/widget/gtk/mozgtk/stub/backend.mk:LIB_IS_C_ONLY := 1 ``` However, if you --enable-stdcxx-compat they disappear because the build frontend notices the dependency on stdc++compat from the Library template. I think we should get my patch landed anyway since it's no worse than the current situation, and then we can sort out how to fix the template thing in a followup.
Blocks: 1295937
Summary: Remove config/external/sqlite/Makefile.in → Make the build system infer LIB_IS_C_ONLY
The try push was green.
Comment on attachment 8784480 [details] bug 1256642 - have the build system automatically detect LIB_IS_C_ONLY. https://reviewboard.mozilla.org/r/73926/#review73438 ::: python/mozbuild/mozbuild/frontend/emitter.py:817 (Diff revision 1) > UNIFIED_SOURCES=(UnifiedSources, None, ['.c', '.mm', '.cpp']), > ) > + # Track whether there are any C++ source files. > + # Technically this won't do the right thing for SIMPLE_PROGRAMS in > + # a directory with mixed C and C++ source, but it's not that important. > + cxx_sources = dict((k, False) for k in varmap.keys()) defaultdict(bool) Or you can just use a dict and .get() from it in the loop setting cxx_link.
Attachment #8784480 - Flags: review?(mh+mozilla) → review+
Blocks: 1305960
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Depends on: 1423802
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: