Re: [PATCH] vlc: rework qt PACKAGECONFIG

[Paul Barker <paul@betafive.co.uk>]

On 23/04/2019 01:07, Andreas Müller wrote:
> On Tue, Apr 23, 2019 at 1:55 AM Paul Barker <paul@betafive.co.uk> wrote:
>>
>> On 23/04/2019 00:46, Andreas Müller wrote:
>>> * qt4 support is gone -> move to qt5
>>> * enable qt5 gui in case meta-qt5 is in layers (it is more or less useless
>>>     without gui)
>>> * while at it remove noop libtool copy
>>>
>>> Signed-off-by: Andreas Müller <schnitzeltony@gmail.com>
>>> ---
>>> V1 -> V2: Aargh again typo in commit messeage...
>>>    .../recipes-multimedia/vlc/vlc.inc            | 25 ++++++++++++-------
>>>    1 file changed, 16 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/meta-multimedia/recipes-multimedia/vlc/vlc.inc b/meta-multimedia/recipes-multimedia/vlc/vlc.inc
>>> index 19ac8206c..6ba1444b5 100644
>>> --- a/meta-multimedia/recipes-multimedia/vlc/vlc.inc
>>> +++ b/meta-multimedia/recipes-multimedia/vlc/vlc.inc
>>> @@ -36,12 +36,18 @@ EXTRA_OECONF = "\
>>>        --enable-realrtsp \
>>>        --disable-libtar \
>>>        --enable-avcodec \
>>> -    ac_cv_path_MOC=${STAGING_BINDIR_NATIVE}/moc4 \
>>> -    ac_cv_path_RCC=${STAGING_BINDIR_NATIVE}/rcc4 \
>>> -    ac_cv_path_UIC=${STAGING_BINDIR_NATIVE}/uic4 \
>>> +    ac_cv_path_MOC=${STAGING_BINDIR_NATIVE}${QT_DIR_NAME}/moc \
>>> +    ac_cv_path_RCC=${STAGING_BINDIR_NATIVE}${QT_DIR_NAME}/rcc \
>>> +    ac_cv_path_UIC=${STAGING_BINDIR_NATIVE}${QT_DIR_NAME}/uic \
>>>    "
>>>
>>> -PACKAGECONFIG ?= " live555 dc1394 dv1394 notify fontconfig freetype dvdread png ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11', '', d)}"
>>> +PACKAGECONFIG ?= " \
>>> +    live555 dc1394 dv1394 notify fontconfig freetype dvdread png \
>>> +    ${@bb.utils.contains('DISTRO_FEATURES', 'x11', 'x11', '', d)} \
>>> +    ${@bb.utils.contains('BBFILE_COLLECTIONS', 'qt5-layer', 'qt5', '', d)} \
>>
>> I don't like this change based on BBFILE_COLLECTIONS, it's just a
>> different way of having things change when you add an extra layer. We
>> wouldn't want an unconditional bbappend in meta-qt5 to set this and so
>> we don't want to set it here when that layer is added.
>>
>> Perhaps we should have qt5 as a distro feature instead?
> Isn't that a different naming for the same thing not working properly
> if you don't have meta-qt5 in layers?
> 

Agreed, we shouldn't give people more opportunities to shoot themselves 
in the foot!

For my builds I have all layers added all the time and just 
enable/disable features as needed. So just because meta-qt5 is included 
as a layer doesn't mean I'm even building an x11 or weston 
configuration, it could be command line only in which case I wouldn't 
want this extra PACKAGECONFIG creeping in.

meta-virtualization handles this by gating overrides on a 
'virtualization' distro feature and adding a sanity check that you 
enabled that distro feature when adding the layer 
(http://git.yoctoproject.org/cgit/cgit.cgi/meta-virtualization/tree/classes/sanity-meta-virt.bbclass). 
The sanity check can be disabled by setting SKIP_META_VIRT_SANITY_CHECK. 
I find it a bit messy but it does work.

What would work for this case is a bbappend in meta-qt5 which checks for 
the 'qt5' distro feature. The benefit of using a distro feature is that 
we can gate changes to other recipes on the same configuration setting 
so that we don't need to set lots of PACKAGECONFIG values explicitly in 
a distro or local.conf.

Forgive my 01:40 AM brain if any of that doesn't make sense.

> Andreas
> 

-- 
Paul Barker
Managing Director & Principal Engineer
Beta Five Ltd