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)
Firefox Build System
General
Tracking
(firefox52 fixed)
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: ted, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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
Comment 1•9 years ago
|
||
(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...
Reporter | ||
Comment 2•8 years ago
|
||
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.
Comment hidden (mozreview-request) |
Reporter | ||
Updated•8 years ago
|
Blocks: 1295937
Summary: Remove config/external/sqlite/Makefile.in → Make the build system infer LIB_IS_C_ONLY
Reporter | ||
Comment 4•8 years ago
|
||
The try push was green.
Comment 5•8 years ago
|
||
mozreview-review |
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+
Reporter | ||
Comment 6•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/83f3a00b2a1905bfbad4dd74358423969ed181d0
bug 1256642 - have the build system automatically detect LIB_IS_C_ONLY. r=glandium
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•