[v5] package: add qt5virtualkeyboard

This patch adds the Qt virtualkeyboard package.

Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
Reviewed-by: Peter Seiderer <ps.report@gmx.net>
---
Changes since v4:
 - Prefix local variables to avoid potential naming collisions.

Changes since v3:
 - Fix another build issue when installing to target
   in case of zh_TW (or other layout) is not set.
 - Add licenses files when using 3rd-part modules.
 - Add support for handwriting module.

Changes since v2:
 - Fix build issue when installing libqtvirtualkeyboardplugin.so library
   in case of some other qt5 package installs something to
   plugins/platforminputcontexts.

Changes since v1:
 - Add missing hash file
 - Add license approval statement
 - Add support for languages layouts
   Note: Chinese, Japanese and Korean does not display properly (font?)
 - Install sample if is compiled (BR2_PACKAGE_QT5BASE_EXAMPLES=y)
 - Apply reviews from Peter:
   Add version constraint: message and dependency (needs at least qt 5.8)
   Update help to mention GPLv3 license

 package/qt5/Config.in                              |  1 +
 package/qt5/qt5virtualkeyboard/Config.in           | 52 +++++++++++++
 .../qt5/qt5virtualkeyboard/qt5virtualkeyboard.hash |  2 +
 .../qt5/qt5virtualkeyboard/qt5virtualkeyboard.mk   | 86 ++++++++++++++++++++++
 4 files changed, 141 insertions(+)
 create mode 100644 package/qt5/qt5virtualkeyboard/Config.in
 create mode 100644 package/qt5/qt5virtualkeyboard/qt5virtualkeyboard.hash
 create mode 100644 package/qt5/qt5virtualkeyboard/qt5virtualkeyboard.mk

Hello,

On Wed, 29 Mar 2017 11:58:22 -0400, Gaël PORTAY wrote:

> Changes since v4:
>  - Prefix local variables to avoid potential naming collisions.

You forgot one.


> +QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS = $(call qstrip,$(BR2_PACKAGE_QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +QT5VIRTUALKEYBOARD_ALL_LAYOUTS = $(findstring all,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +ifneq ($(strip $(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS)),)
> +QMAKEFLAGS += CONFIG+="$(foreach lang,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS),lang-$(lang))"

QMAKEFLAGS is still not properly prefixed.

Thomas

Hello,

On Wed, 29 Mar 2017 11:58:22 -0400, Gaël PORTAY wrote:
> +QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS = $(call qstrip,$(BR2_PACKAGE_QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +QT5VIRTUALKEYBOARD_ALL_LAYOUTS = $(findstring all,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +ifneq ($(strip $(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS)),)
> +QMAKEFLAGS += CONFIG+="$(foreach lang,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS),lang-$(lang))"
> +
> +QT5VIRTUALKEYBOARD_OPENWNN = $(findstring ja_JP,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +ifneq ($(strip $(QT5VIRTUALKEYBOARD_OPENWNN)$(QT5VIRTUALKEYBOARD_ALL_LAYOUTS)),)
> +QT5VIRTUALKEYBOARD_LICENSE += Apache-2.0

This way of concatenating the licenses doesn't work, because the final
string will be:

"GPLv3 Apache-2.0"

while we want "GPLv3, Apache-2.0". Look at other Qt5 packages how they
are handling this:

QT5BASE_LICENSE := $(QT5BASE_LICENSE), BSD-3c (examples)

Note that next to the license, we also want an information on which
part of the package it applies.

> +QT5VIRTUALKEYBOARD_PINYIN = $(findstring zh_CN,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> +ifneq ($(strip $(QT5VIRTUALKEYBOARD_PINYIN)$(QT5VIRTUALKEYBOARD_ALL_LAYOUTS)),)
> +QT5VIRTUALKEYBOARD_3RDPARTY_PARTS += pinyin

Do you really need the name here? You don't use it anywhere. What about:

QT5VIRTUALKEYBOARD_INSTALL_3RDPARTY = YES

instead.

Best regards,

Thomas

Hello Thomas,

On Wed, Mar 29, 2017 at 06:00:16PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 29 Mar 2017 11:58:22 -0400, Gaël PORTAY wrote:
> 
> > Changes since v4:
> >  - Prefix local variables to avoid potential naming collisions.
> 
> You forgot one.
> 
> 
> > +QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS = $(call qstrip,$(BR2_PACKAGE_QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> > +QT5VIRTUALKEYBOARD_ALL_LAYOUTS = $(findstring all,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> > +ifneq ($(strip $(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS)),)
> > +QMAKEFLAGS += CONFIG+="$(foreach lang,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS),lang-$(lang))"
> 
> QMAKEFLAGS is still not properly prefixed.
> 

Indeed, sorry for this missing one.

> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

Let me resend a new patch...

Regards,
Gael

Hi Thomas,

On Wed, Mar 29, 2017 at 06:03:11PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 29 Mar 2017 11:58:22 -0400, Gaël PORTAY wrote:
>
> [...]
> 
> This way of concatenating the licenses doesn't work, because the final
> string will be:
> 
> "GPLv3 Apache-2.0"
> 
> while we want "GPLv3, Apache-2.0". Look at other Qt5 packages how they
> are handling this:
> 
> QT5BASE_LICENSE := $(QT5BASE_LICENSE), BSD-3c (examples)
> 
> Note that next to the license, we also want an information on which
> part of the package it applies.
> 

I modified the Makefile so the LICENSE variable is set to (all layouts +
handwriting):

"GPLv3, Apache-2.0 (openwnn), Apache-2.0 (pinyin), Apache-2.0 BSD-3c (tcime), MIT (lipi-toolkit)"

Is it correct?

I can also be more explicit:

"GPLv3, Apache-2.0 (openwnn/ja_JP), Apache-2.0 (pinyin/zh_TW), Apache-2.0 BSD-3c (tcime/zh_TW), MIT (lipi-toolkit/handwriting)"

Thank you for your review. I did not understand why some packages have a comma
in LICENSE while others have 'and/or' or are simply white-space separated.

BTW, here are the modifications:

-QT5VIRTUALKEYBOARD_LICENSE += Apache-2.0
+QT5VIRTUALKEYBOARD_LICENSE := $(QT5VIRTUALKEYBOARD_LICENSE), Apache-2.0 (openwnn)

-QT5VIRTUALKEYBOARD_LICENSE += Apache-2.0
+QT5VIRTUALKEYBOARD_LICENSE := $(QT5VIRTUALKEYBOARD_LICENSE), Apache-2.0 (pinyin)

-QT5VIRTUALKEYBOARD_LICENSE += Apache-2.0 BSD-3c
+QT5VIRTUALKEYBOARD_LICENSE := $(QT5VIRTUALKEYBOARD_LICENSE), Apache-2.0 BSD-3c (tcime)

-QT5VIRTUALKEYBOARD_LICENSE += MIT
+QT5VIRTUALKEYBOARD_LICENSE := $(QT5VIRTUALKEYBOARD_LICENSE), MIT (lipi-toolkit)

> > +QT5VIRTUALKEYBOARD_PINYIN = $(findstring zh_CN,$(QT5VIRTUALKEYBOARD_LANGUAGE_LAYOUTS))
> > +ifneq ($(strip $(QT5VIRTUALKEYBOARD_PINYIN)$(QT5VIRTUALKEYBOARD_ALL_LAYOUTS)),)
> > +QT5VIRTUALKEYBOARD_3RDPARTY_PARTS += pinyin
> 
> Do you really need the name here? You don't use it anywhere. What about:
> 
> QT5VIRTUALKEYBOARD_INSTALL_3RDPARTY = YES
> 
> instead.

Indeed I did not need them.

I used the white-space separated values for debugging purpose.

The YES value make the Makefile more simple.

-QT5VIRTUALKEYBOARD_3RDPARTY_PARTS += pinyin
+QT5VIRTUALKEYBOARD_3RDPARTY_PARTS = YES

-QT5VIRTUALKEYBOARD_3RDPARTY_PARTS += tcime
+QT5VIRTUALKEYBOARD_3RDPARTY_PARTS = YES

-QT5VIRTUALKEYBOARD_3RDPARTY_PARTS += lipi_toolkit
+QT5VIRTUALKEYBOARD_3RDPARTY_PARTS = YES

> 
> Best regards,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

If you agree with those modifications, I will send a v6 of this patch (including
missing prefix for QMAKEFLAGS).

Regards,
Gael

Hello,

On Wed, 29 Mar 2017 13:29:02 -0400, Gaël PORTAY wrote:

> I modified the Makefile so the LICENSE variable is set to (all layouts +
> handwriting):
> 
> "GPLv3, Apache-2.0 (openwnn), Apache-2.0 (pinyin), Apache-2.0 BSD-3c (tcime), MIT (lipi-toolkit)"
> 
> Is it correct?

It looks good. Note that I'm saying it looks good from a formatting
point of view: I haven't checked the actual licenses.

> I can also be more explicit:
> 
> "GPLv3, Apache-2.0 (openwnn/ja_JP), Apache-2.0 (pinyin/zh_TW), Apache-2.0 BSD-3c (tcime/zh_TW), MIT (lipi-toolkit/handwriting)"

This is probably a bit too verbose, what you proposed above as the
first solution looks sufficient to me.

> Thank you for your review. I did not understand why some packages have a comma
> in LICENSE while others have 'and/or' or are simply white-space separated.

The Buildroot manual normally explains what the formatting should be,
but it is very possible that some packages do not comply 100% with the
rules described in the manual (simply because those packages were added
before the rules were clarified in the manual).

Basically, the idea is that: a comma separates multiple licenses that
cover different parts of the package, with parenthesis to indicate what
component is covered by that license. The "or" statement is used when
the package is available under a choice of licenses (i.e you can chose
to use say the GPLv2 terms *OR* the MIT terms for the whole package,
for example).

> If you agree with those modifications, I will send a v6 of this patch
> (including missing prefix for QMAKEFLAGS).

The modifications look good to me. Thanks a lot!

Thomas