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

[Lukas Bulwahn <lukas.bulwahn@oss.bmw-carit.de>]

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.