Patchwork [1/2] qt4(-embedded).inc: create variables to ease overriding

login
register
mail settings
Submitter Andreas Oberritter
Date May 22, 2012, 11:37 a.m.
Message ID <1337686647-1027-1-git-send-email-obi@opendreambox.org>
Download mbox | patch
Permalink /patch/28269/
State New
Headers show

Comments

Andreas Oberritter - May 22, 2012, 11:37 a.m.
* No functional change besides ordering of configure arguments.

Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
---
* This doesn't cover all possible flags, but only those that I need
  to customize in my layer. Other flags may be added later if need
  arises.

 meta/recipes-qt/qt4/qt4-embedded.inc |   12 +++++++++---
 meta/recipes-qt/qt4/qt4.inc          |   17 ++++++++++++++---
 2 files changed, 23 insertions(+), 6 deletions(-)
Paul Eggleton - May 22, 2012, 12:51 p.m.
On Tuesday 22 May 2012 13:37:25 Andreas Oberritter wrote:
> * No functional change besides ordering of configure arguments.
> 
> Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
> ---
> * This doesn't cover all possible flags, but only those that I need
>   to customize in my layer. Other flags may be added later if need
>   arises.
> 
>  meta/recipes-qt/qt4/qt4-embedded.inc |   12 +++++++++---
>  meta/recipes-qt/qt4/qt4.inc          |   17 ++++++++++++++---
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/recipes-qt/qt4/qt4-embedded.inc
> b/meta/recipes-qt/qt4/qt4-embedded.inc index 8c15352..158ee17 100644
> --- a/meta/recipes-qt/qt4/qt4-embedded.inc
> +++ b/meta/recipes-qt/qt4/qt4-embedded.inc
> @@ -9,12 +9,18 @@ QT_BASE_LIB  ?= "libqt-embedded"
>  # Set necessary variables in the profile
>  SRC_URI += "file://qte.sh"
> 
> +QT_DECORATION_FLAGS ?= ""
> +QT_GFX_DRIVER_FLAGS ?= "-plugin-gfx-transformed -plugin-gfx-qvfb
> -plugin-gfx-vnc -plugin-gfx-directfb" +QT_KBD_DRIVER_FLAGS ?= "-qt-kbd-tty"
> +QT_MOUSE_DRIVER_FLAGS ?= "-plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb
> -qt-mouse-linuxinput" +
>  QT_CONFIG_FLAGS += " \
>      -embedded ${QT_ARCH} \
>      -qtlibinfix ${QT_LIBINFIX} \
> -    -plugin-gfx-transformed -plugin-gfx-qvfb -plugin-gfx-vnc
> -plugin-gfx-directfb \ -    -plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb
> -qt-mouse-linuxinput \ -    -qt-kbd-tty \
> +    ${QT_DECORATION_FLAGS} \
> +    ${QT_GFX_DRIVER_FLAGS} \
> +    ${QT_KBD_DRIVER_FLAGS} \
> +    ${QT_MOUSE_DRIVER_FLAGS} \
>      -DQT_KEYPAD_NAVIGATION \
>      "
> 
> diff --git a/meta/recipes-qt/qt4/qt4.inc b/meta/recipes-qt/qt4/qt4.inc
> index 468a46f..c70b335 100644
> --- a/meta/recipes-qt/qt4/qt4.inc
> +++ b/meta/recipes-qt/qt4/qt4.inc
> @@ -10,17 +10,28 @@ QT_ENDIAN = "${@qt_endian(d)}"
>  QT_DISTRO_FLAGS ?= "-no-accessibility -no-sm"
>  QT_DISTRO_FLAGS_linuxstdbase = "-sm"
> 
> +QT_GLIB_FLAGS ?= "-glib"
> +QT_IMAGEFORMAT_FLAGS ?= "-system-libjpeg -system-libpng -system-libtiff
> -system-zlib" +QT_PHONON_FLAGS ?= "-phonon"
> +QT_QDBUS_FLAGS ?= "-qdbus"
> +QT_QT3SUPPORT_FLAGS ?= "-qt3support"
>  QT_SQL_DRIVER_FLAGS ?= "-no-sql-ibase -no-sql-mysql -no-sql-psql
> -no-sql-odbc -plugin-sql-sqlite" +QT_WEBKIT_FLAGS ?= "-webkit"
> 
>  QT_GLFLAGS ?= ""
> 
>  QT_CONFIG_FLAGS += "-release -no-cups -reduce-relocations \
>                      -shared -no-nas-sound -no-nis \
> -                    -system-libjpeg -system-libpng -system-libtiff
> -system-zlib \ -                    -no-pch -qdbus -stl -glib -phonon
> -webkit \
> -                    -xmlpatterns -no-rpath -qt3support -silent \
> +                    -no-pch -stl \
> +                    -xmlpatterns -no-rpath -silent \
>                      ${@base_contains('DISTRO_FEATURES', 'pulseaudio',
> '--enable-pulseaudio', '--disable-pulseaudio', d)} \ +                   
> ${QT_GLIB_FLAGS} \
> +                    ${QT_IMAGEFORMAT_FLAGS} \
> +                    ${QT_PHONON_FLAGS} \
> +                    ${QT_QDBUS_FLAGS} \
> +                    ${QT_QT3SUPPORT_FLAGS} \
>                      ${QT_SQL_DRIVER_FLAGS} \
> +                    ${QT_WEBKIT_FLAGS} \
>                      ${QT_DISTRO_FLAGS} \
>                      ${QT_GLFLAGS}"

I think when we start getting to this level, especially because some of these 
options imply extra DEPENDS, we should try to use PACKAGECONFIG rather than 
specific variables.

Cheers,
Paul
Andreas Oberritter - May 22, 2012, 1:28 p.m.
On 22.05.2012 14:51, Paul Eggleton wrote:
> On Tuesday 22 May 2012 13:37:25 Andreas Oberritter wrote:
>> * No functional change besides ordering of configure arguments.
>>
>> Signed-off-by: Andreas Oberritter <obi@opendreambox.org>
>> ---
>> * This doesn't cover all possible flags, but only those that I need
>>   to customize in my layer. Other flags may be added later if need
>>   arises.
>>
>>  meta/recipes-qt/qt4/qt4-embedded.inc |   12 +++++++++---
>>  meta/recipes-qt/qt4/qt4.inc          |   17 ++++++++++++++---
>>  2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/meta/recipes-qt/qt4/qt4-embedded.inc
>> b/meta/recipes-qt/qt4/qt4-embedded.inc index 8c15352..158ee17 100644
>> --- a/meta/recipes-qt/qt4/qt4-embedded.inc
>> +++ b/meta/recipes-qt/qt4/qt4-embedded.inc
>> @@ -9,12 +9,18 @@ QT_BASE_LIB  ?= "libqt-embedded"
>>  # Set necessary variables in the profile
>>  SRC_URI += "file://qte.sh"
>>
>> +QT_DECORATION_FLAGS ?= ""
>> +QT_GFX_DRIVER_FLAGS ?= "-plugin-gfx-transformed -plugin-gfx-qvfb
>> -plugin-gfx-vnc -plugin-gfx-directfb" +QT_KBD_DRIVER_FLAGS ?= "-qt-kbd-tty"
>> +QT_MOUSE_DRIVER_FLAGS ?= "-plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb
>> -qt-mouse-linuxinput" +
>>  QT_CONFIG_FLAGS += " \
>>      -embedded ${QT_ARCH} \
>>      -qtlibinfix ${QT_LIBINFIX} \
>> -    -plugin-gfx-transformed -plugin-gfx-qvfb -plugin-gfx-vnc
>> -plugin-gfx-directfb \ -    -plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb
>> -qt-mouse-linuxinput \ -    -qt-kbd-tty \
>> +    ${QT_DECORATION_FLAGS} \
>> +    ${QT_GFX_DRIVER_FLAGS} \
>> +    ${QT_KBD_DRIVER_FLAGS} \
>> +    ${QT_MOUSE_DRIVER_FLAGS} \
>>      -DQT_KEYPAD_NAVIGATION \
>>      "
>>
>> diff --git a/meta/recipes-qt/qt4/qt4.inc b/meta/recipes-qt/qt4/qt4.inc
>> index 468a46f..c70b335 100644
>> --- a/meta/recipes-qt/qt4/qt4.inc
>> +++ b/meta/recipes-qt/qt4/qt4.inc
>> @@ -10,17 +10,28 @@ QT_ENDIAN = "${@qt_endian(d)}"
>>  QT_DISTRO_FLAGS ?= "-no-accessibility -no-sm"
>>  QT_DISTRO_FLAGS_linuxstdbase = "-sm"
>>
>> +QT_GLIB_FLAGS ?= "-glib"
>> +QT_IMAGEFORMAT_FLAGS ?= "-system-libjpeg -system-libpng -system-libtiff
>> -system-zlib" +QT_PHONON_FLAGS ?= "-phonon"
>> +QT_QDBUS_FLAGS ?= "-qdbus"
>> +QT_QT3SUPPORT_FLAGS ?= "-qt3support"
>>  QT_SQL_DRIVER_FLAGS ?= "-no-sql-ibase -no-sql-mysql -no-sql-psql
>> -no-sql-odbc -plugin-sql-sqlite" +QT_WEBKIT_FLAGS ?= "-webkit"
>>
>>  QT_GLFLAGS ?= ""
>>
>>  QT_CONFIG_FLAGS += "-release -no-cups -reduce-relocations \
>>                      -shared -no-nas-sound -no-nis \
>> -                    -system-libjpeg -system-libpng -system-libtiff
>> -system-zlib \ -                    -no-pch -qdbus -stl -glib -phonon
>> -webkit \
>> -                    -xmlpatterns -no-rpath -qt3support -silent \
>> +                    -no-pch -stl \
>> +                    -xmlpatterns -no-rpath -silent \
>>                      ${@base_contains('DISTRO_FEATURES', 'pulseaudio',
>> '--enable-pulseaudio', '--disable-pulseaudio', d)} \ +                   
>> ${QT_GLIB_FLAGS} \
>> +                    ${QT_IMAGEFORMAT_FLAGS} \
>> +                    ${QT_PHONON_FLAGS} \
>> +                    ${QT_QDBUS_FLAGS} \
>> +                    ${QT_QT3SUPPORT_FLAGS} \
>>                      ${QT_SQL_DRIVER_FLAGS} \
>> +                    ${QT_WEBKIT_FLAGS} \
>>                      ${QT_DISTRO_FLAGS} \
>>                      ${QT_GLFLAGS}"
> 
> I think when we start getting to this level, especially because some of these 
> options imply extra DEPENDS, we should try to use PACKAGECONFIG rather than 
> specific variables.

Introducing PACKAGECONFIG is a more complex change. It can still be done
in a later patch. This patch just follows the semantics introduced by
QT_SQL_DRIVER_FLAGS and doesn't even require a PR bump.

Regards,
Andreas
Paul Eggleton - May 22, 2012, 1:36 p.m.
On Tuesday 22 May 2012 15:28:17 you wrote:
> On 22.05.2012 14:51, Paul Eggleton wrote:
> > I think when we start getting to this level, especially because some of
> > these options imply extra DEPENDS, we should try to use PACKAGECONFIG
> > rather than specific variables.
> 
> Introducing PACKAGECONFIG is a more complex change. It can still be done
> in a later patch. 

It could be, but then we're introducing variables that will potentially go 
into people's distro configs only to take them away in the near future. I'm not 
especially keen on doing that.

> This patch just follows the semantics introduced by
> QT_SQL_DRIVER_FLAGS 

Right, and when that was introduced some time ago we did not have 
PACKAGECONFIG at all.

I realise this puts extra burden upon you, sorry about that. I can perhaps 
offer to do the PACKAGECONFIG changes for you, but I won't be able to get to 
them until next week at the earliest.

Cheers,
Paul
Andreas Oberritter - May 22, 2012, 2:10 p.m.
On 22.05.2012 15:36, Paul Eggleton wrote:
> On Tuesday 22 May 2012 15:28:17 you wrote:
>> On 22.05.2012 14:51, Paul Eggleton wrote:
>>> I think when we start getting to this level, especially because some of
>>> these options imply extra DEPENDS, we should try to use PACKAGECONFIG
>>> rather than specific variables.
>>
>> Introducing PACKAGECONFIG is a more complex change. It can still be done
>> in a later patch. 
> 
> It could be, but then we're introducing variables that will potentially go 
> into people's distro configs only to take them away in the near future. I'm not 
> especially keen on doing that.
> 
>> This patch just follows the semantics introduced by
>> QT_SQL_DRIVER_FLAGS 
> 
> Right, and when that was introduced some time ago we did not have 
> PACKAGECONFIG at all.
> 
> I realise this puts extra burden upon you, sorry about that. I can perhaps 
> offer to do the PACKAGECONFIG changes for you, but I won't be able to get to 
> them until next week at the earliest.

No need to hurry. I'll keep using my patch in my denzil-based branch
anyway, because denzil-next is unlikely to include either variant.

I'm not a big fan of PACKAGECONFIG. Its syntax is hard to read and hard
to write, maybe unless you're the inventor of it.

Looking for users of PACKAGECONFIG in OE-Core denzil, I saw it's used in
only 6 recipes. Even less in meta-openembedded (exactly 1). It looks
like it's not going to get adopted by many. So your statement about
taking away newly created variables in the near future is not
necessarily going to become true.

Besides that, introducing a new PACKAGECONFIG variable for Qt, that
includes new flags for basically everything already in QT_CONFIG_FLAGS,
doesn't seem to be an improvement.

Furthermore, as I understand it, PACKAGECONFIG handles only simple
on/off switches, but QT_CONFIG_FLAGS has switches for
on/off/plugin/system etc., and not everything you can build into qt can
be built as a plugin and vice versa, so the resulting set of
PACKAGECONFIG flags will likely become quite huge in order to be able to
express all options.

Regards,
Andreas
Paul Eggleton - May 22, 2012, 2:49 p.m.
On Tuesday 22 May 2012 16:10:52 Andreas Oberritter wrote:
> No need to hurry. I'll keep using my patch in my denzil-based branch
> anyway, because denzil-next is unlikely to include either variant.
> 
> I'm not a big fan of PACKAGECONFIG. Its syntax is hard to read and hard
> to write, maybe unless you're the inventor of it.

Can you suggest an alternative syntax that we could have consistent across 
multiple recipes and handles adding/removing DEPENDS as well as configure 
options? That is what it attempts to provide.
 
> Looking for users of PACKAGECONFIG in OE-Core denzil, I saw it's used in
> only 6 recipes. Even less in meta-openembedded (exactly 1). It looks
> like it's not going to get adopted by many. So your statement about
> taking away newly created variables in the near future is not
> necessarily going to become true.

It's still pretty new. Besides, we haven't exactly gone out on a program of 
adding it everywhere there is a configuration option for something - we only 
add it on an as-needed basis.

> Besides that, introducing a new PACKAGECONFIG variable for Qt, that
> includes new flags for basically everything already in QT_CONFIG_FLAGS,
> doesn't seem to be an improvement.

It is now our standard way of switching on and off features particularly where 
those features imply a change to DEPENDS.
 
> Furthermore, as I understand it, PACKAGECONFIG handles only simple
> on/off switches, but QT_CONFIG_FLAGS has switches for
> on/off/plugin/system etc., and not everything you can build into qt can
> be built as a plugin and vice versa, so the resulting set of
> PACKAGECONFIG flags will likely become quite huge in order to be able to
> express all options.

So this might present a problem I agree. Perhaps PACKAGECONFIG is not going to 
work here. I dislike however that we would have the ability to configure these 
options and yet there is no management of DEPENDS to match.

Cheers,
Paul

Patch

diff --git a/meta/recipes-qt/qt4/qt4-embedded.inc b/meta/recipes-qt/qt4/qt4-embedded.inc
index 8c15352..158ee17 100644
--- a/meta/recipes-qt/qt4/qt4-embedded.inc
+++ b/meta/recipes-qt/qt4/qt4-embedded.inc
@@ -9,12 +9,18 @@  QT_BASE_LIB  ?= "libqt-embedded"
 # Set necessary variables in the profile
 SRC_URI += "file://qte.sh"
 
+QT_DECORATION_FLAGS ?= ""
+QT_GFX_DRIVER_FLAGS ?= "-plugin-gfx-transformed -plugin-gfx-qvfb -plugin-gfx-vnc -plugin-gfx-directfb"
+QT_KBD_DRIVER_FLAGS ?= "-qt-kbd-tty"
+QT_MOUSE_DRIVER_FLAGS ?= "-plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb -qt-mouse-linuxinput"
+
 QT_CONFIG_FLAGS += " \
     -embedded ${QT_ARCH} \
     -qtlibinfix ${QT_LIBINFIX} \
-    -plugin-gfx-transformed -plugin-gfx-qvfb -plugin-gfx-vnc -plugin-gfx-directfb \
-    -plugin-mouse-tslib -qt-mouse-pc -qt-mouse-qvfb -qt-mouse-linuxinput \
-    -qt-kbd-tty \
+    ${QT_DECORATION_FLAGS} \
+    ${QT_GFX_DRIVER_FLAGS} \
+    ${QT_KBD_DRIVER_FLAGS} \
+    ${QT_MOUSE_DRIVER_FLAGS} \
     -DQT_KEYPAD_NAVIGATION \
     "
 
diff --git a/meta/recipes-qt/qt4/qt4.inc b/meta/recipes-qt/qt4/qt4.inc
index 468a46f..c70b335 100644
--- a/meta/recipes-qt/qt4/qt4.inc
+++ b/meta/recipes-qt/qt4/qt4.inc
@@ -10,17 +10,28 @@  QT_ENDIAN = "${@qt_endian(d)}"
 QT_DISTRO_FLAGS ?= "-no-accessibility -no-sm"
 QT_DISTRO_FLAGS_linuxstdbase = "-sm"
 
+QT_GLIB_FLAGS ?= "-glib"
+QT_IMAGEFORMAT_FLAGS ?= "-system-libjpeg -system-libpng -system-libtiff -system-zlib"
+QT_PHONON_FLAGS ?= "-phonon"
+QT_QDBUS_FLAGS ?= "-qdbus"
+QT_QT3SUPPORT_FLAGS ?= "-qt3support"
 QT_SQL_DRIVER_FLAGS ?= "-no-sql-ibase -no-sql-mysql -no-sql-psql -no-sql-odbc -plugin-sql-sqlite"
+QT_WEBKIT_FLAGS ?= "-webkit"
 
 QT_GLFLAGS ?= ""
 
 QT_CONFIG_FLAGS += "-release -no-cups -reduce-relocations \
                     -shared -no-nas-sound -no-nis \
-                    -system-libjpeg -system-libpng -system-libtiff -system-zlib \
-                    -no-pch -qdbus -stl -glib -phonon -webkit \
-                    -xmlpatterns -no-rpath -qt3support -silent \
+                    -no-pch -stl \
+                    -xmlpatterns -no-rpath -silent \
                     ${@base_contains('DISTRO_FEATURES', 'pulseaudio', '--enable-pulseaudio', '--disable-pulseaudio', d)} \
+                    ${QT_GLIB_FLAGS} \
+                    ${QT_IMAGEFORMAT_FLAGS} \
+                    ${QT_PHONON_FLAGS} \
+                    ${QT_QDBUS_FLAGS} \
+                    ${QT_QT3SUPPORT_FLAGS} \
                     ${QT_SQL_DRIVER_FLAGS} \
+                    ${QT_WEBKIT_FLAGS} \
                     ${QT_DISTRO_FLAGS} \
                     ${QT_GLFLAGS}"