[oe] [PATCH] qtserialport: Adds initial recipe.

[Paul Eggleton]

Hi Wayne,

Some of this feedback is a little bit pedantic I'm afraid, but it's probably 
best to get everything right.

On Friday 19 April 2013 08:09:43 Wayne Warren wrote:
> ---

Please include Signed-off-by.

>  meta-oe/recipes-qt/qtserialport/qtserialport.inc   |   34
> ++++++++++++++++++++ .../recipes-qt/qtserialport/qtserialport_5.1.0.bb  |  
>  6 ++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 meta-oe/recipes-qt/qtserialport/qtserialport.inc
>  create mode 100644 meta-oe/recipes-qt/qtserialport/qtserialport_5.1.0.bb
> 
> diff --git a/meta-oe/recipes-qt/qtserialport/qtserialport.inc
> b/meta-oe/recipes-qt/qtserialport/qtserialport.inc new file mode 100644
> index 0000000..09f2f9a
> --- /dev/null
> +++ b/meta-oe/recipes-qt/qtserialport/qtserialport.inc
> @@ -0,0 +1,34 @@
> +DESCRIPTION = "Qt Serial Port Library"

Would you mind using SUMMARY instead of DESCRIPTION here? (DESCRIPTION will be 
set from SUMMARY as well).

> +SECTION = "qt/libs"
> +HOMEPAGE = "http://qt-project.org/wiki/QtSerialPort"
> +
> +LICENSE = "LGPL FDL GPL "

I'm not 100% sure, but I think for this particular piece of software, this 
should actually be "(LGPL-2.1 | GPL-3.0) & GFDL-1.3"

> +LIC_FILES_CHKSUM = " \
> +    file://LICENSE.FDL;md5=3801d7932fdc07fd9efe89f9854a6caa \
> +    file://LICENSE.GPL;md5=d32239bcb673463ab874e80d47fae504 \
> +    file://LICENSE.LGPL;md5=4fbd65380cdd255951079008b364516c \
> +    "
> +
> +inherit qt4e pkgconfig

If this is to be built against Qt Embedded, the recipe should be named 
qtserialport-e; in fact I'd suggest replacing this with just "inherit 
pkgconfig" and then adding "inherit qt4e" to the .bb file and then it would be 
possible for someone else to create a qtserialport-x11 recipe that used the 
same inc file.

> +SRC_URI = " \
> +    git://gitorious.org/qt/qtserialport.git \
> +"
> +
> +S = "${WORKDIR}/git"
> +B = "${WORKDIR}/${BPN}-${PV}"
> +
> +QMAKE_PROFILES="${S}/qtserialport.pro"
> +EXTRA_QMAKEVARS_PRE += "PREFIX=/usr"

I think this should be ${prefix} rather than hardcoding /usr.

> +EXTRA_OEMAKE += "INSTALL_ROOT=${D}"
> +
> +do_compile_append() {
> + 	BOGUS_PREFIX=`qmake2 -query QT_INSTALL_PREFIX`
> +	for f in `find ${B} -name "Makefile"` ;do
> +		sed -i -e "s|\(\$(INSTALL_ROOT)\)${BOGUS_PREFIX}|\1${prefix}|g" ${f}
> +	done
> +}

It seems a bit extreme to me that this should be necessary; googling suggests 
that qt.conf is supposed to be used to override QT_INSTALL_PREFIX. My build is 
still running so I haven't been able to check if this can work yet or if 
there's an alternative method but I'll let you know.

> +
> +do_install() {
> +    oe_runmake install DESTDIR=${D} INCLUDEDIR=${includedir}
> +}
> diff --git a/meta-oe/recipes-qt/qtserialport/qtserialport_5.1.0.bb
> b/meta-oe/recipes-qt/qtserialport/qtserialport_5.1.0.bb new file mode
> 100644
> index 0000000..d903eb3
> --- /dev/null
> +++ b/meta-oe/recipes-qt/qtserialport/qtserialport_5.1.0.bb
> @@ -0,0 +1,6 @@
> +require qtserialport.inc
> +
> +PR = "r0"

This doesn't need to be set this since it's the default value (and we're no 
longer incrementing this value manually).

> +
> +SRCREV="22e45f0b0f956c025339d72ad53dca57ef684858"
> +

This is really pedantic, but we usually prefer spaces around the assignment 
(as you have elsewhere.)

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre