[Buildroot] [PATCH] Add packages sip and PyQt
sergey kostanbaev
sergey.kostanbaev at gmail.com
Fri Oct 18 06:26:58 UTC 2013
Hi Arnout,
Thank you for your detailed comments and fast review.
On Fri, Oct 18, 2013 at 3:58 AM, Arnout Vandecappelle <arnout at mind.be> wrote:
> Hi Sergey,
>
> Thank you for your contribution! I have quite a few comments on your patch,
> though, so hopefully you can modify it and resend.
>
>
> On 17/10/13 13:27, Sergey Kostanbaev wrote:
>>
>> Add support for sip-4.15.3 and pyqt-4.9.6 to work with qt-4.8.x. Tested on
>> arm platform.
>
>
> Two packages should come in two separate patches: the first one adding sip,
> the second one adding pyqt. Such patches normally have a subject like:
>
> sip: new package
>
> Also, we prefix python-related packages with 'python-'. So the packages
> should be called python-sip and python-pyqt.
Ok
>
>
>>
>> Signed-off-by: Sergey Kostanbaev <sergey.kostanbaev at gmail.com>
>> ---
>> Config.in | 5
>> pyqt/Config.in | 6
>> pyqt/pyqt-4.9.6-configure.patch | 924
>> ++++++++++++++++++++++++++++++++++++
>> pyqt/pyqt-4.9.6-full.patch | 949
>> ++++++++++++++++++++++++++++++++++++++
>
>
> Patches shouldn't have a version number, but should be numbered. So
>
> python-pyqt-0001-configure.patch
> python-pyqt-0002-full.patch
Ok
>
>
>> pyqt/pyqt.mk | 63 ++
>> sip/Config.in | 5
>> sip/sip-4.15.3-configure.py.patch | 38 +
>> sip/sip.mk | 71 ++
>> 8 files changed, 2061 insertions(+)
>>
>> diff --git a/package/Config.in b/package/Config.in
>> index a94cb62..9c9ee19 100644
>> --- a/package/Config.in
>> +++ b/package/Config.in
>> @@ -157,6 +157,11 @@ source "package/qextserialport/Config.in"
>> source "package/qjson/Config.in"
>> source "package/qtuio/Config.in"
>> source "package/qwt/Config.in"
>> +
>> +comment "PyQT libraries"
>
>
> I don't think this comment is needed.
>
>
>> +source "package/sip/Config.in"
>> +source "package/pyqt/Config.in"
>> +
>> endif
>>
>> source "package/qt5/Config.in"
>> diff --git a/package/sip/Config.in b/package/sip/Config.in
>> new file mode 100644
>> index 0000000..bbdb943
>> --- /dev/null
>> +++ b/package/sip/Config.in
>> @@ -0,0 +1,5 @@
>> +config BR2_PACKAGE_SIP
>> + bool "sip"
>> + depends on BR2_PACKAGE_PYTHON
>> + help
>> + SIP for PyQt4
>> \ No newline at end of file
>
>
> I don't think we really need sip on the target. sip is needed to build
> pyqt, but we're not going to build it on the target. So you also don't need
> a Config.in for it then.
Actually the target uses it. I just tried:
# ls /usr/lib/python2.7/site-packages/PyQt4/
Qt.so QtMultimedia.so QtTest.so __init__.pyc
QtCore.so QtNetwork.so QtXml.so uic
QtGui.so QtSql.so __init__.py
# ls /usr/lib/python2.7/site-packages/
PyQt4 README sip.so
# mv /usr/lib/python2.7/site-packages/sip.so __sip__so__
# cat /root/test.py
#!/usr/bin/python
import sys
from PyQt4 import QtGui
app = QtGui.QApplication(sys.argv)
widget = QtGui.QWidget()
widget.resize(640, 400)
widget.setWindowTitle('Simple')
widget.show()
sys.exit(app.exec_())
# /root/test.py
Traceback (most recent call last):
File "/root/test.py", line 5, in <module>
from PyQt4 import QtGui
ImportError: No module named sip
so it uses it somehow internally. I haven't dig deep enough why it's
used on target, though it has small size (~84K on arm) compared to the
just PyQt4.QtGui library, so I left it
>
>
>> diff --git a/package/sip/sip-4.15.3-configure.py.patch
>> b/package/sip/sip-4.15.3-configure.py.patch
>> new file mode 100644
>> index 0000000..06591c0
>> --- /dev/null
>> +++ b/package/sip/sip-4.15.3-configure.py.patch
>
>
> Patches should have a description, similar to a commit message. They should
> also have a Signed-off-by tag. See
> http://buildroot.net/downloads/manual/manual.html#_format_and_licensing_of_the_package_patches
>
> However, this patch is probably not needed if cross-compilation is not
> needed.
Ok, but see above.
>
>
>> @@ -0,0 +1,38 @@
>> +--- a/configure.py 2013-04-24 12:41:35.382000017 +0400
>> ++++ b/configure.py 2013-04-24 13:09:30.000000000 +0400
>> +@@ -273,9 +273,9 @@
>> + "default_mod_dir": plat_py_site_dir,
>> + "default_sip_dir": opts.sipsipdir,
>> + "py_version": py_version,
>> +- "py_inc_dir": plat_py_inc_dir,
>> +- "py_conf_inc_dir": plat_py_conf_inc_dir,
>> +- "py_lib_dir": plat_py_lib_dir,
>> ++ "py_inc_dir": opts.py_inc_dir,
>> ++ "py_conf_inc_dir": opts.py_conf_inc_dir,
>> ++ "py_lib_dir": opts.py_lib_dir,
>> + "universal": opts.universal,
>> + "arch": opts.arch,
>> + "deployment_target": opts.deployment_target
>> +@@ -399,6 +399,22 @@
>> + "macros")
>> + p.add_option_group(g)
>> +
>> ++ # Python configuration
>> ++ g = optparse.OptionGroup(p, title="Python include path")
>> ++ g.add_option("-i", "--py_inc_dir", action="callback",
>> ++ default=plat_py_inc_dir, type="string", metavar="DIR",
>> ++ dest="py_inc_dir", callback=store_abspath, help="where the
>> Python "
>> ++ "include directory located [default: %s]" % plat_py_inc_dir)
>> ++ g.add_option("-c", "--py_conf_inc_dir", action="callback",
>> ++ default=plat_py_conf_inc_dir, type="string", metavar="DIR",
>> ++ dest="py_conf_inc_dir", callback=store_abspath, help="where
>> the Python "
>> ++ "Configuration include directory located [default: %s]" %
>> plat_py_conf_inc_dir)
>> ++ g.add_option("-l", "--py_lib_dir", action="callback",
>> ++ default=plat_py_conf_inc_dir, type="string", metavar="DIR",
>> ++ dest="py_lib_dir", callback=store_abspath, help="where the
>> Python "
>> ++ "library located [default: %s]" % plat_py_lib_dir)
>> ++
>> ++
>> + # Installation.
>> + g = optparse.OptionGroup(p, title="Installation")
>> + g.add_option("-b", "--bindir", action="callback",
>> diff --git a/package/sip/sip.mk b/package/sip/sip.mk
>> new file mode 100644
>> index 0000000..2a4254c
>> --- /dev/null
>> +++ b/package/sip/sip.mk
>> @@ -0,0 +1,71 @@
>> +#############################################################
>> +#
>> +# sip
>> +#
>> +#############################################################
>
>
> That should be 80 #'s
>
>
>> +SIP_VERSION = 4.15.3
>> +SIP_SOURCE = sip-$(SIP_VERSION).tar.gz
>> +SIP_SITE =
>> http://sourceforge.net/projects/pyqt/files/sip/sip-$(SIP_VERSION)
>
>
> We use the direct download links on sourceforge:
>
> http://downloads.sourceforge.net/project/pyqt/sip/sip-$(SIP_VERSION)
>
>> +
>> +################################################
>> +# HOST
>
>
> This is not needed.
>
>
>> +
>> +HOST_SIP_DEPENDENCIES = host-python
>> +define HOST_SIP_CONFIGURE_CMDS
>> + (cd $(@D); \
>> + echo "Host Configuring DIR=$(PWD)"; \
>
>
> This echo is not needed.
>
>> + LD_LIBRARY_PATH=$(HOST_DIR)/lib $(HOST_DIR)/usr/bin/python
>> configure.py; \
>
>
> You should also pass HOST_CONFIGURE_OPTS in the environment. And this
> already contains (a better) LD_LIBRARY_PATH definition.
Ok
>
>> + )
>> +endef
>> +
>
> [snip target definitions which are not needed IMO]
>>
>> +$(eval $(host-autotools-package))
>
>
> This is not an autotools package, so you shouldn't use the autotools rules.
> That does some additional things that may not apply here.
>
>
>> diff --git a/package/pyqt/Config.in b/package/pyqt/Config.in
>> new file mode 100644
>> index 0000000..4e4ff07
>> --- /dev/null
>> +++ b/package/pyqt/Config.in
>> @@ -0,0 +1,6 @@
>> +config BR2_PACKAGE_PYQT
>> + bool "pyqt"
>> + depends on BR2_PACKAGE_QT
>
>
> It's already in an if BR2_PACKAGE_QT, so this is not needed.
>
>> + depends on BR2_PACKAGE_SIP
>
>
> This is not needed IMO. It requires the host sip, but not the target sip.
see above
>
>
>> + help
>> + PyQT4 for Qt Embedded 4
>
>
> There should be an upstream URL, i.e.
> http://www.riverbankcomputing.com/software/pyqt/
>
> Also, help text indentation should be one tab + two spaces, not four
> spaces.
>
>
>> \ No newline at end of file
>
>
> There should be a newline at the end of the file.
>
> [snip 2 patches]
>
> These patches are suspiciously large. They lack a description, but I guess
> they are needed for cross-compilation. Upstream says that configure-ng will
> support cross-compilation in the future [1] so I think you should probably
> start with that script. And check in upstream's mercurial repository what is
> already possible. Or perhaps even ask upstream for help.
>
> [1] http://pyqt.sourceforge.net/Docs/PyQt4/installation.html
Yes that was needed for cross-compilation and mostly for Embedded Qt
(aka QWS) . I'll check with upstream.
>
>
> If you do need such large patches, try to split them up a little more and
> certainly give a good comment, so we can try to review them. Also, if
> possible, try to send them upstream. Upstream can give much more valuable
> comments on the patches.
>
Ok
>
>
>> diff --git a/package/pyqt/pyqt.mk b/package/pyqt/pyqt.mk
>> new file mode 100644
>> index 0000000..6c3ccfb
>> --- /dev/null
>> +++ b/package/pyqt/pyqt.mk
>> @@ -0,0 +1,63 @@
>> +#############################################################
>
>
> 80 #'s
>
>
>> +#
>> +# PyQT Open Source Edition for Qt Embedded (QWS)
>
>
> Just
>
> # python-pyqt
>
Ok
>
>> +#
>> +#############################################################
>> +PYQT_VERSION = 4.9.6
>> +PYQT_SOURCE = PyQt-x11-gpl-$(PYQT_VERSION).tar.gz
>> +PYQT_SITE =
>> http://sourceforge.net/projects/pyqt/files/PyQt4/PyQt-$(PYQT_VERSION)
>
>
> Use downloads.sourceforge.net
>
>
>> +
>> +################################################
>> +# TARGET
>> +
>> +PYQT_DEPENDENCIES = sip qt
>
>
> host-sip
see above
>
>
>> +
>> +define PYQT_CONFIGURE_CMDS
>> + echo "Construting qtdirs"
>
>
> echo is not needed.
Yes that echos was my debug outputs. I forget to clean it up.
>
>
>> + echo $(STAGING_DIR)/usr > $(@D)/qtdirs.out
>> + echo $(STAGING_DIR)/usr/include >> $(@D)/qtdirs.out
>> + echo $(STAGING_DIR)/usr/lib >> $(@D)/qtdirs.out
>> + echo $(HOST_DIR)/usr/bin >> $(@D)/qtdirs.out
>> + echo $(STAGING_DIR)/usr >> $(@D)/qtdirs.out
>> + echo $(STAGING_DIR)/usr/plugins >> $(@D)/qtdirs.out
>> +
>> + echo 264196 >> $(@D)/qtdirs.out
>> + echo 8 >> $(@D)/qtdirs.out
>> + echo Open Source >> $(@D)/qtdirs.out
>> + echo shared >> $(@D)/qtdirs.out
>> + echo PyQt_Accessibility >> $(@D)/qtdirs.out
>> + echo PyQt_SessionManager >> $(@D)/qtdirs.out
>> + echo PyQt_qreal_double >> $(@D)/qtdirs.out
>> + echo PyQt_OpenSSL >> $(@D)/qtdirs.out
>> + echo PyQt_Shortcut >> $(@D)/qtdirs.out
>> + echo PyQt_ButtonGroup >> $(@D)/qtdirs.out
>> + echo PyQt_RawFont >> $(@D)/qtdirs.out
>> + echo WS_MACX >> $(@D)/qtdirs.out
>> + echo WS_WIN >> $(@D)/qtdirs.out
>
>
> This really needs some comments to explain what it does.
Ok, I'll add comments. That file qtdirs.out originally generates
running and compiling C++ programs on target (AFAIK), so we can't do
it. So I add generation of it here with the specific for Qt Embedded
(QWS). Also I found a python script that analysis directories and
generates it, but it was quite huge and it was far away from upstream,
so I did simple generation.
>
> Also, it's nicer to keep a template in package/python-pyqt and sed-ing it
> to the build directory.
>
>
>> +
>> + ( cd $(@D); \
>> + echo "TARGET_DIR=$(TARGET_DIR)" \
>> + echo "HOST_DIR=$(HOST_DIR)" \
>> + echo "BUILD_DIR=$(BUILD_DIR)" \
>> + echo "PyQT Configuring Target DIR=`pwd`"; \
>
>
> That's a pretty weird echo... And anyway not needed.
Yes. That was debug.
>
>> + PATH=$$PATH:$(HOST_DIR)/usr/bin \
>
>
> Use TARGET_CONFIGURE_OPTS.
>
>> + CROSS_SIPCONFIG=$(BUILD_DIR)/sip-$(SIP_VERSION)/ \
>
>
> Hm, we don't like referring to another build dir. Is it possible to install
> whatever is needed somewhere in $(HOST_DIR) and refer to that?
I'll try
>
>
>> +
>> QMAKESPEC=$(BUILD_DIR)/qt-$(QT_VERSION)/mkspecs/qws/linux-$(QT_EMB_PLATFORM)-g++
>> \
>> + LD_LIBRARY_PATH=$(HOST_DIR)/lib $(HOST_DIR)/usr/bin/python
>> configure.py \
>
>
> Is LD_LIBRARY_PATH really needed?
Will try without it
>
>
>> + -b $(TARGET_DIR)/usr/bin \
>> + -d
>> $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages \
>> + -l
>> $(STAGING_DIR)/usr/include/python$(PYTHON_VERSION_MAJOR) \
>> + -m
>> $(STAGING_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/config \
>> + -q $(HOST_DIR)/usr/bin/qmake \
>> + -v $(TARGET_DIR)/usr/share/sip/PyQt4 \
>> + -w --confirm-license \
>> + )
>> +endef
>> +
>> +define PYQT_INSTALL_TARGET_CMDS
>> +# TODO copy only needed files DESTDIR=
>
>
> Don't leave comments from your drafts lingering around. If there are todos,
> mention them in your commit message.
Ok
>
>
>> + PATH="$(PATH):$(HOST_DIR)/usr/bin" $(MAKE1) install -C $(@D)
>
>
> Use the same environment as above.
>
>> + touch
>> $(TARGET_DIR)/usr/lib/python$(PYTHON_VERSION_MAJOR)/site-packages/PyQt4/__init__.py
>
>
> Why is this needed?
Without this dummy file I get
# /root/test.py
Traceback (most recent call last):
File "/root/test.py", line 5, in <module>
from PyQt4 import QtGui
ImportError: No module named PyQt4
May be there's a solution how to avoid it but haven't figured it out
>
>> +endef
>> +
>> +$(eval $(autotools-package))
>
>
> Again, it's not really an autotools package.
>
>
> Regards,
> Arnout
>
>> --
>> 1.8.3.2
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>>
>>
>
>
> --
> Arnout Vandecappelle arnout at mind be
> Senior Embedded Software Architect +32-16-286500
> Essensium/Mind http://www.mind.be
> G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
> LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
> GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
More information about the buildroot
mailing list