incubator-couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Davis <paul.joseph.da...@gmail.com>
Subject Re: git commit: fix build with custom path - close #COUCHDB-1426
Date Thu, 01 Mar 2012 18:14:19 GMT
A few hunks in here concern me:

-    CPPFLAGS="$CPPFLAGS -I/opt/local/include/js"
    CPPFLAGS="$CPPFLAGS -I/usr/local/include"
-    CPPFLAGS="$CPPFLAGS -I/usr/local/include/js"
    CPPFLAGS="$CPPFLAGS -I/usr/include"
-    CPPFLAGS="$CPPFLAGS -I/usr/include/js"

I remember having to add some of these for extremely specific
scenarios. Are we absolutely sure this isn't going to lead to a
regression for some people?

+AC_MSG_RESULT(js flags $CPPFLAGS)
+

This syntax looks weird to me. Its probably right but does anyone know
the rules on when strings need square brackets?

+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+                   #include <jsconfig.h>
+                   ]],
+                   [[
+                    #if JS_VERSION == 170
+                    // Use spidermonkey 1.7.0
+                    #else
+                    # Not Spidermonkey 1.7.0
+                    #endif
+                    ]])],
+                    [use_js170=yes],
+                    [])

Please don't invent a new indentation scheme for random hunks in a
patch. Also, what's actually being tested here? I'm not seeing how
this would fail unless we're depending on jsconfig.h only existing in
1.7.0 builds, in which case a test compile is not the way to run this
check.

Everything after that hunk looks quite weird to me. I'm fairly certain
we just had a commit that fixes this issue. I don't remember the
details off the top of my head but this is getting into the land of
silly walks with all the bending over backwards to find SpiderMonkey.
At a certain point we might want to just ixnay a lot of this and just
guess at the most common cases and if we can't find a version then we
should just add a configure option.

On Wed, Feb 29, 2012 at 6:01 PM,  <jhs@apache.org> wrote:
> Updated Branches:
>  refs/heads/COUCHDB-1426 [created] 56a00ede8
>
>
> fix build with custom path - close #COUCHDB-1426
>
> This patch make sure that js libs and include given using the options
> --with-js-lib and --with-js-include are used in priority, ie before the
> detection of the version. Also if spidermonkey 1.7.0 is detected it
> removes useless tests.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/couchdb/repo
> Commit: http://git-wip-us.apache.org/repos/asf/couchdb/commit/56a00ede
> Tree: http://git-wip-us.apache.org/repos/asf/couchdb/tree/56a00ede
> Diff: http://git-wip-us.apache.org/repos/asf/couchdb/diff/56a00ede
>
> Branch: refs/heads/COUCHDB-1426
> Commit: 56a00ede8ab237fc3b2c1a3ea23a688e5aa997c0
> Parents: 766d461
> Author: benoitc <benoitc@apache.org>
> Authored: Thu Mar 1 00:16:14 2012 +0100
> Committer: Jason Smith (air) <jhs@apache.org>
> Committed: Wed Feb 29 23:59:55 2012 +0000
>
> ----------------------------------------------------------------------
>  configure.ac |  128 +++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 80 insertions(+), 48 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/couchdb/blob/56a00ede/configure.ac
> ----------------------------------------------------------------------
> diff --git a/configure.ac b/configure.ac
> index 7ce4842..e526a18 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -137,13 +137,33 @@ AC_ARG_WITH([erlang], [AC_HELP_STRING([--with-erlang=PATH],
>  ])
>  AC_SUBST(ERLANG_FLAGS)
>
> -PKG_CHECK_MODULES([JS], [mozjs185], [
> +AC_ARG_WITH([js-lib], [AC_HELP_STRING([--with-js-lib=PATH],
> +            [set PATH to the SpiderMonkey library directory])],
> +    [
> +    JS_LIB_DIR=$withval
> +    JS_LIBS="-L$JS_LIB_DIR"
> +    AC_MSG_RESULT(yo $JS_LIBS)
> +], [
> +    PKG_CHECK_MODULES([JS], [mozjs185], [
>     JS_LIB_DIR="$(${PKG_CONFIG} --variable=libdir mozjs185)"
> +    ], [
> +        PKG_CHECK_MODULES([JS], [mozilla-js >= 1.7], [
> +            JS_LIB_DIR="$(${PKG_CONFIG} --variable=sdkdir mozilla-js)/lib"
> +        ], [
> +            JS_LIB_DIR="${libdir}"
> +        ])
> +    ])
> +])
> +
> +
> +AC_ARG_WITH([js-include], [AC_HELP_STRING([--with-js-include=PATH],
> +    [set PATH to the SpiderMonkey include directory])], [
> +    JS_INCLUDE="$withval"
> +    JS_CFLAGS="-I$JS_INCLUDE"
>  ], [
>     PKG_CHECK_MODULES([JS], [mozilla-js >= 1.7], [
> -        JS_LIB_DIR="$(${PKG_CONFIG} --variable=sdkdir mozilla-js)/lib"
> +        JS_CFLAGS="$(${PKG_CONFIG} --variable=includedir mozilla-js)/lib"
>     ], [
> -        JS_LIB_DIR="${libdir}"
>         JS_CFLAGS="-I/usr/include"
>         JS_CFLAGS="$JS_CFLAGS -I/usr/include/js"
>         JS_CFLAGS="$JS_CFLAGS -I/usr/include/mozjs"
> @@ -152,19 +172,6 @@ PKG_CHECK_MODULES([JS], [mozjs185], [
>     ])
>  ])
>
> -AC_ARG_WITH([js-include], [AC_HELP_STRING([--with-js-include=PATH],
> -    [set PATH to the SpiderMonkey include directory])], [
> -    JS_INCLUDE="$withval"
> -    JS_CFLAGS="-I$JS_INCLUDE"
> -], [])
> -
> -AC_ARG_WITH([js-lib], [AC_HELP_STRING([--with-js-lib=PATH],
> -    [set PATH to the SpiderMonkey library directory])],
> -    [
> -    JS_LIB_DIR=$withval
> -    JS_LIBS="-L$withval"
> -], [])
> -
>  use_js_trunk=no
>  AC_ARG_ENABLE([js-trunk], [AC_HELP_STRING([--enable-js-trunk],
>     [allow use of SpiderMonkey versions newer than js185-1.0.0])], [
> @@ -177,11 +184,8 @@ AS_CASE([$(uname -s)],
>     [CYGWIN*], [] ,
>     [*], [
>     CPPFLAGS="$CPPFLAGS -I/opt/local/include"
> -    CPPFLAGS="$CPPFLAGS -I/opt/local/include/js"
>     CPPFLAGS="$CPPFLAGS -I/usr/local/include"
> -    CPPFLAGS="$CPPFLAGS -I/usr/local/include/js"
>     CPPFLAGS="$CPPFLAGS -I/usr/include"
> -    CPPFLAGS="$CPPFLAGS -I/usr/include/js"
>     LDFLAGS="$LDFLAGS -L/opt/local/lib"
>     LDFLAGS="$LDFLAGS -L/usr/local/lib"
>  ])
> @@ -211,6 +215,8 @@ LIBS="$JS_LIBS $LIBS"
>  OLD_CPPFLAGS="$CPPFLAGS"
>  CPPFLAGS="$JS_CFLAGS $CPPFLAGS"
>
> +AC_MSG_RESULT(js flags $CPPFLAGS)
> +
>  AC_CHECK_HEADER([jsapi.h], [], [
>     AC_CHECK_HEADER([js/jsapi.h],
>         [
> @@ -222,39 +228,65 @@ AC_CHECK_HEADER([jsapi.h], [], [
>  Are the Mozilla SpiderMonkey headers installed?])
>         ])])
>
> -AC_CHECK_LIB([mozjs185], [JS_NewContext], [JS_LIB_BASE=mozjs185], [
> -    AC_CHECK_LIB([mozjs185-1.0], [JS_NewContext], [JS_LIB_BASE=mozjs185-1.0], [
> -        AC_CHECK_LIB([mozjs], [JS_NewContext], [JS_LIB_BASE=mozjs], [
> -            AC_CHECK_LIB([js], [JS_NewContext], [JS_LIB_BASE=js], [
> -                AC_CHECK_LIB([js3250], [JS_NewContext], [JS_LIB_BASE=js3250],
[
> -                    AC_CHECK_LIB([js32], [JS_NewContext], [JS_LIB_BASE=js32],
[
> -                        AC_MSG_ERROR([Could not find the js library.
> -
> -Is the Mozilla SpiderMonkey library installed?])])])])])])])
> -
> -# Figure out what version of SpiderMonkey to use
> -
> -AC_CHECK_LIB([$JS_LIB_BASE], [JS_NewCompartmentAndGlobalObject],
> -    # Prevent people from accidentally using SpiderMonkey's that are too new
> +use_js170=no
> +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
> +                   #include <jsconfig.h>
> +                   ]],
> +                   [[
> +                    #if JS_VERSION == 170
> +                    // Use spidermonkey 1.7.0
> +                    #else
> +                    # Not Spidermonkey 1.7.0
> +                    #endif
> +                    ]])],
> +                    [use_js170=yes],
> +                    [])
> +
> +if test "$use_js170" = "no"; then
> +    AC_CHECK_LIB([mozjs185], [JS_NewContext], [JS_LIB_BASE=mozjs185], [
> +        AC_CHECK_LIB([mozjs185-1.0], [JS_NewContext], [JS_LIB_BASE=mozjs185-1.0],
[
> +            AC_CHECK_LIB([mozjs], [JS_NewContext], [JS_LIB_BASE=mozjs], [
> +                AC_CHECK_LIB([js], [JS_NewContext], [JS_LIB_BASE=js], [
> +                    AC_CHECK_LIB([js3250], [JS_NewContext], [JS_LIB_BASE=js3250],
[
> +                        AC_CHECK_LIB([js32], [JS_NewContext], [JS_LIB_BASE=js32],
[
> +                            AC_MSG_ERROR([Could not find the js library.
> +
> +    Is the Mozilla SpiderMonkey library installed?])])])])])])])
> +
> +    # Figure out what version of SpiderMonkey to use
> +
> +    AC_CHECK_LIB([$JS_LIB_BASE], [JS_NewCompartmentAndGlobalObject],
> +        # Prevent people from accidentally using SpiderMonkey's that are too new
> +
> +        if test "$use_js_trunk" = "no"; then
> +            AC_CHECK_DECL([JSOPTION_ANONFUNFIX], [], [
> +                AC_MSG_ERROR([Your SpiderMonkey library is too new.
> +
> +    Versions of SpiderMonkey after the js185-1.0.0 release remove the optional
> +    enforcement of preventing anonymous functions in a statement context. This
> +    will most likely break your existing JavaScript code as well as render all
> +    example code invalid.
> +
> +    If you wish to ignore this error pass --enable-js-trunk to ./configure.])],
> +            [[#include <jsapi.h>]])
> +        fi
> +        AC_DEFINE([SM185], [1],
> +            [Use SpiderMonkey 1.8.5]))
>
> -    if test "$use_js_trunk" = "no"; then
> -        AC_CHECK_DECL([JSOPTION_ANONFUNFIX], [], [
> -            AC_MSG_ERROR([Your SpiderMonkey library is too new.
> +    AC_CHECK_LIB([$JS_LIB_BASE], [JS_ThrowStopIteration],
> +        AC_DEFINE([SM180], [1],
> +            [Use SpiderMonkey 1.8.0]))
>
> -Versions of SpiderMonkey after the js185-1.0.0 release remove the optional
> -enforcement of preventing anonymous functions in a statement context. This
> -will most likely break your existing JavaScript code as well as render all
> -example code invalid.
> +else
>
> -If you wish to ignore this error pass --enable-js-trunk to ./configure.])],
> -        [[#include <jsapi.h>]])
> -    fi
> -    AC_DEFINE([SM185], [1],
> -        [Use SpiderMonkey 1.8.5]))
> +    AC_CHECK_LIB([mozjs], [JS_NewContext], [JS_LIB_BASE=mozjs], [
> +                AC_CHECK_LIB([js], [JS_NewContext], [JS_LIB_BASE=js], [
> +                    AC_CHECK_LIB([js3250], [JS_NewContext], [JS_LIB_BASE=js3250],
[
> +                        AC_CHECK_LIB([js32], [JS_NewContext], [JS_LIB_BASE=js32],
[
> +                            AC_MSG_ERROR([Could not find the js library.
>
> -AC_CHECK_LIB([$JS_LIB_BASE], [JS_ThrowStopIteration],
> -    AC_DEFINE([SM180], [1],
> -        [Use SpiderMonkey 1.8.0]))
> +    Is the Mozilla SpiderMonkey library installed?])])])])])
> +fi
>
>  AC_CHECK_LIB([$JS_LIB_BASE], [JS_GetStringCharsAndLength],
>     AC_DEFINE([HAVE_JS_GET_STRING_CHARS_AND_LENGTH], [1],
>

Mime
View raw message