Closed Bug 489579 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox Build System :: General, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: rain1, Assigned: rain1)

References

Details

(Whiteboard: [fixed by bug 503756 after backout+modification+relanding])

Attachments

(2 files, 3 obsolete files)

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.
Attached patch patch to use -FI (obsolete) — Splinter Review
Attachment #374069 - Flags: review?(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?
(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.
Attachment #374069 - Flags: review?(ted.mielczarek) → review-
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.
The md flag part still required a conditional block. :(
Attachment #374069 - Attachment is obsolete: true
Attachment #375954 - Flags: review?(ted.mielczarek)
Attachment #375954 - Flags: review?(ted.mielczarek) → review+
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/ffbbf6fa6da0
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
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]
Attachment #383166 - Flags: review?(ted.mielczarek)
Attachment #383166 - Attachment is obsolete: true
Attachment #383167 - Flags: review?(ted.mielczarek)
Attachment #383166 - Flags: review?(ted.mielczarek)
Ted, which one do you think is better?

I'm surprised no one's complained about this happening with gcc's -include.
Attachment #383168 - Flags: review?(ted.mielczarek)
Attachment #383168 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 383168 [details] [diff] [review]
Workaround 2 (remove $(DEPTH))
[Checkin: Comment 12]

Let's go with this one.
Attachment #383167 - Flags: review?(ted.mielczarek) → review-
(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
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?
(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?
Blocks: 503756
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.
Reopened because of bug 503756.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.2a1 → mozilla1.9.3
Resolved fixed after bug 503756 was fixed.
Target Milestone: mozilla1.9.3 → mozilla1.9.3a1
Attachment #383167 - Attachment is obsolete: true
Attachment #383168 - Attachment description: Workaround 2 (remove $(DEPTH)) → Workaround 2 (remove $(DEPTH)) [Checkin: Comment 12]
Attachment #375954 - Attachment description: add -FI to target specific block instead → add -FI to target specific block instead [Checkin: Comment 6]
Flags: in-testsuite-
Whiteboard: [fixed by bug 503756 after backout+modification+relanding]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.