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

Jiri Vanek jvanek at redhat.com
Thu Feb 20 07:37:55 PST 2014


On 02/19/2014 08:05 PM, Omair Majid wrote:
> 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.
>

hope that is better:

	* acinclude.m4: added (IT_CHECK_XULRUNNER_API_VERSION_CONSTCHAR) macro,
	Added (IT_CHECK_XULRUNNER_API_VERSION_C11)
	* 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.
>>> 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.
>

ok as separate changeset please (how time permit)

>> 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).

ok send to separate lines
>
> 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.
>
Hmm, I tried to do so, but I was not able to keep includes separate. Maybe you know the cure. If not 
can I go with this version?

>> +  AC_MSG_CHECKING([for xulrunner enforcing C11 standard])
>
> Please replace C11 with C++11. These are different things,
> apparently [2] :)

ugh! sure fixed! Sorry:(


J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: runOnAllKnownXulrunnersSince2010_3.patch
Type: text/x-patch
Size: 2288 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140220/26b72bec/runOnAllKnownXulrunnersSince2010_3.patch 


More information about the distro-pkg-dev mailing list