Closed
Bug 488175
Opened 16 years ago
Closed 16 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)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
AFAIK, the only headers needing this atm are those in bug 58904.
Assignee | ||
Comment 1•16 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•16 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•16 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•16 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•16 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•16 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•16 years ago
|
||
My gmake gets confused when you put a define inside an if(n)def :-(
Attachment #378276 -
Flags: review?(ted.mielczarek)
Comment 8•16 years ago
|
||
Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]
So does sicking's, apparently.
Assignee | ||
Updated•16 years ago
|
Attachment #378276 -
Flags: review?(ted.mielczarek) → review-
Assignee | ||
Comment 9•16 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•16 years ago
|
Attachment #378276 -
Flags: review- → review?(benjamin)
Comment 10•16 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•16 years ago
|
Attachment #378276 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 11•16 years ago
|
||
Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]
Ugh, ok.
Assignee | ||
Comment 12•16 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•16 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•16 years ago
|
||
Some pkgconfig files in xulrunner/installer still have stable/unstable issue.
Attachment #379160 -
Flags: review?(benjamin)
Assignee | ||
Updated•16 years ago
|
Attachment #379160 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 15•16 years ago
|
||
Pushed the pkgconfig changes as http://hg.mozilla.org/mozilla-central/rev/8b297d7065d8
Updated•15 years ago
|
Updated•15 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•15 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•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/eeb44ecd8038
gmake 3.80 bustage fix
Updated•15 years ago
|
Attachment #378276 -
Attachment description: gmake 3.80 bustage fix? → gmake 3.80 bustage fix?
[Checkin: Comment 17]
Comment 18•15 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•15 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•15 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•15 years ago
|
||
Is there a reason to keep $libdir/$app-devel-$version/sdk/{bin,lib} around ?
Comment 22•15 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•15 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•15 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•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
•