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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

AFAIK, the only headers needing this atm are those in bug 58904.

Comment 1

[Benjamin Smedberg]

Attachment: EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1 [Checkin: See comment 12]

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.

Comment 2

[(not currently active) 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 3

[(not currently active) Ted Mielczarek]

Comment on attachment 375655 [details] [diff] [review]
EXPORTS_NAMESPACES, variable verification, and other goodies, rev. 1
[Checkin: See comment 12]

Oops, r=me

Comment 4

[Benjamin Smedberg]

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

[Mark Banner (:standard8)]

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

[Shawn Wilsher :sdwilsh]

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

[[e-mail]]

Attachment: gmake 3.80 bustage fix? [Checkin: Comment 17]

My gmake gets confused when you put a define inside an if(n)def :-(

Comment 8

[[e-mail]]

Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

So does sicking's, apparently.

Comment 9

[Benjamin Smedberg]

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.

Comment 10

[[e-mail]]

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.

Comment 11

[Benjamin Smedberg]

Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

Ugh, ok.

Comment 12

[Benjamin Smedberg]

http://hg.mozilla.org/mozilla-central/rev/a6b13b312e78
and followup for trace-malloc http://hg.mozilla.org/mozilla-central/rev/ad628ca8eddb

Comment 13

[[e-mail]]

Comment on attachment 378276 [details] [diff] [review]
gmake 3.80 bustage fix?
[Checkin: Comment 17]

Pushed changeset eeb44ecd8038 to mozilla-central.

Comment 14

[Takanori MATSUURA]

Attachment: More fix for pkgconfig files in xulrunner/installer [Checkin: Comment 15]

Some pkgconfig files in xulrunner/installer still have stable/unstable issue.

Comment 15

[Benjamin Smedberg]

Pushed the pkgconfig changes as http://hg.mozilla.org/mozilla-central/rev/8b297d7065d8

Comment 17

[Serge Gautherie (:sgautherie)]

http://hg.mozilla.org/mozilla-central/rev/eeb44ecd8038
gmake 3.80 bustage fix

Comment 18

[Serge Gautherie (:sgautherie)]

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

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1-CC) Copy it to comm-central, with bug 494172 too [Checkin: See comment 24]

http://build.mozillamessaging.com/buildbot/try/changes/375
Linux: hg issue, submitted again :-/
MacOSX: still building :-|
Windows: passed :-)

Comment 20

[Justin Wood (: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.

Comment 21

[Mike Hommey [:glandium]]

Is there a reason to keep $libdir/$app-devel-$version/sdk/{bin,lib} around ?

Comment 22

[Justin Wood (:Callek)]

(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

[Serge Gautherie (:sgautherie)]

(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

[Serge Gautherie (:sgautherie)]

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).