[Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Quentin Schulz]

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

The cam application requires libevent. Since there's no Kconfig option
for it, cam building ability is checked by meson build system by default.

If libevent is present in the sysroot, cam is built.

The issue is that there's no explicit dependency on libevent in
libcamera package. This means that it is possible for libevent AND
libcamera to be built, but have libcamera be built before libevent.
Meaning that even if all requirements seem to be fulfilled, cam still
won't be enabled in some cases.

This fixes the possible race by expliciting the dependency to libevent
if the libevent package is enabled. Otherwise, explicitly disable cam
building as it's already known that it isn't going to build.

Cc: Quentin Schulz <foss+buildroot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---

v2:
 - added -Dcam=enabled when libevent package is enabled, to catch
 possible additional dependencies to cam in later upgrades,

 package/libcamera/libcamera.mk | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
index 41d6a5abef..3f336cb797 100644
--- a/package/libcamera/libcamera.mk
+++ b/package/libcamera/libcamera.mk
@@ -84,6 +84,13 @@ else
 LIBCAMERA_CONF_OPTS += -Dqcam=disabled
 endif
 
+ifeq ($(BR2_PACKAGE_LIBEVENT),y)
+LIBCAMERA_CONF_OPTS += -Dcam=enabled
+LIBCAMERA_DEPENDENCIES += libevent
+else
+LIBCAMERA_CONF_OPTS += -Dcam=disabled
+endif
+
 ifeq ($(BR2_PACKAGE_TIFF),y)
 LIBCAMERA_DEPENDENCIES += tiff
 endif
-- 
2.36.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Kieran Bingham]

Quoting Quentin Schulz (2022-07-11 14:18:23)
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> The cam application requires libevent. Since there's no Kconfig option
> for it, cam building ability is checked by meson build system by default.
> 
> If libevent is present in the sysroot, cam is built.
> 
> The issue is that there's no explicit dependency on libevent in
> libcamera package. This means that it is possible for libevent AND
> libcamera to be built, but have libcamera be built before libevent.
> Meaning that even if all requirements seem to be fulfilled, cam still
> won't be enabled in some cases.
> 
> This fixes the possible race by expliciting the dependency to libevent
> if the libevent package is enabled. Otherwise, explicitly disable cam
> building as it's already known that it isn't going to build.

I'm not opposed to this as it fixes the race though. I guess it depends
how much resistance there is to adding a specific libcamera kconfig
option to select cam.

> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
> 
> v2:
>  - added -Dcam=enabled when libevent package is enabled, to catch
>  possible additional dependencies to cam in later upgrades,
> 
>  package/libcamera/libcamera.mk | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 41d6a5abef..3f336cb797 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -84,6 +84,13 @@ else
>  LIBCAMERA_CONF_OPTS += -Dqcam=disabled
>  endif
>  
> +ifeq ($(BR2_PACKAGE_LIBEVENT),y)
> +LIBCAMERA_CONF_OPTS += -Dcam=enabled
> +LIBCAMERA_DEPENDENCIES += libevent
> +else
> +LIBCAMERA_CONF_OPTS += -Dcam=disabled
> +endif
> +
>  ifeq ($(BR2_PACKAGE_TIFF),y)
>  LIBCAMERA_DEPENDENCIES += tiff

I see we're already doing things like this though... And I think tiff is
only used by cam? (Or perhaps also qcam).

--
Kieran


>  endif
> -- 
> 2.36.1
>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Quentin Schulz]

Hi Kieran,

On 7/11/22 21:53, Kieran Bingham wrote:
> Quoting Quentin Schulz (2022-07-11 14:18:23)
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> The cam application requires libevent. Since there's no Kconfig option
>> for it, cam building ability is checked by meson build system by default.
>>
>> If libevent is present in the sysroot, cam is built.
>>
>> The issue is that there's no explicit dependency on libevent in
>> libcamera package. This means that it is possible for libevent AND
>> libcamera to be built, but have libcamera be built before libevent.
>> Meaning that even if all requirements seem to be fulfilled, cam still
>> won't be enabled in some cases.
>>
>> This fixes the possible race by expliciting the dependency to libevent
>> if the libevent package is enabled. Otherwise, explicitly disable cam
>> building as it's already known that it isn't going to build.
> 
> I'm not opposed to this as it fixes the race though. I guess it depends
> how much resistance there is to adding a specific libcamera kconfig
> option to select cam.
> 
>>
>> Cc: Quentin Schulz <foss+buildroot@0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> ---
>>
>> v2:
>>   - added -Dcam=enabled when libevent package is enabled, to catch
>>   possible additional dependencies to cam in later upgrades,
>>
>>   package/libcamera/libcamera.mk | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
>> index 41d6a5abef..3f336cb797 100644
>> --- a/package/libcamera/libcamera.mk
>> +++ b/package/libcamera/libcamera.mk
>> @@ -84,6 +84,13 @@ else
>>   LIBCAMERA_CONF_OPTS += -Dqcam=disabled
>>   endif
>>   
>> +ifeq ($(BR2_PACKAGE_LIBEVENT),y)
>> +LIBCAMERA_CONF_OPTS += -Dcam=enabled
>> +LIBCAMERA_DEPENDENCIES += libevent
>> +else
>> +LIBCAMERA_CONF_OPTS += -Dcam=disabled
>> +endif
>> +
>>   ifeq ($(BR2_PACKAGE_TIFF),y)
>>   LIBCAMERA_DEPENDENCIES += tiff
> 
> I see we're already doing things like this though... And I think tiff is
> only used by cam? (Or perhaps also qcam).
> 

qcam meson.build is the only one specifying the dependency and 
README.rst only mentions this dependency for qcam.

I guess we could move libtiff next to the:
ifeq ($(BR2_PACKAGE_QT5BASE_WIDGETS),y)
check to "force" enabling of qcam if qt widgets and libtiff packages are 
enabled?

Cheers,
Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Kieran Bingham]

Quoting Quentin Schulz (2022-07-12 09:00:51)
> Hi Kieran,
> 
> On 7/11/22 21:53, Kieran Bingham wrote:
> > Quoting Quentin Schulz (2022-07-11 14:18:23)
> >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>
> >> The cam application requires libevent. Since there's no Kconfig option
> >> for it, cam building ability is checked by meson build system by default.
> >>
> >> If libevent is present in the sysroot, cam is built.
> >>
> >> The issue is that there's no explicit dependency on libevent in
> >> libcamera package. This means that it is possible for libevent AND
> >> libcamera to be built, but have libcamera be built before libevent.
> >> Meaning that even if all requirements seem to be fulfilled, cam still
> >> won't be enabled in some cases.
> >>
> >> This fixes the possible race by expliciting the dependency to libevent
> >> if the libevent package is enabled. Otherwise, explicitly disable cam
> >> building as it's already known that it isn't going to build.
> > 
> > I'm not opposed to this as it fixes the race though. I guess it depends
> > how much resistance there is to adding a specific libcamera kconfig
> > option to select cam.
> > 
> >>
> >> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >> ---
> >>
> >> v2:
> >>   - added -Dcam=enabled when libevent package is enabled, to catch
> >>   possible additional dependencies to cam in later upgrades,
> >>
> >>   package/libcamera/libcamera.mk | 7 +++++++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> >> index 41d6a5abef..3f336cb797 100644
> >> --- a/package/libcamera/libcamera.mk
> >> +++ b/package/libcamera/libcamera.mk
> >> @@ -84,6 +84,13 @@ else
> >>   LIBCAMERA_CONF_OPTS += -Dqcam=disabled
> >>   endif
> >>   
> >> +ifeq ($(BR2_PACKAGE_LIBEVENT),y)
> >> +LIBCAMERA_CONF_OPTS += -Dcam=enabled
> >> +LIBCAMERA_DEPENDENCIES += libevent
> >> +else
> >> +LIBCAMERA_CONF_OPTS += -Dcam=disabled
> >> +endif
> >> +
> >>   ifeq ($(BR2_PACKAGE_TIFF),y)
> >>   LIBCAMERA_DEPENDENCIES += tiff
> > 
> > I see we're already doing things like this though... And I think tiff is
> > only used by cam? (Or perhaps also qcam).
> > 
> 
> qcam meson.build is the only one specifying the dependency and 
> README.rst only mentions this dependency for qcam.

Ah, yes, I see now. libtiff is used for writing out RAW / DNG files. So
this is an 'optional feature' of qcam.

We're into optional features of optional components ;-) - how many
levels should be configurable, vs 'detected' based on the rest of the
system image I wonder.


> I guess we could move libtiff next to the:
> ifeq ($(BR2_PACKAGE_QT5BASE_WIDGETS),y)
> check to "force" enabling of qcam if qt widgets and libtiff packages are 
> enabled?

I think we'd only 'force' enabling qcam (+/dng writer) if the user
explicitly selected a qcam sub option for libcamera (which doesn't yet
exist). Maybe cam/qcam should be configurable but I don't think it's a
big deal right now.
--
Kieran


> 
> Cheers,
> Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Yann E. MORIN]

Kieran, Quentin, All,

On 2022-07-13 10:02 +0100, Kieran Bingham spake thusly:
> Quoting Quentin Schulz (2022-07-12 09:00:51)
> > On 7/11/22 21:53, Kieran Bingham wrote:
> > > Quoting Quentin Schulz (2022-07-11 14:18:23)
> > >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > >> If libevent is present in the sysroot, cam is built.
[--SNIP--]
> > >> This fixes the possible race by expliciting the dependency to libevent
> > >> if the libevent package is enabled. Otherwise, explicitly disable cam
> > >> building as it's already known that it isn't going to build.
> > > I'm not opposed to this as it fixes the race though. I guess it depends
> > > how much resistance there is to adding a specific libcamera kconfig
> > > option to select cam.
[--SNIP details--]

For now, I've applied just this patch. Since you said can is just an
optional, test component, that is good enough as is.

If there are more complex conditions and new dependencies in the future,
we can revisit that, and add a Kconfig option to explicitly enable cam.
That option would be responsible for 1) select-ing needed dependencies
in Kconfig, and 2) add the proper dependencies and config flags in the
.mk.

Until then, the current situation is good enough (for me!). Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Yann E. MORIN]

Quentin, All,

On 2022-07-11 15:18 +0200, Quentin Schulz spake thusly:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> The cam application requires libevent. Since there's no Kconfig option
> for it, cam building ability is checked by meson build system by default.
> 
> If libevent is present in the sysroot, cam is built.
> 
> The issue is that there's no explicit dependency on libevent in
> libcamera package. This means that it is possible for libevent AND
> libcamera to be built, but have libcamera be built before libevent.
> Meaning that even if all requirements seem to be fulfilled, cam still
> won't be enabled in some cases.
> 
> This fixes the possible race by expliciting the dependency to libevent
> if the libevent package is enabled. Otherwise, explicitly disable cam
> building as it's already known that it isn't going to build.
> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
> 
> v2:
>  - added -Dcam=enabled when libevent package is enabled, to catch
>  possible additional dependencies to cam in later upgrades,
> 
>  package/libcamera/libcamera.mk | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 41d6a5abef..3f336cb797 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -84,6 +84,13 @@ else
>  LIBCAMERA_CONF_OPTS += -Dqcam=disabled
>  endif
>  
> +ifeq ($(BR2_PACKAGE_LIBEVENT),y)
> +LIBCAMERA_CONF_OPTS += -Dcam=enabled
> +LIBCAMERA_DEPENDENCIES += libevent
> +else
> +LIBCAMERA_CONF_OPTS += -Dcam=disabled
> +endif
> +
>  ifeq ($(BR2_PACKAGE_TIFF),y)
>  LIBCAMERA_DEPENDENCIES += tiff
>  endif
> -- 
> 2.36.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v2] package/libcamera: add explicit dependency on libevent if libevent package to be built

[Peter Korsgaard]

>>>>> "Quentin" == Quentin Schulz <foss+buildroot@0leil.net> writes:

 > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
 > The cam application requires libevent. Since there's no Kconfig option
 > for it, cam building ability is checked by meson build system by default.

 > If libevent is present in the sysroot, cam is built.

 > The issue is that there's no explicit dependency on libevent in
 > libcamera package. This means that it is possible for libevent AND
 > libcamera to be built, but have libcamera be built before libevent.
 > Meaning that even if all requirements seem to be fulfilled, cam still
 > won't be enabled in some cases.

 > This fixes the possible race by expliciting the dependency to libevent
 > if the libevent package is enabled. Otherwise, explicitly disable cam
 > building as it's already known that it isn't going to build.

 > Cc: Quentin Schulz <foss+buildroot@0leil.net>
 > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
 > ---

 > v2:
 >  - added -Dcam=enabled when libevent package is enabled, to catch
 >  possible additional dependencies to cam in later upgrades,

Committed to 2022.05.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot