Need to include |namespace mozilla {Foo}| headers as |#include "mozilla/Foo"|

RESOLVED FIXED in mozilla1.9.2a1

Status

defect
RESOLVED FIXED
11 years ago
2 years ago

People

(Reporter: cjones, Assigned: benjamin)

Tracking

(Blocks 1 bug)

Trunk
mozilla1.9.2a1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

AFAIK, the only headers needing this atm are those in bug 58904.
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 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 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+
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.
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
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?
My gmake gets confused when you put a define inside an if(n)def :-(
Attachment #378276 - Flags: review?(ted.mielczarek)
Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

So does sicking's, apparently.
Attachment #378276 - Flags: review?(ted.mielczarek) → review-
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.
Attachment #378276 - Flags: review- → review?(benjamin)
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.
Attachment #378276 - Flags: review?(benjamin) → review+
Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

Ugh, ok.
http://hg.mozilla.org/mozilla-central/rev/a6b13b312e78
and followup for trace-malloc http://hg.mozilla.org/mozilla-central/rev/ad628ca8eddb
No longer blocks: 493672
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Depends on: 493672, 493648
Resolution: --- → FIXED
Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

Pushed changeset eeb44ecd8038 to mozilla-central.
Depends on: 493834
Some pkgconfig files in xulrunner/installer still have stable/unstable issue.
Attachment #379160 - Flags: review?(benjamin)
Attachment #379160 - Flags: review?(benjamin) → review+
Pushed the pkgconfig changes as http://hg.mozilla.org/mozilla-central/rev/8b297d7065d8
Duplicate of this bug: 398573
Blocks: 494490
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Depends on: 544333
Attachment #379160 - Attachment description: More fix for pkgconfig files in xulrunner/installer → More fix for pkgconfig files in xulrunner/installer [Checkin: Comment 15]
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]
Depends on: 494172
Attachment #378276 - Attachment description: gmake 3.80 bustage fix? → gmake 3.80 bustage fix? [Checkin: Comment 17]
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?
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 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+
Depends on: 547175
Depends on: 547257
Is there a reason to keep $libdir/$app-devel-$version/sdk/{bin,lib} around ?
(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
(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 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]
Depends on: 556813
No longer depends on: 556813
Duplicate of this bug: 540524
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.