| Message ID | 1455962149-29459-1-git-send-email-angelo.compagnucci@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Angelo, On Sat, 20 Feb 2016 10:55:49 +0100, Angelo Compagnucci wrote: > This patch adds python-pillow, the friendly python image library fork. > > Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> This package does not build when jpeg is disabled: running build_ext Traceback (most recent call last): File "setup.py", line 711, in <module> zip_safe=not debug_build(), File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/core.py", line 148, in setup dist.run_commands() File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 955, in run_commands self.run_command(cmd) File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 974, in run_command cmd_obj.run() File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/command/build.py", line 126, in run self.run_command(cmd_name) File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/cmd.py", line 313, in run_command self.distribution.run_command(command) File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 974, in run_command cmd_obj.run() File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/command/build_ext.py", line 342, in run self.build_extensions() File "setup.py", line 459, in build_extensions % (f, f)) ValueError: --enable-jpeg requested but jpeg not found, aborting. It seems like you should pass explicit --enable/--disable options, since at least jpeg and zlib are by default expected to be enabled. See https://pillow.readthedocs.org/en/latest/installation.html#external-libraries. Unfortunately, those options are only recognized when calling "setup.py build_ext", not when calling "setup.py build" as the python-package infrastructure is doing. So you might need to override the build commands for this package. > diff --git a/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch > new file mode 100644 > index 0000000..5f43013 > --- /dev/null > +++ b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch > @@ -0,0 +1,85 @@ > +From cb8c67c0b7ee805100c381300ea29262e8b5838a Mon Sep 17 00:00:00 2001 > +From: Angelo Compagnucci <angelo.compagnucci@gmail.com> > +Date: Thu, 22 Oct 2015 22:45:31 +0200 > +Subject: [PATCH] setup.py: removing unneeded platform guess > + > +Platform guess is not needed when cross compiling on buildroot > +so removing it. > + > +Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> Can you submit this patch upstream or at least report the issue? Thanks! Thomas
Dear Thomas Petazzoni, 2016-02-21 15:26 GMT+01:00 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>: > Angelo, > > On Sat, 20 Feb 2016 10:55:49 +0100, Angelo Compagnucci wrote: >> This patch adds python-pillow, the friendly python image library fork. >> >> Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> > > This package does not build when jpeg is disabled: > > running build_ext > Traceback (most recent call last): > File "setup.py", line 711, in <module> > zip_safe=not debug_build(), > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/core.py", line 148, in setup > dist.run_commands() > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 955, in run_commands > self.run_command(cmd) > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 974, in run_command > cmd_obj.run() > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/command/build.py", line 126, in run > self.run_command(cmd_name) > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/cmd.py", line 313, in run_command > self.distribution.run_command(command) > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/dist.py", line 974, in run_command > cmd_obj.run() > File "/home/thomas/projets/buildroot/output/host/usr/lib/python3.4/distutils/command/build_ext.py", line 342, in run > self.build_extensions() > File "setup.py", line 459, in build_extensions > % (f, f)) > ValueError: --enable-jpeg requested but jpeg not found, aborting. > > It seems like you should pass explicit --enable/--disable options, > since at least jpeg and zlib are by default expected to be enabled. > See https://pillow.readthedocs.org/en/latest/installation.html#external-libraries. > > Unfortunately, those options are only recognized when calling "setup.py > build_ext", not when calling "setup.py build" as the python-package > infrastructure is doing. So you might need to override the build > commands for this package. Right. I actually implemented something lie this in pkg-python.mk [...] $(2)_BUILD_TARGET ?= build [...] $(2)_BASE_BUILD_TGT = $$($(2)_BUILD_TARGET) [...] and seems to work and not breaking other packages. This way I can add: PYTHON_PILLOW_BUILD_TARGET = build_ext What do you think? >> diff --git a/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch >> new file mode 100644 >> index 0000000..5f43013 >> --- /dev/null >> +++ b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch >> @@ -0,0 +1,85 @@ >> +From cb8c67c0b7ee805100c381300ea29262e8b5838a Mon Sep 17 00:00:00 2001 >> +From: Angelo Compagnucci <angelo.compagnucci@gmail.com> >> +Date: Thu, 22 Oct 2015 22:45:31 +0200 >> +Subject: [PATCH] setup.py: removing unneeded platform guess >> + >> +Platform guess is not needed when cross compiling on buildroot >> +so removing it. >> + >> +Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> > > Can you submit this patch upstream or at least report the issue? I don't think so. That piece of code is used to guess the platform on which your are compiling with the assumption that host arch == target arch. Probably, pillow is not originally designed to be cross compiled and the build system would require a rewrite for a more structured approach. The only way to let buildroot passing the appropriate options to build and setup is to remove that piece of code. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Hello, On Tue, 23 Feb 2016 23:18:47 +0100, Angelo Compagnucci wrote: > Right. > > I actually implemented something lie this in pkg-python.mk > > [...] > $(2)_BUILD_TARGET ?= build > [...] > $(2)_BASE_BUILD_TGT = $$($(2)_BUILD_TARGET) > [...] > > and seems to work and not breaking other packages. > This way I can add: > > PYTHON_PILLOW_BUILD_TARGET = build_ext > > What do you think? Since it's the first package to require this, I would rather suggest to override PYTHON_PILLOW_BUILD_CMDS in python-pillow.mk, so that this hack is limited to this package. Should more packages need this in the future, we can add better support in the infrastructure. But a Python package that needs to pass options only to build_ext is IMO broken, and should allow them to be passed to the "build" target. build_ext is more or less an internal step, which we shouldn't have to worry about. > >> +Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> > > > > Can you submit this patch upstream or at least report the issue? > > I don't think so. That piece of code is used to guess the platform on > which your are compiling with the assumption that host arch == target > arch. Probably, pillow is not originally designed to be cross compiled > and the build system would require a rewrite for a more structured > approach. > The only way to let buildroot passing the appropriate options to build > and setup is to remove that piece of code. Well, you can make it upstreamable by making it understand some environment variable, or better some option, to indicate that we are cross-compiling. So yes, your patch is not acceptable upstream as is, but my comment is precisely that is should ideally be in a form that can potentially be accepted upstream. Thanks! Thomas
On 02/23/16 23:18, Angelo Compagnucci wrote: > Dear Thomas Petazzoni, > > 2016-02-21 15:26 GMT+01:00 Thomas Petazzoni > <thomas.petazzoni@free-electrons.com>: [snip] >> It seems like you should pass explicit --enable/--disable options, >> since at least jpeg and zlib are by default expected to be enabled. >> See https://pillow.readthedocs.org/en/latest/installation.html#external-libraries. >> >> Unfortunately, those options are only recognized when calling "setup.py >> build_ext", not when calling "setup.py build" as the python-package >> infrastructure is doing. So you might need to override the build >> commands for this package. > > Right. > > I actually implemented something lie this in pkg-python.mk > > [...] > $(2)_BUILD_TARGET ?= build > [...] > $(2)_BASE_BUILD_TGT = $$($(2)_BUILD_TARGET) > [...] > > and seems to work and not breaking other packages. > This way I can add: > > PYTHON_PILLOW_BUILD_TARGET = build_ext > > What do you think? Looks like a good idea to me - parallel with autotools-package. Regards, Arnout [snip]
On 02/23/16 23:35, Thomas Petazzoni wrote: > On Tue, 23 Feb 2016 23:18:47 +0100, Angelo Compagnucci wrote: > >> > Right. >> > >> > I actually implemented something lie this in pkg-python.mk >> > >> > [...] >> > $(2)_BUILD_TARGET ?= build >> > [...] >> > $(2)_BASE_BUILD_TGT = $$($(2)_BUILD_TARGET) >> > [...] >> > >> > and seems to work and not breaking other packages. >> > This way I can add: >> > >> > PYTHON_PILLOW_BUILD_TARGET = build_ext >> > >> > What do you think? > Since it's the first package to require this, I would rather suggest to > override PYTHON_PILLOW_BUILD_CMDS in python-pillow.mk, so that this > hack is limited to this package. > > Should more packages need this in the future, we can add better > support in the infrastructure. Hm, good point as well. Especially because the build commands are just a few lines. I notice now that in pkg-python we define _BASE_BUILD_TGT in four different places, and it's always 'build'. Probably can be removed... Regards, Arnout
Dear Thomas Petazzoni, 2016-02-23 23:35 GMT+01:00 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>: > Hello, > > On Tue, 23 Feb 2016 23:18:47 +0100, Angelo Compagnucci wrote: > >> Right. >> >> I actually implemented something lie this in pkg-python.mk >> >> [...] >> $(2)_BUILD_TARGET ?= build >> [...] >> $(2)_BASE_BUILD_TGT = $$($(2)_BUILD_TARGET) >> [...] >> >> and seems to work and not breaking other packages. >> This way I can add: >> >> PYTHON_PILLOW_BUILD_TARGET = build_ext >> >> What do you think? > > Since it's the first package to require this, I would rather suggest to > override PYTHON_PILLOW_BUILD_CMDS in python-pillow.mk, so that this > hack is limited to this package. > > Should more packages need this in the future, we can add better > support in the infrastructure. I just sent a patch in which I implemented custom BUILD_CMDS and INSTALL_TARGET_CMDS. > But a Python package that needs to pass options only to build_ext is > IMO broken, and should allow them to be passed to the "build" target. > build_ext is more or less an internal step, which we shouldn't have to > worry about. > > >> >> +Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> >> > >> > Can you submit this patch upstream or at least report the issue? >> >> I don't think so. That piece of code is used to guess the platform on >> which your are compiling with the assumption that host arch == target >> arch. Probably, pillow is not originally designed to be cross compiled >> and the build system would require a rewrite for a more structured >> approach. >> The only way to let buildroot passing the appropriate options to build >> and setup is to remove that piece of code. > > Well, you can make it upstreamable by making it understand some > environment variable, or better some option, to indicate that we are > cross-compiling. > > So yes, your patch is not acceptable upstream as is, but my comment is > precisely that is should ideally be in a form that can potentially be > accepted upstream. IMHO not having pillow in Buildroot in really a pity. I used it so many times in various projects that I was shocked when I realized it was lacking. I would invest some time to try to understand better the build system and try to submit an upstream patch for a fully cross compilation, in the meantime, could you accept this patch? It now looks good to me! > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
diff --git a/package/Config.in b/package/Config.in index 3eb3618..55f16cc 100644 --- a/package/Config.in +++ b/package/Config.in @@ -674,6 +674,7 @@ menu "External python modules" source "package/python-paho-mqtt/Config.in" source "package/python-pam/Config.in" source "package/python-paramiko/Config.in" + source "package/python-pillow/Config.in" source "package/python-posix-ipc/Config.in" source "package/python-protobuf/Config.in" source "package/python-psutil/Config.in" diff --git a/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch new file mode 100644 index 0000000..5f43013 --- /dev/null +++ b/package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch @@ -0,0 +1,85 @@ +From cb8c67c0b7ee805100c381300ea29262e8b5838a Mon Sep 17 00:00:00 2001 +From: Angelo Compagnucci <angelo.compagnucci@gmail.com> +Date: Thu, 22 Oct 2015 22:45:31 +0200 +Subject: [PATCH] setup.py: removing unneeded platform guess + +Platform guess is not needed when cross compiling on buildroot +so removing it. + +Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> +--- + setup.py | 58 +--------------------------------------------------------- + 1 file changed, 1 insertion(+), 57 deletions(-) + +diff --git a/setup.py b/setup.py +index 4cb7257..038b7dc 100644 +--- a/setup.py ++++ b/setup.py +@@ -241,63 +241,7 @@ class pil_build_ext(build_ext): + _add_directory(include_dirs, "/usr/X11/include") + + elif sys.platform.startswith("linux"): +- arch_tp = (plat.processor(), plat.architecture()[0]) +- if arch_tp == ("x86_64", "32bit"): +- # 32 bit build on 64 bit machine. +- _add_directory(library_dirs, "/usr/lib/i386-linux-gnu") +- else: +- for platform_ in arch_tp: +- +- if not platform_: +- continue +- +- if platform_ in ["x86_64", "64bit"]: +- _add_directory(library_dirs, "/lib64") +- _add_directory(library_dirs, "/usr/lib64") +- _add_directory( +- library_dirs, "/usr/lib/x86_64-linux-gnu") +- break +- elif platform_ in ["i386", "i686", "32bit"]: +- _add_directory( +- library_dirs, "/usr/lib/i386-linux-gnu") +- break +- elif platform_ in ["aarch64"]: +- _add_directory(library_dirs, "/usr/lib64") +- _add_directory( +- library_dirs, "/usr/lib/aarch64-linux-gnu") +- break +- elif platform_ in ["arm", "armv7l"]: +- _add_directory( +- library_dirs, "/usr/lib/arm-linux-gnueabi") +- break +- elif platform_ in ["ppc64"]: +- _add_directory(library_dirs, "/usr/lib64") +- _add_directory( +- library_dirs, "/usr/lib/ppc64-linux-gnu") +- _add_directory( +- library_dirs, "/usr/lib/powerpc64-linux-gnu") +- break +- elif platform_ in ["ppc"]: +- _add_directory(library_dirs, "/usr/lib/ppc-linux-gnu") +- _add_directory( +- library_dirs, "/usr/lib/powerpc-linux-gnu") +- break +- elif platform_ in ["s390x"]: +- _add_directory(library_dirs, "/usr/lib64") +- _add_directory( +- library_dirs, "/usr/lib/s390x-linux-gnu") +- break +- elif platform_ in ["s390"]: +- _add_directory(library_dirs, "/usr/lib/s390-linux-gnu") +- break +- else: +- raise ValueError( +- "Unable to identify Linux platform: `%s`" % platform_) +- +- # XXX Kludge. Above /\ we brute force support multiarch. Here we +- # try Barry's more general approach. Afterward, something should +- # work ;-) +- self.add_multiarch_paths() ++ pass + + elif sys.platform.startswith("gnu"): + self.add_multiarch_paths() +-- +1.9.1 + diff --git a/package/python-pillow/Config.in b/package/python-pillow/Config.in new file mode 100644 index 0000000..d5c3809 --- /dev/null +++ b/package/python-pillow/Config.in @@ -0,0 +1,12 @@ +config BR2_PACKAGE_PYTHON_PILLOW + bool "python-pillow" + help + Pillow is the "friendly" PIL fork by Alex Clark and Contributors. PIL is + the Python Imaging Library by Fredrik Lundh and Contributors. + + Pillow relies on external libraries to provide support various + image formats. Select the corresponding package(s) to get this + support. Pillow can use jpeg, zlib (for PNG), tiff, freetype, webp, + and openjpeg (JPEG-2000). + + https://pypi.python.org/pypi/Pillow/ diff --git a/package/python-pillow/python-pillow.hash b/package/python-pillow/python-pillow.hash new file mode 100644 index 0000000..fd2978a --- /dev/null +++ b/package/python-pillow/python-pillow.hash @@ -0,0 +1,2 @@ +# sha256 locally computed +sha256 780f21465e2b7690fc55925188373cd54668ea4d71964f971e1fea4bc16d365e python-pillow-3.0.0.tar.gz diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk new file mode 100644 index 0000000..a956a2e --- /dev/null +++ b/package/python-pillow/python-pillow.mk @@ -0,0 +1,17 @@ +################################################################################ +# +# python-pillow +# +################################################################################ + +PYTHON_PILLOW_VERSION = 3.0.0 +PYTHON_PILLOW_SITE = $(call github,python-pillow,Pillow,$(PYTHON_PILLOW_VERSION)) +PYTHON_PILLOW_SETUP_TYPE = setuptools +PYTHON_PILLOW_DEPENDENCIES = $(if $(BR2_PACKAGE_JPEG),jpeg) \ + $(if $(BR2_PACKAGE_ZLIB),zlib) \ + $(if $(BR2_PACKAGE_TIFF),tiff) \ + $(if $(BR2_PACKAGE_FREETYPE),freetype) \ + $(if $(BR2_PACKAGE_WEBP),webp) \ + $(if $(BR2_PACKAGE_OPENJPEG),openjpeg) + +$(eval $(python-package))
This patch adds python-pillow, the friendly python image library fork. Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com> --- Changes: v3 -> v4: * Fixing a couple of typos v2 -> v3: * Simplified package * Better handling of setup type * Adding hash file v1 -> v2: * Added patch to remove platform guessing, now headers should be searched in buildroot folders * Fixed optional configure options package/Config.in | 1 + ...setup.py-removing-unneeded-platform-guess.patch | 85 ++++++++++++++++++++++ package/python-pillow/Config.in | 12 +++ package/python-pillow/python-pillow.hash | 2 + package/python-pillow/python-pillow.mk | 17 +++++ 5 files changed, 117 insertions(+) create mode 100644 package/python-pillow/0001-setup.py-removing-unneeded-platform-guess.patch create mode 100644 package/python-pillow/Config.in create mode 100644 package/python-pillow/python-pillow.hash create mode 100644 package/python-pillow/python-pillow.mk