[libcamera-devel,v3] meson: options: Add an option to control compilation of qcam

Message ID 20200622132948.1398048-1-niklas.soderlund@ragnatech.se
State Accepted
Commit fc520719a92f0f5cb3f673f09816686f6aaae682
Headers show
Series
  • [libcamera-devel,v3] meson: options: Add an option to control compilation of qcam
Related show

Commit Message

Niklas Söderlund June 22, 2020, 1:29 p.m. UTC
Add an option to control compilation of the qcam test application. The
default behavior is to compile qcam, no change in behavior without user
intervention.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v2
- Default to auto instead of enabled.

* Changes since v1
- Sort options alphabetical in meson_options.txt
- Use feature instead of boolean and attach it to the qt5_dep
---
 meson_options.txt    | 5 +++++
 src/qcam/meson.build | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Umang Jain June 23, 2020, 5:30 a.m. UTC | #1
Hi Niklas,

One query:
The value of global 'auto_features' defaults to 'auto' (which in turn means
require: false). I didn't see 'auto_features' being declared anywhere in the
codebase.

With this patch applied:
$ meson --auto-features=auto build && ninja -C build
built qcam for me. It shouldn't have been built because 'auto'
implies 'requires: false', no?

Maybe a meson bug or am I missing something.

On 6/22/20 6:59 PM, Niklas Söderlund wrote:
> Add an option to control compilation of the qcam test application. The
> default behavior is to compile qcam, no change in behavior without user
> intervention.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v2
> - Default to auto instead of enabled.
>
> * Changes since v1
> - Sort options alphabetical in meson_options.txt
> - Use feature instead of boolean and attach it to the qt5_dep
> ---
>   meson_options.txt    | 5 +++++
>   src/qcam/meson.build | 2 +-
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/meson_options.txt b/meson_options.txt
> index badace151bb62bc9..e9e815fde366c89d 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -19,6 +19,11 @@ option('pipelines',
>           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
>           description : 'Select which pipeline handlers to include')
>   
> +option('qcam',
> +        type : 'feature',
> +        value : 'auto',
> +        description : 'Compile the qcam test application')
> +
>   option('test',
>           type : 'boolean',
>           description: 'Compile and include the tests')
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 045db52acf26d71b..6ea886a32236d40f 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -22,7 +22,7 @@ qt5 = import('qt5')
>   qt5_dep = dependency('qt5',
>                        method : 'pkg-config',
>                        modules : ['Core', 'Gui', 'Widgets'],
> -                     required : false)
> +                     required : get_option('qcam'))
>   
>   if qt5_dep.found()
>       qcam_deps = [

Reviewed-by: Umang Jain <email@uajain.com>

Thanks!
Laurent Pinchart June 25, 2020, 3:33 a.m. UTC | #2
Hi Umang,

On Tue, Jun 23, 2020 at 05:30:45AM +0000, Umang Jain wrote:
> Hi Niklas,
> 
> One query:
> The value of global 'auto_features' defaults to 'auto' (which in turn means
> require: false). I didn't see 'auto_features' being declared anywhere in the
> codebase.
> 
> With this patch applied:
> $ meson --auto-features=auto build && ninja -C build
> built qcam for me. It shouldn't have been built because 'auto'
> implies 'requires: false', no?

"requires : false" doesn't imply that qcam won't get built, it implies
that if its dependencies are not found, it can be skipped. In this case,
with the qcam option having the value 'auto', qcam will be built if Qt
is found, and skipped otherwise. If you set the qcam option to 'enabled'
instead, and Qt is missing, meson will complain. If you set the option
to 'disabled', qcam will not be built, regardless of whether Qt is
available or not. I haven't tested this though, so my understanding
could be wrong.

> Maybe a meson bug or am I missing something.
> 
> On 6/22/20 6:59 PM, Niklas Söderlund wrote:
> > Add an option to control compilation of the qcam test application. The
> > default behavior is to compile qcam, no change in behavior without user
> > intervention.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since v2
> > - Default to auto instead of enabled.
> >
> > * Changes since v1
> > - Sort options alphabetical in meson_options.txt
> > - Use feature instead of boolean and attach it to the qt5_dep
> > ---
> >   meson_options.txt    | 5 +++++
> >   src/qcam/meson.build | 2 +-
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/meson_options.txt b/meson_options.txt
> > index badace151bb62bc9..e9e815fde366c89d 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -19,6 +19,11 @@ option('pipelines',
> >           choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
> >           description : 'Select which pipeline handlers to include')
> >   
> > +option('qcam',
> > +        type : 'feature',
> > +        value : 'auto',
> > +        description : 'Compile the qcam test application')
> > +
> >   option('test',
> >           type : 'boolean',
> >           description: 'Compile and include the tests')
> > diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> > index 045db52acf26d71b..6ea886a32236d40f 100644
> > --- a/src/qcam/meson.build
> > +++ b/src/qcam/meson.build
> > @@ -22,7 +22,7 @@ qt5 = import('qt5')
> >   qt5_dep = dependency('qt5',
> >                        method : 'pkg-config',
> >                        modules : ['Core', 'Gui', 'Widgets'],
> > -                     required : false)
> > +                     required : get_option('qcam'))
> >   
> >   if qt5_dep.found()
> >       qcam_deps = [
> 
> Reviewed-by: Umang Jain <email@uajain.com>
Umang Jain June 25, 2020, 6 a.m. UTC | #3
Hi Laurent, Niklas,

Thanks for the explanation.

Now I understand what 'auto' exactly means. :)

On 6/25/20 9:03 AM, Laurent Pinchart wrote:
> Hi Umang,
>
> On Tue, Jun 23, 2020 at 05:30:45AM +0000, Umang Jain wrote:
>> Hi Niklas,
>>
>> One query:
>> The value of global 'auto_features' defaults to 'auto' (which in turn means
>> require: false). I didn't see 'auto_features' being declared anywhere in the
>> codebase.
>>
>> With this patch applied:
>> $ meson --auto-features=auto build && ninja -C build
>> built qcam for me. It shouldn't have been built because 'auto'
>> implies 'requires: false', no?
> "requires : false" doesn't imply that qcam won't get built, it implies
> that if its dependencies are not found, it can be skipped. In this case,
> with the qcam option having the value 'auto', qcam will be built if Qt
> is found, and skipped otherwise. If you set the qcam option to 'enabled'
> instead, and Qt is missing, meson will complain. If you set the option
> to 'disabled', qcam will not be built, regardless of whether Qt is
> available or not. I haven't tested this though, so my understanding
> could be wrong.
>
>> Maybe a meson bug or am I missing something.
>>
>> On 6/22/20 6:59 PM, Niklas Söderlund wrote:
>>> Add an option to control compilation of the qcam test application. The
>>> default behavior is to compile qcam, no change in behavior without user
>>> intervention.
>>>
>>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>> * Changes since v2
>>> - Default to auto instead of enabled.
>>>
>>> * Changes since v1
>>> - Sort options alphabetical in meson_options.txt
>>> - Use feature instead of boolean and attach it to the qt5_dep
>>> ---
>>>    meson_options.txt    | 5 +++++
>>>    src/qcam/meson.build | 2 +-
>>>    2 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/meson_options.txt b/meson_options.txt
>>> index badace151bb62bc9..e9e815fde366c89d 100644
>>> --- a/meson_options.txt
>>> +++ b/meson_options.txt
>>> @@ -19,6 +19,11 @@ option('pipelines',
>>>            choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
>>>            description : 'Select which pipeline handlers to include')
>>>    
>>> +option('qcam',
>>> +        type : 'feature',
>>> +        value : 'auto',
>>> +        description : 'Compile the qcam test application')
>>> +
>>>    option('test',
>>>            type : 'boolean',
>>>            description: 'Compile and include the tests')
>>> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
>>> index 045db52acf26d71b..6ea886a32236d40f 100644
>>> --- a/src/qcam/meson.build
>>> +++ b/src/qcam/meson.build
>>> @@ -22,7 +22,7 @@ qt5 = import('qt5')
>>>    qt5_dep = dependency('qt5',
>>>                         method : 'pkg-config',
>>>                         modules : ['Core', 'Gui', 'Widgets'],
>>> -                     required : false)
>>> +                     required : get_option('qcam'))
>>>    
>>>    if qt5_dep.found()
>>>        qcam_deps = [
>> Reviewed-by: Umang Jain <email@uajain.com>

Patch

diff --git a/meson_options.txt b/meson_options.txt
index badace151bb62bc9..e9e815fde366c89d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -19,6 +19,11 @@  option('pipelines',
         choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
         description : 'Select which pipeline handlers to include')
 
+option('qcam',
+        type : 'feature',
+        value : 'auto',
+        description : 'Compile the qcam test application')
+
 option('test',
         type : 'boolean',
         description: 'Compile and include the tests')
diff --git a/src/qcam/meson.build b/src/qcam/meson.build
index 045db52acf26d71b..6ea886a32236d40f 100644
--- a/src/qcam/meson.build
+++ b/src/qcam/meson.build
@@ -22,7 +22,7 @@  qt5 = import('qt5')
 qt5_dep = dependency('qt5',
                      method : 'pkg-config',
                      modules : ['Core', 'Gui', 'Widgets'],
-                     required : false)
+                     required : get_option('qcam'))
 
 if qt5_dep.found()
     qcam_deps = [