Closed
Bug 1463637
Opened 6 years ago
Closed 6 years ago
Update Hunspell build config to remove mozilla-config.h hacks
Categories
(Core :: Spelling checker, enhancement, P3)
Core
Spelling checker
Tracking
()
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: kmag, Assigned: kmag)
Details
Attachments
(1 file)
Our Hunspell build config currently uses mozilla-config.h.in to add some extra includes when building Hunspell sources. Adding hacks like that to the top-level build config really isn't ideal. We should add the extra includes via CFLAGS.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8979819 [details] Bug 1463637: Remove mozilla-config.h hacks for Hunspell extra includes. https://reviewboard.mozilla.org/r/245984/#review252024 Please remove the mentions to mozilla-config.h in those headers. Please also r?build for a second round, in case they have something to say about the -include/-FI stuff. Not sure if a one-off is fine or if we should have a more general FORCE_INCLUDE.
Attachment #8979819 -
Flags: review?(mh+mozilla)
Comment hidden (mozreview-request) |
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8979819 [details] Bug 1463637: Remove mozilla-config.h hacks for Hunspell extra includes. https://reviewboard.mozilla.org/r/245984/#review252026 Let's not make that confusing for whoever takes this review in the build team by having a r+ from me, where the remaining r?build could be seen as leftover.
Attachment #8979819 -
Flags: review?(mh+mozilla)
Updated•6 years ago
|
Priority: -- → P3
Updated•6 years ago
|
Attachment #8979819 -
Flags: review?(core-build-config-reviews) → review?(ted)
Comment 5•6 years ago
|
||
mozreview-review |
Comment on attachment 8979819 [details] Bug 1463637: Remove mozilla-config.h hacks for Hunspell extra includes. https://reviewboard.mozilla.org/r/245984/#review252670 This seems fine. ::: extensions/spellcheck/hunspell/glue/common.mozbuild:8 (Diff revision 2) > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +@template > +def ForceInclude(*headers): It might be worth moving this to build/templates.mozbuild, we do use `-FI` / `-include` in a few other places: https://dxr.mozilla.org/mozilla-central/rev/47e81ea1ef10189ef210867934bf36e14cf223dc/media/gmp-clearkey/0.1/moz.build#60 https://dxr.mozilla.org/mozilla-central/rev/47e81ea1ef10189ef210867934bf36e14cf223dc/media/libav/moz.build#67 https://dxr.mozilla.org/mozilla-central/rev/47e81ea1ef10189ef210867934bf36e14cf223dc/media/libsoundtouch/src/moz.build#40 https://dxr.mozilla.org/mozilla-central/rev/47e81ea1ef10189ef210867934bf36e14cf223dc/security/moz.build#124
Attachment #8979819 -
Flags: review?(ted) → review+
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5) > > +@template > > +def ForceInclude(*headers): > > It might be worth moving this to build/templates.mozbuild, we do use `-FI` / > `-include` in a few other places: > https://dxr.mozilla.org/mozilla-central/rev/ > 47e81ea1ef10189ef210867934bf36e14cf223dc/media/gmp-clearkey/0.1/moz.build#60 > https://dxr.mozilla.org/mozilla-central/rev/ > 47e81ea1ef10189ef210867934bf36e14cf223dc/media/libav/moz.build#67 > https://dxr.mozilla.org/mozilla-central/rev/ > 47e81ea1ef10189ef210867934bf36e14cf223dc/media/libsoundtouch/src/moz.build#40 > https://dxr.mozilla.org/mozilla-central/rev/ > 47e81ea1ef10189ef210867934bf36e14cf223dc/security/moz.build#124 All of these happen in places where we're adding compiler-specific CFLAGs for other reasons anyway, so I'm not sure it would help much. But it probably wouldn't hurt either.
Assignee | ||
Comment 7•6 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/79a77332a9e515bfae5e1807b5c57be7cec7c83a Bug 1463637: Remove mozilla-config.h hacks for Hunspell extra includes. r=ted
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/79a77332a9e5
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in
before you can comment on or make changes to this bug.
Description
•