415274 - patch to allow building of JS as static library

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Mike Moening]

Attachment: Static Lib patch V1

A small patch to allow the option of building of JS as a static library.

Requires non-NSPR library such as JSMiniNSPR or other.
Link:  http://www.reteksolutions.com/sections/oss/jsmininspr.asp

CC'ing Brendan & Igor and requesting review...

Comment 1

[Igor Bukanov]

Comment on attachment 300919 [details] [diff] [review]
Static Lib patch V1

>Index: jstypes.h
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jstypes.h,v
>retrieving revision 3.42
>diff -u -w -b -U 8 -p -r3.42 jstypes.h
>--- jstypes.h	18 Jun 2007 19:33:21 -0000	3.42
>+++ jstypes.h	1 Feb 2008 20:31:27 -0000
>@@ -154,25 +154,30 @@
> #else
> #    define JS_IMPORT_DATA(__x)     JS_EXPORT_DATA (__x)
> #endif
> 
> /*
>  * The linkage of JS API functions differs depending on whether the file is
>  * used within the JS library or not.  Any source file within the JS
>  * interpreter should define EXPORT_JS_API whereas any client of the library
>- * should not.
>+ * should not. STATIC_JS_API is used to build JS as a static library.
>  */

Call the macro JS_STATIC_API not to introduce more public macros lacking jS_ prefix.

>+#ifdef STATIC_JS_API
>+#  define JS_PUBLIC_API(t)    t
>+#  define JS_PUBLIC_DATA(t)   t
>+#else
> #ifdef EXPORT_JS_API
> #define JS_PUBLIC_API(t)    JS_EXPORT_API(t)
> #define JS_PUBLIC_DATA(t)   JS_EXPORT_DATA(t)
> #else
> #define JS_PUBLIC_API(t)    JS_IMPORT_API(t)
> #define JS_PUBLIC_DATA(t)   JS_IMPORT_DATA(t)
> #endif
>+#endif

This calls for if defined .. elif defined endif structure. Plus SpiderMonkey's style calls for indenting macro definition at the tab stop (t should be at the same column where JS_EXPORT_API(t) starts). In addition to indent preprocessor declarations use indent level of blanks between # and define, not 2 * (indent level).

Comment 2

[Mike Moening]

Attachment: Static Lib patch V2

Changes per Igor plus added jsapi.c dllmain fixes into the patch which I forgot about last time.

Comment 3

[Igor Bukanov]

Comment on attachment 301158 [details] [diff] [review]
Static Lib patch V2

>Index: jstypes.h
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jstypes.h,v
>retrieving revision 3.42
>diff -u -w -b -U 8 -p -r3.42 jstypes.h
>--- jstypes.h	18 Jun 2007 19:33:21 -0000	3.42
>+++ jstypes.h	1 Feb 2008 20:31:27 -0000
>@@ -154,25 +154,30 @@
> #else
> #    define JS_IMPORT_DATA(__x)     JS_EXPORT_DATA (__x)
> #endif
> 
> /*
>  * The linkage of JS API functions differs depending on whether the file is
>  * used within the JS library or not.  Any source file within the JS
>  * interpreter should define EXPORT_JS_API whereas any client of the library
>- * should not.
>+ * should not. JS_STATIC_API is used to build JS as a static library.
>  */
>-#ifdef EXPORT_JS_API
>+#ifdef JS_STATIC_API

This should be #if defined(JS_STATIC_API) for symmetry with the following #elif

>+#  define JS_PUBLIC_API(t)    t
>+#  define JS_PUBLIC_DATA(t)   t
>+#elif EXPORT_JS_API

This should be #elif defined(EXPORT_JS_API)

> #  define JS_PUBLIC_API(t)    JS_EXPORT_API(t)
> #  define JS_PUBLIC_DATA(t)   JS_EXPORT_DATA(t)
> #else
> #  define JS_PUBLIC_API(t)    JS_IMPORT_API(t)
> #  define JS_PUBLIC_DATA(t)   JS_IMPORT_DATA(t)
> #endif
> 
> #define JS_FRIEND_API(t)    JS_PUBLIC_API(t)
>
>Index: jsapi.c
>===================================================================
>RCS file: /cvsroot/mozilla/js/src/jsapi.c,v
>retrieving revision 3.400
>diff -u -w -b -U 8 -p -r3.400 jsapi.c
>--- jsapi.c	1 Feb 2008 19:59:59 -0000	3.400
>+++ jsapi.c	1 Feb 2008 22:30:37 -0000
>@@ -5694,16 +5694,17 @@ JS_SetGCZeal(JSContext *cx, uint8 zeal)
> /*
>  * Global Instance handle...
>  * In Win32 this is the module handle of the DLL.
>  *
>  * In Win16 this is the instance handle of the application
>  * which loaded the DLL.
>  */
> 
>+#ifndef STATIC_JS_API

This should be #ifndef JS_STATIC_API

> #ifdef _WIN32
> BOOL WINAPI DllMain (HINSTANCE hDLL, DWORD dwReason, LPVOID lpReserved)
> {
>     return TRUE;
> }
> 
> #else  /* !_WIN32 */

Preexisting nit: !_WIN32 case was for Win16. SpiderMonkey has not supported this for quite some time. So just assume that _WIN32 is defined and remove comments about Win16.
 

> 
>@@ -5712,11 +5713,11 @@ int CALLBACK LibMain( HINSTANCE hInst, W
> {
>     return TRUE;
> }
> 
> BOOL CALLBACK __loadds WEP(BOOL fSystemExit)
> {
>     return TRUE;
> }
>-
> #endif /* !_WIN32 */
>+#endif /* !STATIC_JS_API */
> #endif /* XP_WIN */

Comment 4

[Mike Moening]

Attachment: Static Lib Patch V3

Fixed nits and removed WIN16 comments

Comment 5

[Igor Bukanov]

Comment on attachment 301280 [details] [diff] [review]
Static Lib Patch V3

>Index: jstypes.h
>+#ifndef JS_STATIC_API
> #ifdef _WIN32

I was not clear the last time: could you remove this "#ifdef _WIN32" line and the following "#else" - "#endif /* !_WIN32 */" lines for Win16 support since this patch touches the lines in any case?

> #else  /* !_WIN32 */
> 
>@@ -5712,11 +5710,11 @@ int CALLBACK LibMain( HINSTANCE hInst, W
> {
>     return TRUE;
> }
> 
> BOOL CALLBACK __loadds WEP(BOOL fSystemExit)
> {
>     return TRUE;
> }
>-
> #endif /* !_WIN32 */
>+#endif /* !JS_STATIC_API */
> #endif /* XP_WIN */
>

Comment 6

[Mike Moening]

Attachment: Static Lib Patch V4

Totally removes WIN16 support.

Comment 7

[Igor Bukanov]

Comment on attachment 301377 [details] [diff] [review]
Static Lib Patch V4

Nominating for 1.9 as the patch allows to build SpiderMonkey as a static library on Windows.

Comment 8

[Mike Moening]

Comment on attachment 301377 [details] [diff] [review]
Static Lib Patch V4

Brendan,
If you approve can you check in? Don't know if I have rights do that yet.

Comment 9

[Reed Loden [:reed]]

Comment on attachment 301377 [details] [diff] [review]
Static Lib Patch V4

JavaScript doesn't require superreview. Once this patch has approval and the tree is open, the patch can be landed.

Comment 10

[Mike Moening]

Why is it closed?
And who will land it?

Comment 11

[Bob Clary [:bc] (inactive)]

The tree is closed for Firefox beta 3 and should open relatively soon. If reedbot ;-) or Igor don't land it, I can do it.

Comment 12

[Igor Bukanov]

Attachment: v4 + using same style for in jstypes.h

This version has extra changes to use the same style in jstypes,h for the preprocessor defines.

Comment 13

[Mike Moening]

I can't look at the patch. Seems to be garbled??

Comment 14

[Igor Bukanov]

Attachment: v4 + using same style for in jstypes.h for real

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 302168 [details] [diff] [review]
v4 + using same style for in jstypes.h for real

Looks good (diff -w would be helpful, I have faith).

/be

Comment 16

[Igor Bukanov]

Attachment: diff -wB version of the previous patch

Comment 17

[Brendan Eich [:brendan]]

Comment on attachment 302241 [details] [diff] [review]
diff -wB version of the previous patch

>+#if defined(JS_STATIC_API)
>+
>+# define JS_PUBLIC_API(t)   t
>+# define JS_PUBLIC_DATA(t)  t
>+
>+#elif defined(EXPORT_JS_API)

This brings to light the naming convention break -- I personally don't think JS_ has to go at the front to avoid collisions, and would prefer STATIC_JS_API for symmetry, as MikeM had it.

/be

Comment 18

[Igor Bukanov]

Attachment: back to STATIC_JS_API

The new version renames JS_STATIC_API back to STATIC_JS_API.

Comment 19

[Igor Bukanov]

I checked in the patch from comment 18 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1202566500&maxdate=1202567006&who=igor%25mir2.org