Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v4] package/python-pillow: new package
@ 2016-02-20  9:55 Angelo Compagnucci
  2016-02-21 14:26 ` Thomas Petazzoni
  0 siblings, 1 reply; 10+ messages in thread
From: Angelo Compagnucci @ 2016-02-20  9:55 UTC (permalink / raw)
  To: buildroot

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

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))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-20  9:55 [Buildroot] [PATCH v4] package/python-pillow: new package Angelo Compagnucci
@ 2016-02-21 14:26 ` Thomas Petazzoni
  2016-02-23 22:18   ` Angelo Compagnucci
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2016-02-21 14:26 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-21 14:26 ` Thomas Petazzoni
@ 2016-02-23 22:18   ` Angelo Compagnucci
  2016-02-23 22:35     ` Thomas Petazzoni
  2016-02-23 22:36     ` Arnout Vandecappelle
  0 siblings, 2 replies; 10+ messages in thread
From: Angelo Compagnucci @ 2016-02-23 22:18 UTC (permalink / raw)
  To: buildroot

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



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-23 22:18   ` Angelo Compagnucci
@ 2016-02-23 22:35     ` Thomas Petazzoni
  2016-02-23 22:45       ` Arnout Vandecappelle
  2016-02-24  7:06       ` Angelo Compagnucci
  2016-02-23 22:36     ` Arnout Vandecappelle
  1 sibling, 2 replies; 10+ messages in thread
From: Thomas Petazzoni @ 2016-02-23 22:35 UTC (permalink / raw)
  To: buildroot

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-23 22:18   ` Angelo Compagnucci
  2016-02-23 22:35     ` Thomas Petazzoni
@ 2016-02-23 22:36     ` Arnout Vandecappelle
  1 sibling, 0 replies; 10+ messages in thread
From: Arnout Vandecappelle @ 2016-02-23 22:36 UTC (permalink / raw)
  To: buildroot

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]

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-23 22:35     ` Thomas Petazzoni
@ 2016-02-23 22:45       ` Arnout Vandecappelle
  2016-02-24  7:06       ` Angelo Compagnucci
  1 sibling, 0 replies; 10+ messages in thread
From: Arnout Vandecappelle @ 2016-02-23 22:45 UTC (permalink / raw)
  To: buildroot

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

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-02-23 22:35     ` Thomas Petazzoni
  2016-02-23 22:45       ` Arnout Vandecappelle
@ 2016-02-24  7:06       ` Angelo Compagnucci
  1 sibling, 0 replies; 10+ messages in thread
From: Angelo Compagnucci @ 2016-02-24  7:06 UTC (permalink / raw)
  To: buildroot

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



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
@ 2016-06-12 11:38 Angelo Compagnucci
  2016-06-13 14:48 ` Peter Korsgaard
  0 siblings, 1 reply; 10+ messages in thread
From: Angelo Compagnucci @ 2016-06-12 11:38 UTC (permalink / raw)
  To: buildroot

This patch adds python pillow, the friendly python fork, it
includes a backported patch to disable configuration platfom guessing.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/Config.in                                  |  1 +
 ...y-add-option-to-disable-platform-guessing.patch | 63 ++++++++++++++++++++++
 package/python-pillow/Config.in                    | 12 +++++
 package/python-pillow/python-pillow.hash           |  2 +
 package/python-pillow/python-pillow.mk             | 39 ++++++++++++++
 5 files changed, 117 insertions(+)
 create mode 100644 package/python-pillow/0001-setup.py-add-option-to-disable-platform-guessing.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

diff --git a/package/Config.in b/package/Config.in
index 3d99d72..a26043d 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -704,6 +704,7 @@ menu "External python modules"
 	source "package/python-pathtools/Config.in"
 	source "package/python-pathvalidate/Config.in"
 	source "package/python-pexpect/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-add-option-to-disable-platform-guessing.patch b/package/python-pillow/0001-setup.py-add-option-to-disable-platform-guessing.patch
new file mode 100644
index 0000000..a1a4cbe
--- /dev/null
+++ b/package/python-pillow/0001-setup.py-add-option-to-disable-platform-guessing.patch
@@ -0,0 +1,63 @@
+From 3344eff32427f1d10f3ca82ca4344c501a1a2055 Mon Sep 17 00:00:00 2001
+From: Angelo Compagnucci <angelo.compagnucci@gmail.com>
+Date: Sun, 12 Jun 2016 12:06:59 +0200
+Subject: [PATCH] setup.py: add option to disable platform guessing
+
+When cross-compiling (ex on Buildroot) platform guessing is not needed
+cause the environment is correctly built externally.
+This patch adds an option to disable platform guessing on Linux.
+---
+ setup.py | 19 ++++++++++++++-----
+ 1 file changed, 14 insertions(+), 5 deletions(-)
+
+diff --git a/setup.py b/setup.py
+index 0f74a40..f1eac1e 100644
+--- a/setup.py
++++ b/setup.py
+@@ -121,9 +121,13 @@ class pil_build_ext(build_ext):
+         ('disable-%s' % x, None, 'Disable support for %s' % x) for x in feature
+     ] + [
+         ('enable-%s' % x, None, 'Enable support for %s' % x) for x in feature
++    ] + [
++        ('disable-platform-guessing', None, 'Disable platform guessing on Linux'),
++        ('debug', None, 'Debug logging')
+     ]
+ 
+     def initialize_options(self):
++        self.disable_platform_guessing = None
+         build_ext.initialize_options(self)
+         for x in self.feature:
+             setattr(self, 'disable_%s' % x, None)
+@@ -190,7 +194,10 @@ class pil_build_ext(build_ext):
+         #
+         # add platform directories
+ 
+-        if sys.platform == "cygwin":
++        if self.disable_platform_guessing:
++            pass
++        
++        elif sys.platform == "cygwin":
+             # pythonX.Y.dll.a is in the /usr/lib/pythonX.Y/config directory
+             _add_directory(library_dirs,
+                            os.path.join("/usr/lib", "python%s" %
+@@ -352,11 +359,13 @@ class pil_build_ext(build_ext):
+                 _add_directory(include_dirs, tcl_dir)
+ 
+         # standard locations
+-        _add_directory(library_dirs, "/usr/local/lib")
+-        _add_directory(include_dirs, "/usr/local/include")
++        if not self.disable_platform_guessing:
++            _add_directory(library_dirs, "/usr/local/lib")
++            _add_directory(include_dirs, "/usr/local/include")
++
++            _add_directory(library_dirs, "/usr/lib")
++            _add_directory(include_dirs, "/usr/include")
+ 
+-        _add_directory(library_dirs, "/usr/lib")
+-        _add_directory(include_dirs, "/usr/include")
+ 
+         # on Windows, look for the OpenJPEG libraries in the location that
+         # the official installer puts them
+-- 
+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..0fc349c
--- /dev/null
+++ b/package/python-pillow/python-pillow.hash
@@ -0,0 +1,2 @@
+https://pypi.python.org/pypi?:action=show_md5&digest=7cfd093c11205d9e2ebe3c51dfcad510
+md5 7cfd093c11205d9e2ebe3c51dfcad510  Pillow-3.2.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..f186ab6
--- /dev/null
+++ b/package/python-pillow/python-pillow.mk
@@ -0,0 +1,39 @@
+################################################################################
+#
+# python-pillow
+#
+################################################################################
+
+PYTHON_PILLOW_VERSION = 3.2.0
+PYTHON_PILLOW_SOURCE = Pillow-$(PYTHON_PILLOW_VERSION).tar.gz
+PYTHON_PILLOW_SITE = https://pypi.python.org/packages/source/P/Pillow
+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)
+
+PYTHON_PILLOW_BUILD_OPTS += --disable-platform-guessing
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_JPEG),--enable-jpeg,--disable-jpeg)
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_ZLIB),--enable-zlib,--disable-zlib)
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_TIFF),--enable-tiff,--disable-tiff)
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_FREETYPE),--enable-freetype,--disable-freetype)
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_WEBP),--enable-webp,--disable-webp)
+PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_OPENJPEG),--enable-jpeg2000,--disable-jpeg2000)
+
+PYTHON_PILLOW_INSTALL_TARGET_OPTS += $(PYTHON_PILLOW_BUILD_OPTS)
+
+PYTHON_PILLOW_BUILD_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
+		$(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
+		$(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
+		$(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS))
+
+PYTHON_PILLOW_INSTALL_TARGET_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
+		$(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
+		$(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
+		$(PYTHON_PILLOW_INSTALL_TARGET_OPTS) install \
+		$(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS))
+
+$(eval $(python-package))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-06-12 11:38 Angelo Compagnucci
@ 2016-06-13 14:48 ` Peter Korsgaard
  2016-06-13 16:05   ` Angelo Compagnucci
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Korsgaard @ 2016-06-13 14:48 UTC (permalink / raw)
  To: buildroot

>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:

 > This patch adds python pillow, the friendly python fork, it
 > includes a backported patch to disable configuration platfom guessing.

It is afaik a fork of the python imaging library, not of python.

 > Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
 > ---
 >  package/Config.in                                  |  1 +
 >  ...y-add-option-to-disable-platform-guessing.patch | 63 ++++++++++++++++++++++
 >  package/python-pillow/Config.in                    | 12 +++++
 >  package/python-pillow/python-pillow.hash           |  2 +
 >  package/python-pillow/python-pillow.mk             | 39 ++++++++++++++
 >  5 files changed, 117 insertions(+)
 >  create mode 100644 package/python-pillow/0001-setup.py-add-option-to-disable-platform-guessing.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

 > +++ b/package/python-pillow/python-pillow.hash
 > @@ -0,0 +1,2 @@
 > +https://pypi.python.org/pypi?:action=show_md5&digest=7cfd093c11205d9e2ebe3c51dfcad510
 > +md5 7cfd093c11205d9e2ebe3c51dfcad510  Pillow-3.2.0.tar.gz

Please also provide a stronger hash like sha256.

> diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
 > new file mode 100644
 > index 0000000..f186ab6
 > --- /dev/null
 > +++ b/package/python-pillow/python-pillow.mk
 > @@ -0,0 +1,39 @@
 > +################################################################################
 > +#
 > +# python-pillow
 > +#
 > +################################################################################
 > +
 > +PYTHON_PILLOW_VERSION = 3.2.0
 > +PYTHON_PILLOW_SOURCE = Pillow-$(PYTHON_PILLOW_VERSION).tar.gz
 > +PYTHON_PILLOW_SITE = https://pypi.python.org/packages/source/P/Pillow
 > +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)
 > +
 > +PYTHON_PILLOW_BUILD_OPTS += --disable-platform-guessing
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_JPEG),--enable-jpeg,--disable-jpeg)
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_ZLIB),--enable-zlib,--disable-zlib)
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_TIFF),--enable-tiff,--disable-tiff)
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_FREETYPE),--enable-freetype,--disable-freetype)
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_WEBP),--enable-webp,--disable-webp)
 > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_OPENJPEG),--enable-jpeg2000,--disable-jpeg2000)
 > +
 > +PYTHON_PILLOW_INSTALL_TARGET_OPTS += $(PYTHON_PILLOW_BUILD_OPTS)

If these are build options, why are you passing them to the install
step?

Why are you using += and not just = ? The default value of
_INSTALL_TARGET_OPTS will only be set when python-package is evaluated
below, so this will not append to the default.

 > +
 > +PYTHON_PILLOW_BUILD_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
 > +		$(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
 > +		$(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
 > +		$(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS))

Isn't this identical to the default build commands (with
PYTHON_PILLOW_BASE_BUILD_TGT = build_ext)?

 > +
 > +PYTHON_PILLOW_INSTALL_TARGET_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
 > +		$(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
 > +		$(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
 > +		$(PYTHON_PILLOW_INSTALL_TARGET_OPTS) install \
 > +		$(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS))
 > +

And this to the default target install. Why do you need to run build_ext
again?

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH v4] package/python-pillow: new package
  2016-06-13 14:48 ` Peter Korsgaard
@ 2016-06-13 16:05   ` Angelo Compagnucci
  0 siblings, 0 replies; 10+ messages in thread
From: Angelo Compagnucci @ 2016-06-13 16:05 UTC (permalink / raw)
  To: buildroot

Dear Peter Korsgaard,

2016-06-13 16:48 GMT+02:00 Peter Korsgaard <peter@korsgaard.com>:
>>>>>> "Angelo" == Angelo Compagnucci <angelo.compagnucci@gmail.com> writes:
>
>  > This patch adds python pillow, the friendly python fork, it
>  > includes a backported patch to disable configuration platfom guessing.
>
> It is afaik a fork of the python imaging library, not of python.

Ops!

>  > Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>  > ---
>  >  package/Config.in                                  |  1 +
>  >  ...y-add-option-to-disable-platform-guessing.patch | 63 ++++++++++++++++++++++
>  >  package/python-pillow/Config.in                    | 12 +++++
>  >  package/python-pillow/python-pillow.hash           |  2 +
>  >  package/python-pillow/python-pillow.mk             | 39 ++++++++++++++
>  >  5 files changed, 117 insertions(+)
>  >  create mode 100644 package/python-pillow/0001-setup.py-add-option-to-disable-platform-guessing.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
>
>  > +++ b/package/python-pillow/python-pillow.hash
>  > @@ -0,0 +1,2 @@
>  > +https://pypi.python.org/pypi?:action=show_md5&digest=7cfd093c11205d9e2ebe3c51dfcad510
>  > +md5 7cfd093c11205d9e2ebe3c51dfcad510  Pillow-3.2.0.tar.gz
>
> Please also provide a stronger hash like sha256.

Right!

>> diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
>  > new file mode 100644
>  > index 0000000..f186ab6
>  > --- /dev/null
>  > +++ b/package/python-pillow/python-pillow.mk
>  > @@ -0,0 +1,39 @@
>  > +################################################################################
>  > +#
>  > +# python-pillow
>  > +#
>  > +################################################################################
>  > +
>  > +PYTHON_PILLOW_VERSION = 3.2.0
>  > +PYTHON_PILLOW_SOURCE = Pillow-$(PYTHON_PILLOW_VERSION).tar.gz
>  > +PYTHON_PILLOW_SITE = https://pypi.python.org/packages/source/P/Pillow
>  > +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)
>  > +
>  > +PYTHON_PILLOW_BUILD_OPTS += --disable-platform-guessing
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_JPEG),--enable-jpeg,--disable-jpeg)
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_ZLIB),--enable-zlib,--disable-zlib)
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_TIFF),--enable-tiff,--disable-tiff)
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_FREETYPE),--enable-freetype,--disable-freetype)
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_WEBP),--enable-webp,--disable-webp)
>  > +PYTHON_PILLOW_BUILD_OPTS += $(if $(BR2_PACKAGE_OPENJPEG),--enable-jpeg2000,--disable-jpeg2000)
>  > +
>  > +PYTHON_PILLOW_INSTALL_TARGET_OPTS += $(PYTHON_PILLOW_BUILD_OPTS)
>
> If these are build options, why are you passing them to the install
> step?

Cause this package as a strange build/install system and requires the
same set of option both in build and install steps.

> Why are you using += and not just = ? The default value of
> _INSTALL_TARGET_OPTS will only be set when python-package is evaluated
> below, so this will not append to the default.

I've not dug so deep inside python-package handling, so I used +=.

>  > +
>  > +PYTHON_PILLOW_BUILD_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
>  > +            $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>  > +            $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
>  > +            $(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS))
>
> Isn't this identical to the default build commands (with
> PYTHON_PILLOW_BASE_BUILD_TGT = build_ext)?

In a previous review [1] was said to me to rewrite both BUILD_CMDS and
INSTALL_TARGET_CMDS instead of modifying PYTHON_PILLOW_BASE_BUILD_TGT
cause this is the first package to have such need.

>  > +
>  > +PYTHON_PILLOW_INSTALL_TARGET_CMDS = (cd $(PYTHON_PILLOW_BUILDDIR); \
>  > +            $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>  > +            $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
>  > +            $(PYTHON_PILLOW_INSTALL_TARGET_OPTS) install \
>  > +            $(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS))
>  > +
>
> And this to the default target install. Why do you need to run build_ext
> again?

Cause the install step of this package doesn't work if called without build_ext.

[1] http://article.gmane.org/gmane.comp.lib.uclibc.buildroot/141233/match=pillow

Sincerely, Angelo

>
> --
> Bye, Peter Korsgaard



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-06-13 16:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-20  9:55 [Buildroot] [PATCH v4] package/python-pillow: new package Angelo Compagnucci
2016-02-21 14:26 ` Thomas Petazzoni
2016-02-23 22:18   ` Angelo Compagnucci
2016-02-23 22:35     ` Thomas Petazzoni
2016-02-23 22:45       ` Arnout Vandecappelle
2016-02-24  7:06       ` Angelo Compagnucci
2016-02-23 22:36     ` Arnout Vandecappelle
  -- strict thread matches above, loose matches on Subject: below --
2016-06-12 11:38 Angelo Compagnucci
2016-06-13 14:48 ` Peter Korsgaard
2016-06-13 16:05   ` Angelo Compagnucci

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox