[PATCH] apr_cv_mutex_recursive=yes added to apr_1.4.6.bb to make rosnodes work

[PATCH] apr_cv_mutex_recursive=yes added to apr_1.4.6.bb to make rosnodes work

[vmayoral]

From: victor <v.mayoralv@gmail.com>

Working with the meta-ros project we detected that the ROS nodes didn't launch properly
the reason was that by default apr_cv_mutex_recursive in apr is set to no and this leads
to the APRENOTIMPL return value of apr_thread_mutex_create in thread_mutex.c when
APR_THREAD_MUTEX_NESTED is requested via flags.

Added CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes" to sources/openembedded-core/meta/recipes-support/apr/apr_1.4.6.bb
to fix this issue. It has also been removed the mention of this variable in
meta/site/powerpc32-linux.

Signed-off-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
---
 meta/recipes-support/apr/apr_1.4.6.bb |    3 +++
 meta/site/powerpc32-linux             |    1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/meta/recipes-support/apr/apr_1.4.6.bb b/meta/recipes-support/apr/apr_1.4.6.bb
index 896f79f..ba59639 100644
--- a/meta/recipes-support/apr/apr_1.4.6.bb
+++ b/meta/recipes-support/apr/apr_1.4.6.bb
@@ -23,6 +23,9 @@ inherit autotools lib_package binconfig multilib_header
 
 OE_BINCONFIG_EXTRA_MANGLE = " -e 's:location=source:location=installed:'"
 
+# Added to fix some issues with cmake. Refer to https://github.com/bmwcarit/meta-ros/issues/68#issuecomment-19896928
+CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes"
+
 do_configure_prepend() {
 	cd ${S}
 	./buildconf
diff --git a/meta/site/powerpc32-linux b/meta/site/powerpc32-linux
index 4550df3..b3973c9 100644
--- a/meta/site/powerpc32-linux
+++ b/meta/site/powerpc32-linux
@@ -203,7 +203,6 @@ apr_cv_use_lfs64=${apr_cv_use_lfs64=yes}
 apr_cv_epoll=${apr_cv_epoll=yes}
 apr_cv_pthreads_cflags=${apr_cv_pthreads_cflags=-pthread}
 apr_cv_pthreads_lib=${apr_cv_pthreads_lib=-lpthread}
-apr_cv_mutex_recursive=${apr_cv_mutex_recursive=yes}
 ac_cv_func_mmap=${ac_cv_func_mmap=yes}
 ac_cv_file__dev_zero=${ac_cv_file__dev_zero=yes}
 ac_cv_sizeof_off_t=${ac_cv_sizeof_off_t=4}
-- 
1.7.9.5

Re: [PATCH] apr_cv_mutex_recursive=yes added to apr_1.4.6.bb to make rosnodes work

[Saul Wold]

On 07/03/2013 06:04 AM, vmayoral wrote:
> From: victor <v.mayoralv@gmail.com>
>
> Working with the meta-ros project we detected that the ROS nodes didn't launch properly
> the reason was that by default apr_cv_mutex_recursive in apr is set to no and this leads
> to the APRENOTIMPL return value of apr_thread_mutex_create in thread_mutex.c when
> APR_THREAD_MUTEX_NESTED is requested via flags.
>
> Added CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes" to sources/openembedded-core/meta/recipes-support/apr/apr_1.4.6.bb
> to fix this issue. It has also been removed the mention of this variable in
> meta/site/powerpc32-linux.
>
> Signed-off-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
> ---
>   meta/recipes-support/apr/apr_1.4.6.bb |    3 +++
>   meta/site/powerpc32-linux             |    1 -
>   2 files changed, 3 insertions(+), 1 deletion(-)
>
This really should be 2 patches, I know it was mentioned that you should 
make the change at the same time.  But it should be done in seperate 
patches since they actually accomplishing different things.

Also, the summary title of the commit should be in the format of:

recipe or file: <Summary>

So in your case:

apr: add apr_cv_mutex_recursive=yes to support meta-ros

<full commit messaage>

powerpc32-linux: remove apr_cv_mutex_recurisve

...


Thanks
	Sau!

> diff --git a/meta/recipes-support/apr/apr_1.4.6.bb b/meta/recipes-support/apr/apr_1.4.6.bb
> index 896f79f..ba59639 100644
> --- a/meta/recipes-support/apr/apr_1.4.6.bb
> +++ b/meta/recipes-support/apr/apr_1.4.6.bb
> @@ -23,6 +23,9 @@ inherit autotools lib_package binconfig multilib_header
>
>   OE_BINCONFIG_EXTRA_MANGLE = " -e 's:location=source:location=installed:'"
>
> +# Added to fix some issues with cmake. Refer to https://github.com/bmwcarit/meta-ros/issues/68#issuecomment-19896928
> +CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes"
> +
>   do_configure_prepend() {
>   	cd ${S}
>   	./buildconf
> diff --git a/meta/site/powerpc32-linux b/meta/site/powerpc32-linux
> index 4550df3..b3973c9 100644
> --- a/meta/site/powerpc32-linux
> +++ b/meta/site/powerpc32-linux
> @@ -203,7 +203,6 @@ apr_cv_use_lfs64=${apr_cv_use_lfs64=yes}
>   apr_cv_epoll=${apr_cv_epoll=yes}
>   apr_cv_pthreads_cflags=${apr_cv_pthreads_cflags=-pthread}
>   apr_cv_pthreads_lib=${apr_cv_pthreads_lib=-lpthread}
> -apr_cv_mutex_recursive=${apr_cv_mutex_recursive=yes}
>   ac_cv_func_mmap=${ac_cv_func_mmap=yes}
>   ac_cv_file__dev_zero=${ac_cv_file__dev_zero=yes}
>   ac_cv_sizeof_off_t=${ac_cv_sizeof_off_t=4}
>

Re: [PATCH] apr_cv_mutex_recursive=yes added to apr_1.4.6.bb to make rosnodes work

[Khem Raj]

On Jul 3, 2013, at 1:11 PM, Saul Wold <sgw@linux.intel.com> wrote:

> On 07/03/2013 06:04 AM, vmayoral wrote:
>> From: victor <v.mayoralv@gmail.com>
>> 
>> Working with the meta-ros project we detected that the ROS nodes didn't launch properly
>> the reason was that by default apr_cv_mutex_recursive in apr is set to no and this leads
>> to the APRENOTIMPL return value of apr_thread_mutex_create in thread_mutex.c when
>> APR_THREAD_MUTEX_NESTED is requested via flags.
>> 
>> Added CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes" to sources/openembedded-core/meta/recipes-support/apr/apr_1.4.6.bb
>> to fix this issue. It has also been removed the mention of this variable in
>> meta/site/powerpc32-linux.
>> 
>> Signed-off-by: Víctor Mayoral Vilches <v.mayoralv@gmail.com>
>> ---
>>  meta/recipes-support/apr/apr_1.4.6.bb |    3 +++
>>  meta/site/powerpc32-linux             |    1 -
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>> 
> This really should be 2 patches, I know it was mentioned that you should make the change at the same time.  But it should be done in seperate patches since they actually accomplishing different things.

You don't want two patches here since logically you are moving the define from global space to recipe space.
its aimed at same things.

> 
> Also, the summary title of the commit should be in the format of:
> 
> recipe or file: <Summary>
> 
> So in your case:
> 
> apr: add apr_cv_mutex_recursive=yes to support meta-ros
> 
> <full commit messaage>
> 
> powerpc32-linux: remove apr_cv_mutex_recurisve
> 
> ...
> 
> 
> Thanks
> 	Sau!
> 
>> diff --git a/meta/recipes-support/apr/apr_1.4.6.bb b/meta/recipes-support/apr/apr_1.4.6.bb
>> index 896f79f..ba59639 100644
>> --- a/meta/recipes-support/apr/apr_1.4.6.bb
>> +++ b/meta/recipes-support/apr/apr_1.4.6.bb
>> @@ -23,6 +23,9 @@ inherit autotools lib_package binconfig multilib_header
>> 
>>  OE_BINCONFIG_EXTRA_MANGLE = " -e 's:location=source:location=installed:'"
>> 
>> +# Added to fix some issues with cmake. Refer to https://github.com/bmwcarit/meta-ros/issues/68#issuecomment-19896928
>> +CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes"
>> +
>>  do_configure_prepend() {
>>  	cd ${S}
>>  	./buildconf
>> diff --git a/meta/site/powerpc32-linux b/meta/site/powerpc32-linux
>> index 4550df3..b3973c9 100644
>> --- a/meta/site/powerpc32-linux
>> +++ b/meta/site/powerpc32-linux
>> @@ -203,7 +203,6 @@ apr_cv_use_lfs64=${apr_cv_use_lfs64=yes}
>>  apr_cv_epoll=${apr_cv_epoll=yes}
>>  apr_cv_pthreads_cflags=${apr_cv_pthreads_cflags=-pthread}
>>  apr_cv_pthreads_lib=${apr_cv_pthreads_lib=-lpthread}
>> -apr_cv_mutex_recursive=${apr_cv_mutex_recursive=yes}
>>  ac_cv_func_mmap=${ac_cv_func_mmap=yes}
>>  ac_cv_file__dev_zero=${ac_cv_file__dev_zero=yes}
>>  ac_cv_sizeof_off_t=${ac_cv_sizeof_off_t=4}
>> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] apr_cv_mutex_recursive=yes added to apr_1.4.6.bb to make rosnodes work

[Lukas Bulwahn]

Hi Victor,

just some comments on your commit message:

On 07/03/2013 03:04 PM, vmayoral wrote:
> From: victor <v.mayoralv@gmail.com>
>
> Working with the meta-ros project we detected that the ROS nodes didn't launch properly
This very specific meta-ros problem should not be the first line, but if 
at all, the last one in the commit message.

> the reason was that by default apr_cv_mutex_recursive in apr is set to no and this leads
> to the APRENOTIMPL return value of apr_thread_mutex_create in thread_mutex.c when
> APR_THREAD_MUTEX_NESTED is requested via flags.
instead of mentioning meta-ros, be more specific what happens in general:
... This then leads to a deadlock in applications using apr, as observed 
in an application with log4cxx.

> Added CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes" to sources/openembedded-core/meta/recipes-support/apr/apr_1.4.6.bb
> to fix this issue. It has also been removed the mention of this variable in
> meta/site/powerpc32-linux.
Maybe, complete sentences are better as well:
This commit adds CACHED_CONFIGUREVARS += "apr_cv_mutex_recursive=yes" to 
apr_1.4.6.bb and removes this variable in powerpc32-linux to address 
this issue.

(Use short file names: The exact location of the files is directly 
visible in the commit anyway.)
(Do not use "fix": It might happen that we have to look at this issue 
again at some point in the future, and then addressing an issue again 
after a "fix", seems inconsistent in retrospective. Better just say, we 
addressed it now.)

In the very end, you can refer to meta-ros, but then mention the url of 
the github repository and/or the url of the concrete issue where we 
discussed the problem. The meta-ros project and the issue you refer to 
is not globally well-known.

Just my two cents,

Lukas

P.S.: Thanks a lot for taking the initiative to carry the issue from our 
meta-ros project to the OpenEmbedded community.