[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Angelo Compagnucci]

This patch changes PYTHON_PILLOW_INSTALL_TARGET_CMDS to actually
install pillow in target directory instead of host.
It also fixes the version for the hash.

Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 package/python-pillow/python-pillow.hash | 2 +-
 package/python-pillow/python-pillow.mk   | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/package/python-pillow/python-pillow.hash b/package/python-pillow/python-pillow.hash
index 033692d..bf7828d 100644
--- a/package/python-pillow/python-pillow.hash
+++ b/package/python-pillow/python-pillow.hash
@@ -1,4 +1,4 @@
 # https://pypi.python.org/pypi?:action=show_md5&digest=b5a15b03bf402fe254636c015fcf04da
 md5 b5a15b03bf402fe254636c015fcf04da  Pillow-3.3.0.tar.gz
 # sha256 locally computed
-sha256 031e7c9c885a4f343d1ad366c7fd2340449dc70318acb4a28d6411994f0accd1  Pillow-3.2.0.tar.gz
+sha256 031e7c9c885a4f343d1ad366c7fd2340449dc70318acb4a28d6411994f0accd1  Pillow-3.3.0.tar.gz
diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
index 878fdad..c7ee2b3 100644
--- a/package/python-pillow/python-pillow.mk
+++ b/package/python-pillow/python-pillow.mk
@@ -59,6 +59,11 @@ PYTHON_PILLOW_BUILD_CMDS = cd $(PYTHON_PILLOW_BUILDDIR); \
 		$(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
 		$(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS)
 
-PYTHON_PILLOW_INSTALL_TARGET_CMDS = $(PYTHON_PILLOW_BUILD_CMDS) install
+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_BUILD_OPTS) install \
+		$(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS) \
+		$(PYTHON_PILLOW_INSTALL_TARGET_OPTS)
 
 $(eval $(python-package))
-- 
1.9.1

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Thomas Petazzoni]

Hello,

On Wed,  6 Jul 2016 00:02:07 +0200, Angelo Compagnucci wrote:

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

What are you doing the build_ext target again here?

> +		$(PYTHON_PILLOW_BUILD_OPTS) install \
> +		$(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS) \
> +		$(PYTHON_PILLOW_INSTALL_TARGET_OPTS)

Also, please use define ... endef for those commands (ditto for the
build command, I missed that when applying the patch).

Thanks,

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

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Angelo Compagnucci]

Dear Thomas Petazzoni,

2016-07-06 14:46 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Wed,  6 Jul 2016 00:02:07 +0200, Angelo Compagnucci wrote:
>
>> -PYTHON_PILLOW_INSTALL_TARGET_CMDS = $(PYTHON_PILLOW_BUILD_CMDS) install
>> +PYTHON_PILLOW_INSTALL_TARGET_CMDS = cd $(PYTHON_PILLOW_BUILDDIR); \
>> +             $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>> +             $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
>
> What are you doing the build_ext target again here?

python pillow build command is:

python setup.py build_ext --enable-[feature]

and installation command is:

python setup.py build_ext --enable-[feature] install

It doesn't work either way, see the documentation here [1].

>> +             $(PYTHON_PILLOW_BUILD_OPTS) install \
>> +             $(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS) \
>> +             $(PYTHON_PILLOW_INSTALL_TARGET_OPTS)
>
> Also, please use define ... endef for those commands (ditto for the
> build command, I missed that when applying the patch).

Define what? They are the manadtory build and installation commands,
why make them conditional?

[1] https://pillow.readthedocs.io/en/3.3.x/installation.html#build-options

>
> Thanks,
>
> Thomas

Sincerely, Angelo

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



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

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Thomas Petazzoni]

Hello,

On Wed, 6 Jul 2016 14:51:26 +0200, Angelo Compagnucci wrote:

> > What are you doing the build_ext target again here?  
> 
> python pillow build command is:
> 
> python setup.py build_ext --enable-[feature]
> 
> and installation command is:
> 
> python setup.py build_ext --enable-[feature] install
> 
> It doesn't work either way, see the documentation here [1].

Ah, ok. Thanks for the explanation. I hate to say I dislike what the
python-pillow package needs to do, but there isn't a better option for
now with the current python-package infrastructure. We'll live with
that for now, and potentially improve the python-package infrastructure
in the future if more packages have similar requirements.

> >> +             $(PYTHON_PILLOW_BUILD_OPTS) install \
> >> +             $(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS) \
> >> +             $(PYTHON_PILLOW_INSTALL_TARGET_OPTS)  
> >
> > Also, please use define ... endef for those commands (ditto for the
> > build command, I missed that when applying the patch).  
> 
> Define what? They are the manadtory build and installation commands,
> why make them conditional?

Use:

define PYTHON_PILLOW_BUILD_CMDS
	...
endef

instead of

PYTHON_PILLOW_BUILD_CMDS = ...

Ditto for the install target commands.

Thanks,

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

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Angelo Compagnucci]

Dear Thomas Petazzoni,

2016-07-06 15:05 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Wed, 6 Jul 2016 14:51:26 +0200, Angelo Compagnucci wrote:
>
>> > What are you doing the build_ext target again here?
>>
>> python pillow build command is:
>>
>> python setup.py build_ext --enable-[feature]
>>
>> and installation command is:
>>
>> python setup.py build_ext --enable-[feature] install
>>
>> It doesn't work either way, see the documentation here [1].
>
> Ah, ok. Thanks for the explanation. I hate to say I dislike what the
> python-pillow package needs to do, but there isn't a better option for
> now with the current python-package infrastructure. We'll live with
> that for now, and potentially improve the python-package infrastructure
> in the future if more packages have similar requirements.

Well, yes, I proposed a solution on a thread months ago, but for now
we a have such a requirement only for this package, so I convey with
you that the problem not arises.

>
>> >> +             $(PYTHON_PILLOW_BUILD_OPTS) install \
>> >> +             $(PYTHON_PILLOW_BASE_INSTALL_TARGET_OPTS) \
>> >> +             $(PYTHON_PILLOW_INSTALL_TARGET_OPTS)
>> >
>> > Also, please use define ... endef for those commands (ditto for the
>> > build command, I missed that when applying the patch).
>>
>> Define what? They are the manadtory build and installation commands,
>> why make them conditional?
>
> Use:
>
> define PYTHON_PILLOW_BUILD_CMDS
>         ...
> endef
>
> instead of
>
> PYTHON_PILLOW_BUILD_CMDS = ...
>
> Ditto for the install target commands.

Will do!

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



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

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Thomas Petazzoni]

Hello,

On Wed, 6 Jul 2016 15:09:26 +0200, Angelo Compagnucci wrote:

> > Ah, ok. Thanks for the explanation. I hate to say I dislike what the
> > python-pillow package needs to do, but there isn't a better option for
> > now with the current python-package infrastructure. We'll live with
> > that for now, and potentially improve the python-package infrastructure
> > in the future if more packages have similar requirements.  
> 
> Well, yes, I proposed a solution on a thread months ago, but for now
> we a have such a requirement only for this package, so I convey with
> you that the problem not arises.

Agreed.

It is not clear to me if it is legal for a Python package to build only
with "build_ext" and not "build". According to the Python documentation
"build" should do everything, and "build_ext" is a sub-command to build
just the "extensions".

Maybe the problem should be reported upstream?

> > Ditto for the install target commands.  
> 
> Will do!

Thanks!

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

[Buildroot] [PATCH] package/python-pillow: fix wrong install step

[Angelo Compagnucci]

Dear Thomas Petazzoni,

2016-07-06 15:15 GMT+02:00 Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>:
> Hello,
>
> On Wed, 6 Jul 2016 15:09:26 +0200, Angelo Compagnucci wrote:
>
>> > Ah, ok. Thanks for the explanation. I hate to say I dislike what the
>> > python-pillow package needs to do, but there isn't a better option for
>> > now with the current python-package infrastructure. We'll live with
>> > that for now, and potentially improve the python-package infrastructure
>> > in the future if more packages have similar requirements.
>>
>> Well, yes, I proposed a solution on a thread months ago, but for now
>> we a have such a requirement only for this package, so I convey with
>> you that the problem not arises.
>
> Agreed.
>
> It is not clear to me if it is legal for a Python package to build only
> with "build_ext" and not "build". According to the Python documentation
> "build" should do everything, and "build_ext" is a sub-command to build
> just the "extensions".
>
> Maybe the problem should be reported upstream?

Well, the python-pillow setup needs a good refactor IMHO, but it's far
from my free time atm.

>
>> > Ditto for the install target commands.
>>
>> Will do!
>
> Thanks!

Sincerely, Angelo

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



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