Closed
Bug 1033122
Opened 8 years ago
Closed 8 years ago
be consistent about speex_resampler symbol visibility
Categories
(Firefox Build System :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla34
People
(Reporter: karlt, Assigned: karlt)
Details
Attachments
(1 file)
8.86 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
Currently the presence of speex/speex_resampler.h in dist/system_wrappers means that some object files are looking for a hidden symbol and others a default symbol. This is because the header is included with "" and so another exported header (in dist/include) finds the speex/speex_resampler.h in dist/include before dist/system_wrappers. Visibility depends on the order of includes. (Fortunately a hidden definition seems to resolve a default visibility undefined symbol.) The convention is to use "" for includes of exported headers within mozilla. <> would probably work but convention implies this is then a system header. We don't want the system header, which may be different from the internal header. If GKMEDIAS_SHARED_LIBRARY is ever meant to work for non-WINNT platforms, then I guess the approach should be to change to <speex/speex_resampler.h> everywhere and wrap only when GKMEDIAS_SHARED_LIBRARY is set. Its only necessary to use <> in exported headers, but code gets copied, so consistency is preferred. I don't know of any intention to support GKMEDIAS_SHARED_LIBRARY elsewhere. A similar approach is required if system includes are ever to be wrapped on WINNT (c.f. bug 1008192). If the GKMEDIAS_SHARED_LIBRARY is only ever WINNT and WRAP_SYSTEM_INCLUDES will never be WINNT, then we could follow the "speex/speex_resampler.h" convention everywhere.
Assignee | ||
Comment 1•8 years ago
|
||
Attachment #8449119 -
Flags: review?(khuey)
Assignee | ||
Comment 2•8 years ago
|
||
Comment on attachment 8449119 [details] [diff] [review] patch The trunk/webrtc include is already a modification from trunk. https://hg.mozilla.org/mozilla-central/rev/ad8a86bfd860#l9.12
Attachment #8449119 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/98d22b0455f6
Flags: in-testsuite-
Comment 4•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/98d22b0455f6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
QA Whiteboard: [qa-]
Updated•5 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•