661663 - Have to #define XP_WIN when building app with embedded SpiderMonkey JS engine

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Mark Giffin [:markg]]

User-Agent:       Mozilla/5.0 (Windows NT 5.1) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.71 Safari/534.24
Build Identifier: SpiderMonkey js185-1.0.0

Should not have to define XP_foo for your platform since about SM 1.8.1. But if I do not add #define XP_WIN to my app that has SM embedded, the build crashes with this error: 
c:\js-1.8.5\js\src\jscpucfg.h(83) : fatal error C1189: #error :  "Must define one of XP_BEOS, XP_OS2, XP_WIN, or XP_UNIX"

Reproducible: Always

Steps to Reproduce:
1. Include jsapi.h in an app to get SpiderMonkey.
2. Do not use #define XP_WIN
3. Build the app in MozillaBuild shell (I used cl.exe from VS 9)
4. Build crashes with the error that is given in Details field ("fatal error C1189" etc)

Actual Results:  
c:\js-1.8.5\js\src\jscpucfg.h(83) : fatal error C1189: #error :  "Must define one of XP_BEOS, XP_OS2, XP_WIN, or XP_UNIX"

Expected Results:  
No build errors

I'm using Windows XP Pro, MozillaBuild bash shell downloaded 4/24/11. 
Full code for my app: http://pastebin.mozilla.org/1240367

Comment 1

[Wesley W. Garland]

Note that this bug affects only Windows, and means that we can't build Win32 embeddings without silly #ifdef hackery!  (Unless you're using a non-MS compiler, anyhow).

Here's why, from jstypes.h:

#ifdef _MSC_VER
# include "jscpucfg.h" /* We can't auto-detect MSVC configuration */
else
# include "jsautocfg.h" /* Use auto-detected configuration */
#endif 

The problem is that jscpucfg.h wants the XP_xxx define, rather than using the autoconf-ery. I don't really understand why.

This was introduced with Bug 475027.

Comment 2

[Benjamin Smedberg]

XP_WIN is defined by configure and ends up in mozilla-config.h. I believe the problem is that this affects the public API, and is therefore supposed to be in js-config.h.

I believe that if you add it to http://mxr.mozilla.org/mozilla-central/source/js/src/js-config.h.in you will be fine.

Comment 3

[Jacek Caban]

Attachment: Don't use XP_WIN in jscpucfg.h

I don't see any reason to use XP_WIN in jscpucfg.h. Checking for _WIN32 and _WIN64 should be enough. The attached (untested!) patch should fix the problem.

BTW, I think that better solution would be fixing generating jsautocfg.h by configure script so that we could get rid of the whole jscpucfg.h.

Comment 4

[Wesley W. Garland]

Jacek, would you be willing to test and r? this patch?

Comment 5

[Jacek Caban]

Attachment: fix v1.1

Sure, but currently my Windows VM is broken, so I can't test standalone build. The fix is, however, very straightforward, so I'm confident about this patch (and may post to try before committing). The attached version cleans up #ifdefs a bit more than previous one. It mostly removes useless #ifdefs that just duplicate the logic.

Comment 6

[Benjamin Smedberg]

Comment on attachment 566857 [details] [diff] [review]
fix v1.1

Needs to pass a tryserver run before landing...

Comment 7

[Jacek Caban]

Thanks for review. Passed on try, pushed to m-i:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1491a374c1a6

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1491a374c1a6