[libcamera-devel] libcamera: Fix compilation issue in musl environment

2759 diff mbox series

From: Madhavan Krishnan <madhavan.krishnan@linaro.org>

New argument dev_t devnum has been added in the registerCamera()
api and we are facing compilation issue in the musl environment
since dev_t is not declared, we addded sys/stat.h header file
---
 src/libcamera/include/pipeline_handler.h | 1 +
 1 file changed, 1 insertion(+)

Hi Madhavan,

The patch is missing your Signed off by (need to use -s with
git commit command).

Peter.

On Wed, 29 Jan 2020 at 11:16, Madhavan Krishnan <
madhavan.krishnan@linaro.org> wrote:

> From: Madhavan Krishnan <madhavan.krishnan@linaro.org>
>
> New argument dev_t devnum has been added in the registerCamera()
> api and we are facing compilation issue in the musl environment
> since dev_t is not declared, we addded sys/stat.h header file
> ---
>  src/libcamera/include/pipeline_handler.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/libcamera/include/pipeline_handler.h
> b/src/libcamera/include/pipeline_handler.h
> index a6c1e1f..c5f84aa 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -12,6 +12,7 @@
>  #include <memory>
>  #include <set>
>  #include <string>
> +#include <sys/stat.h>
>  #include <sys/sysmacros.h>
>  #include <vector>
>
> --
> 2.7.4
>
>

Hi Madhavan,

Thank you for the patch.

On Wed, Jan 29, 2020 at 12:16:27PM +0100, Madhavan Krishnan wrote:
> From: Madhavan Krishnan <madhavan.krishnan@linaro.org>
> 
> New argument dev_t devnum has been added in the registerCamera()
> api and we are facing compilation issue in the musl environment
> since dev_t is not declared, we addded sys/stat.h header file

In addition to your Signed-off-by line, this should also have a Fixes
line pointing to the commit that introduced the issue. See for instance
commit 744fabcbb938d60cdbc0dfadc625bbd6798c2485 for a example.

> ---
>  src/libcamera/include/pipeline_handler.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index a6c1e1f..c5f84aa 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -12,6 +12,7 @@
>  #include <memory>
>  #include <set>
>  #include <string>
> +#include <sys/stat.h>

POSIX mandates dev_t to be defined by sys/types.h, so I think that would
be a better header to include. sys/sysmacros.h should then be dropped as
it wasn't the correct choice. You can explain this in the commit
message, stating that sys/sysmacros.h was an incorrect choice, which
doesn't work with musl.

>  #include <sys/sysmacros.h>
>  #include <vector>
>

Hi Laurent, Madhavan,

Aye aye aye,

I've already submitted such a patch, and it's been reviewed - so I must
not have integrated it yet :-(

https://lists.libcamera.org/pipermail/libcamera-devel/2020-January/006315.html

I'll go check out why it's not yet upstream and make sure something gets
up fast.

Now that this has been reported directly as well I'll add:
"Reported-by: Madhavan Krishnan <madhavan.krishnan@linaro.org>" to my patch.



On 29/01/2020 21:49, Laurent Pinchart wrote:
> Hi Madhavan,
> 
> Thank you for the patch.
> 
> On Wed, Jan 29, 2020 at 12:16:27PM +0100, Madhavan Krishnan wrote:
>> From: Madhavan Krishnan <madhavan.krishnan@linaro.org>
>>
>> New argument dev_t devnum has been added in the registerCamera()
>> api and we are facing compilation issue in the musl environment
>> since dev_t is not declared, we addded sys/stat.h header file
> 
> In addition to your Signed-off-by line, this should also have a Fixes
> line pointing to the commit that introduced the issue. See for instance
> commit 744fabcbb938d60cdbc0dfadc625bbd6798c2485 for a example.
> 
>> ---
>>  src/libcamera/include/pipeline_handler.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
>> index a6c1e1f..c5f84aa 100644
>> --- a/src/libcamera/include/pipeline_handler.h
>> +++ b/src/libcamera/include/pipeline_handler.h
>> @@ -12,6 +12,7 @@
>>  #include <memory>
>>  #include <set>
>>  #include <string>
>> +#include <sys/stat.h>
> 
> POSIX mandates dev_t to be defined by sys/types.h, so I think that would
> be a better header to include. sys/sysmacros.h should then be dropped as
> it wasn't the correct choice. You can explain this in the commit
> message, stating that sys/sysmacros.h was an incorrect choice, which
> doesn't work with musl.


My previous patch doesn't remove sys/sysmacros. I'll update it to do so,
retest, and repost before integrating.

--
Kieran


> 
>>  #include <sys/sysmacros.h>
>>  #include <vector>
>>  
>