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

[Omair Majid]

Hi Jiri,

* Jiri Vanek <jvanek at redhat.com> [2014-02-19 12:20]:
> On 02/18/2014 04:32 PM, Omair Majid wrote:
> >* Jiri Vanek <jvanek at redhat.com> [2014-02-18 04:43]:
> >>	* 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
> 
> zzz, what you dont like?

Sorry, I know this is nitpicking, but the guidelines ask for a specific
format.

The acinclude.m4 change, for example, should be like this:

	* acinclude.m4 (IT_CHECK_XULRUNNER_API_VERSION_CONSTCHAR): New macro.
	(IT_CHECK_XULRUNNER_API_VERSION_C11): New macro.

As in:
 - Please put the macro/function name in first place in the line
 - Colon (`:`) goes after the function/macro name.
 - New functions/macros just get `New function` or `New macro`.
 - Documentation on functions/macros go with the function/macro
   implementation, not in the Changelog.
 - Parenthesis are used where a function is declared, not called. And
   often 'all callers changed' is sufficient.

> >Would it be worth checking if compiler supports the -std=c++0x argument?
> 
> Nope. If it is not supported, we are screwed anyway, or not?

Yeah, we are. But it's much nicer if configure detects and reports this
as the error. The other case, where the build mysteriously fails is not
so nice. But it's your call whether you want to do that or not.

> The c11 aprt needs to be backported to 1.4. ok? And btw - is it
> worthy to so anrelease because of it? tarball and RPMS are now
> not-rebuildable:(

As the maintainer, new release are your call :)

Anyway, +1 from me.

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

Why not just CXXFLAGS="$CXXFLAGS $MOZILLA_CFLAGS" ?

> +  AC_COMPILE_IFELSE([
> +    #include <npfunctions.h>
> +    ][

This '][' is weird style. Wouldn't it be nicer just leaving it out? On
first look, I misread it as `],[` (with a `,` in the middle).

On the other hand, maybe use AC_LANG_PROGRAM to generate the source [1] and
you can keep the [#include <npfunctions.h>] separate. This also avoids
the autoconf warning because autoconf expects AC_LANG_SOURCE or
AC_LANG_PROGRAM to be used with AC_COMIPLE_IFELSE.

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

Please replace C11 with C++11. These are different things,
apparently [2] :)

Thanks,
Omair

[1] https://www.flameeyes.eu/autotools-mythbuster/forwardporting/autoconf.html
[2] http://en.wikipedia.org/wiki/C11_(C_standard_revision)
-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681