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

Jiri Vanek jvanek at redhat.com
Wed Feb 19 09:20:30 PST 2014


Here you go. All should be fixed.
On 02/18/2014 04:32 PM, Omair Majid wrote:
> 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

zzz, what you dont like?
>
>> 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).

Sure - fixed.
>
>> +  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 ?

Thank you!  I'm a bit wiser again :)
>
>> +    #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.

ugh. yes. Fixed.
>
>> +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?
>

Sure fixed.
>> +    CXXFLAGS="$CXXFLAGS_BACKUP -std=c++0x"
>
> 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?

> Also, `man gcc` says the name 'c++0x' is deprecated. Maybe use 'c++11'

Sure.
> instead?
>
> Thanks,
> Omair
>
> [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/Obsolete-Macros.html
>
Thanx for review! I hop eyou will be more happy withnew version.

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:(

J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: runOnAllKnownXulrunnersSince2010_2.patch
Type: text/x-patch
Size: 2289 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140219/a51a16bb/runOnAllKnownXulrunnersSince2010_2.patch 


More information about the distro-pkg-dev mailing list