| Message ID | 1467756127-30393-1-git-send-email-angelo.compagnucci@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
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
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
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
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
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
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
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))
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(-)