Closed
Bug 488175
Opened 15 years ago
Closed 15 years ago
Need to include |namespace mozilla {Foo}| headers as |#include "mozilla/Foo"|
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: cjones, Assigned: benjamin)
References
Details
Attachments
(4 files)
59.36 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
1.44 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
3.16 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
13.81 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
AFAIK, the only headers needing this atm are those in bug 58904.
Assignee | ||
Comment 1•15 years ago
|
||
This patch, in addition to installing headers in subdirectories, verifies that certain variables are not modified after inclusion of rules.mk. The LOCAL_INCLUDES changes under media/ necessary because we shouldn't be installing the config.h files.
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #375655 -
Flags: review?(ted.mielczarek)
Comment 2•15 years ago
|
||
Comment on attachment 375655 [details] [diff] [review] EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1 [Checkin: See comment 12] -ifndef MOZILLA_INTERNAL_API -INCLUDES += -I$(LIBXUL_DIST)/sdk/include -endif Is this no longer important? I don't see you replacing it with anything. Actually it looks like you're getting rid of $(DIST)/sdk/include completely, am I correct in thinking that? Won't that break the SDK packaging? +# generate .h files from into $(XPIDL_GEN_DIR), then export to $(DIST)/include; Can you fix this comment while you're here? (generate .h files from what?) +libs export libs:: + $(CHECK_FROZEN_VARIABLES) libs is in there twice. Is that intentional? +LOCAL_INCLUDES += -I$(srcdir)/../../include/oggplay This might be clearer to read if you started from $(topsrcdir), but I'm not overly concerned. xpcom/build/XPCOM.h I'm definitely interested to see how this impacts build times. (Should be easy to use precompiled headers with this, though!)
Comment 3•15 years ago
|
||
Comment on attachment 375655 [details] [diff] [review] EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1 [Checkin: See comment 12] Oops, r=me
Attachment #375655 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 4•15 years ago
|
||
Yeah, I'm removing dist/sdk/include entirely... I'll make sure the SDK packaging is fixed as well. I think I meant export libs tools:: or something like that.
Comment 5•15 years ago
|
||
Has the analysis of build time performance with "global" include files taken place? Did I miss the outcome? Ref: http://groups.google.com/group/mozilla.dev.platform/msg/80500a6890bcc29b?pli=1
Comment 6•15 years ago
|
||
I'm not convinced the stuff for nested namespaces works. I have no Variant.h nor a storage/ folder inside of mozilla/ in dist/include. Are slashes allowed in make variable names?
Comment 7•15 years ago
|
||
My gmake gets confused when you put a define inside an if(n)def :-(
Attachment #378276 -
Flags: review?(ted.mielczarek)
Comment 8•15 years ago
|
||
Comment on attachment 378276 [details] [diff] [review] gmake 3.80 bustage fix? [Checkin: Comment 17] So does sicking's, apparently.
Assignee | ||
Updated•15 years ago
|
Attachment #378276 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 9•15 years ago
|
||
Comment on attachment 378276 [details] [diff] [review] gmake 3.80 bustage fix? [Checkin: Comment 17] Let's just make the define unconditional and wrap the foreach. I'll have a patch up in a few.
Updated•15 years ago
|
Attachment #378276 -
Flags: review- → review?(benjamin)
Comment 10•15 years ago
|
||
Comment on attachment 378276 [details] [diff] [review] gmake 3.80 bustage fix? [Checkin: Comment 17] My bad, apparently it's the foreach inside the ifdef that it doesn't like.
Assignee | ||
Updated•15 years ago
|
Attachment #378276 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 378276 [details] [diff] [review] gmake 3.80 bustage fix? [Checkin: Comment 17] Ugh, ok.
Assignee | ||
Comment 12•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/a6b13b312e78 and followup for trace-malloc http://hg.mozilla.org/mozilla-central/rev/ad628ca8eddb
Comment 13•15 years ago
|
||
Comment on attachment 378276 [details] [diff] [review] gmake 3.80 bustage fix? [Checkin: Comment 17] Pushed changeset eeb44ecd8038 to mozilla-central.
Comment 14•15 years ago
|
||
Some pkgconfig files in xulrunner/installer still have stable/unstable issue.
Attachment #379160 -
Flags: review?(benjamin)
Assignee | ||
Updated•15 years ago
|
Attachment #379160 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 15•15 years ago
|
||
Pushed the pkgconfig changes as http://hg.mozilla.org/mozilla-central/rev/8b297d7065d8
Updated•15 years ago
|
Updated•14 years ago
|
Attachment #379160 -
Attachment description: More fix for pkgconfig files in xulrunner/installer → More fix for pkgconfig files in xulrunner/installer
[Checkin: Comment 15]
Updated•14 years ago
|
Attachment #375655 -
Attachment description: EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1 → EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1
[Checkin: See comment 12]
Comment 17•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/eeb44ecd8038 gmake 3.80 bustage fix
Updated•14 years ago
|
Attachment #378276 -
Attachment description: gmake 3.80 bustage fix? → gmake 3.80 bustage fix?
[Checkin: Comment 17]
Comment 18•14 years ago
|
||
Was http://mxr.mozilla.org/mozilla-central/search?string=SDK_PUBLIC&case=1&find=%2Fjs%2Fsrc%2FMakefile.in { /js/src/Makefile.in * line 330 -- $(PUBLIC) $(SDK_PUBLIC): config/nsinstall$(HOST_BIN_SUFFIX) } missed?
Comment 19•14 years ago
|
||
http://build.mozillamessaging.com/buildbot/try/changes/375 Linux: hg issue, submitted again :-/ MacOSX: still building :-| Windows: passed :-)
Attachment #427700 -
Flags: review?(bugspam.Callek)
Comment 20•14 years ago
|
||
Comment on attachment 427700 [details] [diff] [review] (Ev1-CC) Copy it to comm-central, with bug 494172 too [Checkin: See comment 24] >-SDK_PUBLIC = $(DIST)/sdk/include >-SDK_IDL_DIR = $(DIST)/sdk/idl > SDK_LIB_DIR = $(DIST)/sdk/lib > SDK_BIN_DIR = $(DIST)/sdk/bin Good catch not removing these other two (see-also: http://hg.mozilla.org/mozilla-central/rev/e391f455c2b8 ) >+ifneq (,$(OBJS)$(XPIDLSRCS)$(SIMPLE_PROGRAMS)) > MDDEPFILES = $(addprefix $(MDDEPDIR)/,$(OBJS:.$(OBJ_SUFFIX)=.pp)) > ifndef NO_GEN_XPT > MDDEPFILES += $(addprefix $(MDDEPDIR)/,$(XPIDLSRCS:.idl=.xpt)) \ > $(addprefix $(MDDEPDIR)/,$(SDK_XPIDLSRCS:.idl=.xpt)) Also need to remove the SDK_XPIDLSRCS line, as in this hunk: http://hg.mozilla.org/mozilla-central/rev/a6b13b312e78#l3.37 You also missed this hunk: http://hg.mozilla.org/mozilla-central/rev/a6b13b312e78#l3.51 r+ with those nits; pending a successful try run.
Attachment #427700 -
Flags: review?(bugspam.Callek) → review+
Comment 21•14 years ago
|
||
Is there a reason to keep $libdir/$app-devel-$version/sdk/{bin,lib} around ?
Comment 22•14 years ago
|
||
(In reply to comment #20) > (From update of attachment 427700 [details] [diff] [review]) > r+ with those nits; pending a successful try run. Serge, status on this? [if its bitrotted beyond _very_ simple changes; spin to a new bug and re-request review please] (In reply to comment #21) > Is there a reason to keep $libdir/$app-devel-$version/sdk/{bin,lib} around ? If this Q was to me or serge, I'll be happy to re-look at this and answer
Comment 23•14 years ago
|
||
(In reply to comment #22) > Serge, status on this? [if its bitrotted beyond _very_ simple changes Afaict, my updated patch is fine, but bug 547175 is still blocking.
Comment 24•14 years ago
|
||
Comment on attachment 427700 [details] [diff] [review] (Ev1-CC) Copy it to comm-central, with bug 494172 too [Checkin: See comment 24] http://hg.mozilla.org/comm-central/rev/cf1cac550d37 Ev1-CC, with comment 20 suggestion(s).
Attachment #427700 -
Attachment description: (Ev1-CC) Copy it to comm-central, with bug 494172 too → (Ev1-CC) Copy it to comm-central, with bug 494172 too
[Checkin: See comment 24]
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•