489579 - Use -FI (force include) for MSVC instead of passing defines via the command line

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Rain [:rain1] (inactive)]

MSVC has the -FI option to do the same thing as gcc's -include, but we don't use it in configure right now. This patch makes MSVC use -FI instead.

I've checked with the WinCE people, and they seem to indicated that their compiler supports it too.

Comment 1

[Rain [:rain1] (inactive)]

Attachment: patch to use -FI

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 374069 [details] [diff] [review]
patch to use -FI

This looks good, but I think these variables would be better set in this block here:
http://mxr.mozilla.org/mozilla-central/source/configure.in#2162

What do you think?

Comment 3

[Rain [:rain1] (inactive)]

(In reply to comment #2)
> What do you think?

Feels a bit out of place to me -- makes somewhat more sense to me to have this set for GCC and MSVC together than to have it fallback first, then later override it for MSVC.

If you prefer it that way, however, I'll make another patch.

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 374069 [details] [diff] [review]
patch to use -FI

I think I'd prefer to have it in the MSVC specific section, since that will result in fewer conditional blocks.

Comment 5

[Rain [:rain1] (inactive)]

Attachment: add -FI to target specific block instead [Checkin: Comment 6]

The md flag part still required a conditional block. :(

Comment 6

[Rain [:rain1] (inactive)]

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/ffbbf6fa6da0

Comment 7

[[e-mail]]

OK, so maybe I have my objdir in a weird location, but my build broke because VC (both 7.1 and 8) found a comm-central mozilla-config.h in a relative path from one of the local includes while trying to compile SVG :-(

There are at least two possibilities that would work around this problem.
1. Move -I. before $(LOCAL_INCLUDES) in config/config.mk
2. Remove the $(DEPTH) after the -FI since $(DIST)/include is in $(INCLUDES)
   [config/Makefile.in installs it there]

Comment 8

[Rain [:rain1] (inactive)]

Attachment: Workaround 1 (move -I. above LOCAL_INCLUDES)

Comment 9

[Rain [:rain1] (inactive)]

Attachment: Workaround 1, "no evil tabs" edition

Comment 10

[Rain [:rain1] (inactive)]

Attachment: Workaround 2 (remove $(DEPTH)) [Checkin: Comment 12]

Ted, which one do you think is better?

I'm surprised no one's complained about this happening with gcc's -include.

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 383168 [details] [diff] [review]
Workaround 2 (remove $(DEPTH))
[Checkin: Comment 12]

Let's go with this one.

Comment 12

[Rain [:rain1] (inactive)]

(In reply to comment #11)
> (From update of attachment 383168 [details] [diff] [review])
> Let's go with this one.

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/0cb931e6aaa7

Comment 13

[Wei Sun]

This bugfix removes $(ACDEFINES) from _DEFINES_CXXFLAGS and results in missing MOZ_JSLOADER when compiling xpconnect.
Or there is another option to enable MOZ_JSLOADER?

Comment 14

[Rain [:rain1] (inactive)]

(In reply to comment #13)
> This bugfix removes $(ACDEFINES) from _DEFINES_CXXFLAGS and results in missing
> MOZ_JSLOADER when compiling xpconnect.
> Or there is another option to enable MOZ_JSLOADER?

MOZ_JSLOADER is defined in mozilla-config.h, and |Components.classes["@mozilla.org/moz/jsloader;1"]| works for me in today's nightly. Which version of VC++ are you using?

Comment 15

[Wei Sun]

I specified a objdir and it works now.

xpcmodule.cpp
Building deps for /e/dev/mozilla-central/js/src/xpconnect/src/xpcmodule.cpp
cl -Foxpcmodule.obj -c  -DMOZILLA_INTERNAL_API -DOSTYPE=\"WINNT6.0\" -DOSARCH=WI
NNT -DJSFILE -DJS_THREADSAFE -DEXPORT_XPC_API  -DJS_TRACER=1 -DFEATURE_NANOJIT=1
 -DAVMPLUS_IA32 -DAVMPLUS_WIN32  -I/e/dev/mozilla-central/js/src/xpconnect/src/.
./loader -I/e/dev/mozilla-central/js/src -I/e/dev/mozilla-central/js/src/nanojit
  -I/e/dev/mozilla-central/js/src/xpconnect/src -I. -I../../../../dist/include -
I../../../../dist/include/nsprpub  -Ie:/dev/mozilla-central/ff-dbg/dist/include/
nspr -Ie:/dev/mozilla-central/ff-dbg/dist/include/nss            -GR- -TP -nolog
o -Zc:wchar_t- -W3 -Gy -Fdgenerated.pdb  -DDEBUG -D_DEBUG -DDEBUG_sunwei -DTRACI
NG -Zi -MDd           -FI mozilla-config.h -DMOZILLA_CLIENT /e/dev/mozilla-centr
al/js/src/xpconnect/src/xpcmodule.cpp
xpcmodule.cpp

This is the command to compile xpcmodule.cpp. Seems that /e/dev/mozilla-central/js/src/mozilla-config.h (which does not define MOZ_JSLOADER) will be included instead of the right one when doing a scrdir build.

Comment 16

[Rain [:rain1] (inactive)]

Reopened because of bug 503756.

Comment 17

[Rain [:rain1] (inactive)]

Resolved fixed after bug 503756 was fixed.