[rfc][icedtea-web] fix to newest xullrunner 27000000 break build

[Omair Majid]

Hi Jiri,

* Jiri Vanek <jvanek at redhat.com> [2014-02-18 04:43]:
> Half of this patch is check for xulrunner enforcing c++x11 standard.
> The second half  is reincarnation  of
> http://icedtea.classpath.org/hg/icedtea-web/rev/c5faf63fc34e

I like the patch overall, but some nits below.

> 	* acinclude.m4: added definition of (IT_CHECK_XULRUNNER_API_VERSION_CONSTCHAR),
> 	which tries to compile small program against new xulrunner api
> 	Added (IT_CHECK_XULRUNNER_API_VERSION_C11) which tries to compile program
> 	with VOID_TO_NPVARIANT call without the c++0x standard and sets CXXFLAGS
> 	accordingly.
> 	* configure.ac: added call of (IT_CHECK_XULRUNNER_API_CONSTCHAR) and
> 	(IT_CHECK_XULRUNNER_API_VERSION_C11)
> 	* plugin/icedteanp/IcedTeaNPPlugin.cc: (NP_GetMIMEDescription)
> 	return type set-up by dependency on defined LEGACY_XULRUNNERAPI.
> 	 This one is set by IT_CHECK_XULRUNNER_API_VERSION during configure.
> 	if defined, then old char* is used. New const char* is used otherwise.

Please read:
http://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs

> diff -r d2563e68c74a acinclude.m4
> --- a/acinclude.m4	Thu Feb 13 12:56:17 2014 +0100
> +++ b/acinclude.m4	Tue Feb 18 10:28:19 2014 +0100
> @@ -520,6 +520,47 @@
>     PKG_CHECK_MODULES([GLIB2_V_216],[glib-2.0 >= 2.16],[],[AC_DEFINE([LEGACY_GLIB])])
>   ])
>  
> +AC_DEFUN_ONCE([IT_CHECK_XULRUNNER_API_VERSION_CONSTCHAR],

This is nitpicking, but I find the macro name confusing. I would either
call it IT_CHECK_XULRUNNER_LEGACY_API (based on what it's changing) or
call it IT_CHECK_XULRUNNER_MIMEDESCRIPTION_CONSTCHAR (based on what it's
actually checking). I am open to other alternatives too (and in the
worst case will to live with IT_CHEC_XULRUNNER_API_VERSION_CONSTCHAR).

> +  AC_LANG_PUSH(C++)

Nice :)

> +  CXXFLAGS_BACKUP=$CXXFLAGS
> +  CXXFLAGS=$CXXFLAGS" "$MOZILLA_CFLAGS

Please double quote all CXX/CFLAGS:

    CXXFLAGS_BACKUP="$CXXFLAGS"
    CXXFLAGS="$CXXFLAGS $MOZILLA_CFLAGS"

> +  AC_TRY_COMPILE([

AC_TRY_COMPILE is obsolete [1]. Maybe use AC_COMPILE_IFELSE ?

> +    #include <npfunctions.h>
> +    const  char* NP_GetMIMEDescription ()
> +    {return (char*) "yap!";}
> +  ],[],[

The first argument to AC_TRY_COMPILE is includes and the second is the
body. You probably want to change the lines to:

    [#include <npfunctions.h>],
    [const  char* NP_GetMIMEDescription ()
    {return (char*) "yap!";}],

Well, with neater syntax.

> +AC_DEFUN_ONCE([IT_CHECK_XULRUNNER_API_VERSION_C11],
> +[
> +  AC_MSG_CHECKING([for xulrunner enforcing C11 standard])

Based on this message, why not call this macro
IT_CHECK_XULRUNNER_REQUIRES_C11?

> +    CXXFLAGS="$CXXFLAGS_BACKUP -std=c++0x"

Would it be worth checking if compiler supports the -std=c++0x argument?
Also, `man gcc` says the name 'c++0x' is deprecated. Maybe use 'c++11'
instead?

Thanks,
Omair

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/Obsolete-Macros.html

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681