* [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
@ 2022-03-16 6:02 James Hilliard
2022-03-20 13:14 ` Angelo Compagnucci
2022-03-21 21:32 ` Arnout Vandecappelle
0 siblings, 2 replies; 8+ messages in thread
From: James Hilliard @ 2022-03-16 6:02 UTC (permalink / raw)
To: buildroot; +Cc: Angelo Compagnucci, James Hilliard, Asaf Kahlon
Currently pillow doesn't correctly search pkg-config system paths
for some libraries which can prevent some libraries from being
properly detected/enabled in pillow.
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
...Search-pkg-config-system-libs-cflags.patch | 33 +++++++++++++++++++
package/python-pillow/python-pillow.mk | 23 +++----------
2 files changed, 37 insertions(+), 19 deletions(-)
create mode 100644 package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
diff --git a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
new file mode 100644
index 0000000000..9f979b048f
--- /dev/null
+++ b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
@@ -0,0 +1,33 @@
+From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
+From: James Hilliard <james.hilliard1@gmail.com>
+Date: Tue, 15 Mar 2022 23:31:59 -0600
+Subject: [PATCH] Search pkg-config system libs/cflags.
+
+We need to search the system paths as well from pkg-config for
+some packages to be found properly.
+
+Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
+[Upstream status:
+https://github.com/python-pillow/Pillow/pull/6138]
+---
+ setup.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/setup.py b/setup.py
+index 3468b260..59d65ce2 100755
+--- a/setup.py
++++ b/setup.py
+@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
+ def _pkg_config(name):
+ try:
+ command = os.environ.get("PKG_CONFIG", "pkg-config")
+- command_libs = [command, "--libs-only-L", name]
+- command_cflags = [command, "--cflags-only-I", name]
++ command_libs = [command, "--libs-only-L", "--keep-system-libs", name]
++ command_cflags = [command, "--cflags-only-I", "--keep-system-cflags", name]
+ if not DEBUG:
+ command_libs.append("--silence-errors")
+ command_cflags.append("--silence-errors")
+--
+2.35.1
+
diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
index 901876e0ee..ef677855b2 100644
--- a/package/python-pillow/python-pillow.mk
+++ b/package/python-pillow/python-pillow.mk
@@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
PYTHON_PILLOW_CPE_ID_VENDOR = python
PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
PYTHON_PILLOW_SETUP_TYPE = setuptools
-PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
+PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
+
+PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
+PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
ifeq ($(BR2_PACKAGE_FREETYPE),y)
PYTHON_PILLOW_DEPENDENCIES += freetype
@@ -68,22 +71,4 @@ else
PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
endif
-define PYTHON_PILLOW_BUILD_CMDS
- cd $(PYTHON_PILLOW_BUILDDIR); \
- PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
- $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
- $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
- $(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS)
-endef
-
-define PYTHON_PILLOW_INSTALL_TARGET_CMDS
- cd $(PYTHON_PILLOW_BUILDDIR); \
- PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
- $(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)
-endef
-
$(eval $(python-package))
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-16 6:02 [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths James Hilliard
@ 2022-03-20 13:14 ` Angelo Compagnucci
2022-03-21 21:32 ` Arnout Vandecappelle
1 sibling, 0 replies; 8+ messages in thread
From: Angelo Compagnucci @ 2022-03-20 13:14 UTC (permalink / raw)
To: James Hilliard; +Cc: buildroot
[-- Attachment #1.1: Type: text/plain, Size: 4435 bytes --]
On Wed, Mar 16, 2022 at 7:02 AM James Hilliard <james.hilliard1@gmail.com>
wrote:
> Currently pillow doesn't correctly search pkg-config system paths
> for some libraries which can prevent some libraries from being
> properly detected/enabled in pillow.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>
Tested-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> ---
> ...Search-pkg-config-system-libs-cflags.patch | 33 +++++++++++++++++++
> package/python-pillow/python-pillow.mk | 23 +++----------
> 2 files changed, 37 insertions(+), 19 deletions(-)
> create mode 100644
> package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>
> diff --git
> a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> new file mode 100644
> index 0000000000..9f979b048f
> --- /dev/null
> +++ b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> @@ -0,0 +1,33 @@
> +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
> +From: James Hilliard <james.hilliard1@gmail.com>
> +Date: Tue, 15 Mar 2022 23:31:59 -0600
> +Subject: [PATCH] Search pkg-config system libs/cflags.
> +
> +We need to search the system paths as well from pkg-config for
> +some packages to be found properly.
> +
> +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> +[Upstream status:
> +https://github.com/python-pillow/Pillow/pull/6138]
> +---
> + setup.py | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/setup.py b/setup.py
> +index 3468b260..59d65ce2 100755
> +--- a/setup.py
> ++++ b/setup.py
> +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
> + def _pkg_config(name):
> + try:
> + command = os.environ.get("PKG_CONFIG", "pkg-config")
> +- command_libs = [command, "--libs-only-L", name]
> +- command_cflags = [command, "--cflags-only-I", name]
> ++ command_libs = [command, "--libs-only-L", "--keep-system-libs",
> name]
> ++ command_cflags = [command, "--cflags-only-I",
> "--keep-system-cflags", name]
> + if not DEBUG:
> + command_libs.append("--silence-errors")
> + command_cflags.append("--silence-errors")
> +--
> +2.35.1
> +
> diff --git a/package/python-pillow/python-pillow.mk
> b/package/python-pillow/python-pillow.mk
> index 901876e0ee..ef677855b2 100644
> --- a/package/python-pillow/python-pillow.mk
> +++ b/package/python-pillow/python-pillow.mk
> @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
> PYTHON_PILLOW_CPE_ID_VENDOR = python
> PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
> PYTHON_PILLOW_SETUP_TYPE = setuptools
> -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
> +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
> +
> +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
> +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
>
> ifeq ($(BR2_PACKAGE_FREETYPE),y)
> PYTHON_PILLOW_DEPENDENCIES += freetype
> @@ -68,22 +71,4 @@ else
> PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
> endif
>
> -define PYTHON_PILLOW_BUILD_CMDS
> - cd $(PYTHON_PILLOW_BUILDDIR); \
> - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
> - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
> - $(PYTHON_PILLOW_BASE_BUILD_OPTS)
> $(PYTHON_PILLOW_BUILD_OPTS)
> -endef
> -
> -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
> - cd $(PYTHON_PILLOW_BUILDDIR); \
> - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> - $(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)
> -endef
> -
> $(eval $(python-package))
> --
> 2.25.1
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
>
--
Angelo Compagnucci
Software Engineer
angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 (0)42 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|
[-- Attachment #1.2: Type: text/html, Size: 10055 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-16 6:02 [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths James Hilliard
2022-03-20 13:14 ` Angelo Compagnucci
@ 2022-03-21 21:32 ` Arnout Vandecappelle
2022-03-21 21:56 ` James Hilliard
1 sibling, 1 reply; 8+ messages in thread
From: Arnout Vandecappelle @ 2022-03-21 21:32 UTC (permalink / raw)
To: James Hilliard, buildroot; +Cc: Angelo Compagnucci, Asaf Kahlon
Hi James,
On 16/03/2022 07:02, James Hilliard wrote:
> Currently pillow doesn't correctly search pkg-config system paths
> for some libraries which can prevent some libraries from being
> properly detected/enabled in pillow.
>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> ...Search-pkg-config-system-libs-cflags.patch | 33 +++++++++++++++++++
> package/python-pillow/python-pillow.mk | 23 +++----------
> 2 files changed, 37 insertions(+), 19 deletions(-)
> create mode 100644 package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>
> diff --git a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> new file mode 100644
> index 0000000000..9f979b048f
> --- /dev/null
> +++ b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> @@ -0,0 +1,33 @@
> +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
> +From: James Hilliard <james.hilliard1@gmail.com>
> +Date: Tue, 15 Mar 2022 23:31:59 -0600
> +Subject: [PATCH] Search pkg-config system libs/cflags.
> +
> +We need to search the system paths as well from pkg-config for
> +some packages to be found properly.
> +
> +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> +[Upstream status:
> +https://github.com/python-pillow/Pillow/pull/6138]
> +---
> + setup.py | 4 ++--
> + 1 file changed, 2 insertions(+), 2 deletions(-)
> +
> +diff --git a/setup.py b/setup.py
> +index 3468b260..59d65ce2 100755
> +--- a/setup.py
> ++++ b/setup.py
> +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
> + def _pkg_config(name):
> + try:
> + command = os.environ.get("PKG_CONFIG", "pkg-config")
> +- command_libs = [command, "--libs-only-L", name]
> +- command_cflags = [command, "--cflags-only-I", name]
> ++ command_libs = [command, "--libs-only-L", "--keep-system-libs", name]
> ++ command_cflags = [command, "--cflags-only-I", "--keep-system-cflags", name]
You gave an additional explanation why this is needed in the upstream PR:
Zlib and anything that has headers in the normal system libs/include folders
seems to not get their headers picked up without this change.
I think this issue is mostly going to be something people hit when cross
compiling since the prefix based system libs/include folder would probably work
in most of the usual cases(searching in sys.prefix will not work when cross
compiling since it points to the host toolchain prefix rather than the target
toolchain sysroot prefix):
However, that implies that either we have to make sure that sys.prefix is set
correctly (i.e. point it to staging instead of host), or pillow is using
sys.prefix incorrectly.
Before that, still, I don't understand how this can be an issue. Unless if
pillow also passes something like -nostdinc, our toolchain wrapper should make
sure that staging/usr/include is in the search path.
There also doesn't seem to be an autobuilder failure due to this... But maybe
it builds successfully, just without some optional dependency? Please provide
more details about the failure in the commit message.
> + if not DEBUG:
> + command_libs.append("--silence-errors")
> + command_cflags.append("--silence-errors")
> +--
> +2.35.1
> +
> diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
> index 901876e0ee..ef677855b2 100644
> --- a/package/python-pillow/python-pillow.mk
> +++ b/package/python-pillow/python-pillow.mk
> @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
> PYTHON_PILLOW_CPE_ID_VENDOR = python
> PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
> PYTHON_PILLOW_SETUP_TYPE = setuptools
> -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
> +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
This change isn't explained in the commit message, and seems unrelated.
> +
> +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
> +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
The need for these two changes is also not clear at all from the commit message.
I've marked the patch as Changes Requested.
Regards,
Arnout
>
> ifeq ($(BR2_PACKAGE_FREETYPE),y)
> PYTHON_PILLOW_DEPENDENCIES += freetype
> @@ -68,22 +71,4 @@ else
> PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
> endif
>
> -define PYTHON_PILLOW_BUILD_CMDS
> - cd $(PYTHON_PILLOW_BUILDDIR); \
> - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
> - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
> - $(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS)
> -endef
> -
> -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
> - cd $(PYTHON_PILLOW_BUILDDIR); \
> - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> - $(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)
> -endef
> -
> $(eval $(python-package))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-21 21:32 ` Arnout Vandecappelle
@ 2022-03-21 21:56 ` James Hilliard
2022-03-21 22:10 ` Angelo Compagnucci
0 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2022-03-21 21:56 UTC (permalink / raw)
To: Arnout Vandecappelle; +Cc: Angelo Compagnucci, Asaf Kahlon, buildroot
On Mon, Mar 21, 2022 at 3:32 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>
> Hi James,
>
> On 16/03/2022 07:02, James Hilliard wrote:
> > Currently pillow doesn't correctly search pkg-config system paths
> > for some libraries which can prevent some libraries from being
> > properly detected/enabled in pillow.
> >
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> > ...Search-pkg-config-system-libs-cflags.patch | 33 +++++++++++++++++++
> > package/python-pillow/python-pillow.mk | 23 +++----------
> > 2 files changed, 37 insertions(+), 19 deletions(-)
> > create mode 100644 package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> >
> > diff --git a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> > new file mode 100644
> > index 0000000000..9f979b048f
> > --- /dev/null
> > +++ b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> > @@ -0,0 +1,33 @@
> > +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
> > +From: James Hilliard <james.hilliard1@gmail.com>
> > +Date: Tue, 15 Mar 2022 23:31:59 -0600
> > +Subject: [PATCH] Search pkg-config system libs/cflags.
> > +
> > +We need to search the system paths as well from pkg-config for
> > +some packages to be found properly.
> > +
> > +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > +[Upstream status:
> > +https://github.com/python-pillow/Pillow/pull/6138]
> > +---
> > + setup.py | 4 ++--
> > + 1 file changed, 2 insertions(+), 2 deletions(-)
> > +
> > +diff --git a/setup.py b/setup.py
> > +index 3468b260..59d65ce2 100755
> > +--- a/setup.py
> > ++++ b/setup.py
> > +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
> > + def _pkg_config(name):
> > + try:
> > + command = os.environ.get("PKG_CONFIG", "pkg-config")
> > +- command_libs = [command, "--libs-only-L", name]
> > +- command_cflags = [command, "--cflags-only-I", name]
> > ++ command_libs = [command, "--libs-only-L", "--keep-system-libs", name]
> > ++ command_cflags = [command, "--cflags-only-I", "--keep-system-cflags", name]
>
> You gave an additional explanation why this is needed in the upstream PR:
>
> Zlib and anything that has headers in the normal system libs/include folders
> seems to not get their headers picked up without this change.
>
> I think this issue is mostly going to be something people hit when cross
> compiling since the prefix based system libs/include folder would probably work
> in most of the usual cases(searching in sys.prefix will not work when cross
> compiling since it points to the host toolchain prefix rather than the target
> toolchain sysroot prefix):
>
>
> However, that implies that either we have to make sure that sys.prefix is set
> correctly (i.e. point it to staging instead of host), or pillow is using
> sys.prefix incorrectly.
I think pillow is using sys.prefix in a way that is not really cross compilation
compatible, however using pkg-config with sysm libs/cflags seems to be
sufficient
for it to pass its non-standard header checks.
>
> Before that, still, I don't understand how this can be an issue. Unless if
> pillow also passes something like -nostdinc, our toolchain wrapper should make
> sure that staging/usr/include is in the search path.
It's due to pillow having custom header checks like this:
https://github.com/python-pillow/Pillow/blob/9.0.1/setup.py#L633
>
> There also doesn't seem to be an autobuilder failure due to this... But maybe
> it builds successfully, just without some optional dependency? Please provide
> more details about the failure in the commit message.
The failure was getting hidden by the non-standard build/install cmd
overrides it
would appear, it seemed pillow was getting built for the host instead
of the target with
those, I didn't investigate in too much detail as those custom
build/install overrides
are not actually needed.
>
>
>
> > + if not DEBUG:
> > + command_libs.append("--silence-errors")
> > + command_cflags.append("--silence-errors")
> > +--
> > +2.35.1
> > +
> > diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
> > index 901876e0ee..ef677855b2 100644
> > --- a/package/python-pillow/python-pillow.mk
> > +++ b/package/python-pillow/python-pillow.mk
> > @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
> > PYTHON_PILLOW_CPE_ID_VENDOR = python
> > PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
> > PYTHON_PILLOW_SETUP_TYPE = setuptools
> > -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
> > +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
>
> This change isn't explained in the commit message, and seems unrelated.
>
> > +
> > +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
> > +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
>
> The need for these two changes is also not clear at all from the commit message.
>
>
> I've marked the patch as Changes Requested.
>
> Regards,
> Arnout
>
> >
> > ifeq ($(BR2_PACKAGE_FREETYPE),y)
> > PYTHON_PILLOW_DEPENDENCIES += freetype
> > @@ -68,22 +71,4 @@ else
> > PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
> > endif
> >
> > -define PYTHON_PILLOW_BUILD_CMDS
> > - cd $(PYTHON_PILLOW_BUILDDIR); \
> > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> > - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
> > - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
> > - $(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS)
> > -endef
> > -
> > -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
> > - cd $(PYTHON_PILLOW_BUILDDIR); \
> > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> > - $(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)
> > -endef
> > -
> > $(eval $(python-package))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-21 21:56 ` James Hilliard
@ 2022-03-21 22:10 ` Angelo Compagnucci
2022-03-21 22:35 ` Angelo Compagnucci
0 siblings, 1 reply; 8+ messages in thread
From: Angelo Compagnucci @ 2022-03-21 22:10 UTC (permalink / raw)
To: James Hilliard; +Cc: Angelo Compagnucci, Asaf Kahlon, buildroot
[-- Attachment #1.1: Type: text/plain, Size: 7323 bytes --]
On Mon, Mar 21, 2022 at 10:56 PM James Hilliard <james.hilliard1@gmail.com>
wrote:
> On Mon, Mar 21, 2022 at 3:32 PM Arnout Vandecappelle <arnout@mind.be>
> wrote:
> >
> > Hi James,
> >
> > On 16/03/2022 07:02, James Hilliard wrote:
> > > Currently pillow doesn't correctly search pkg-config system paths
> > > for some libraries which can prevent some libraries from being
> > > properly detected/enabled in pillow.
> > >
> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > ---
> > > ...Search-pkg-config-system-libs-cflags.patch | 33
> +++++++++++++++++++
> > > package/python-pillow/python-pillow.mk | 23 +++----------
> > > 2 files changed, 37 insertions(+), 19 deletions(-)
> > > create mode 100644
> package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> > >
> > > diff --git
> a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> > > new file mode 100644
> > > index 0000000000..9f979b048f
> > > --- /dev/null
> > > +++
> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
> > > @@ -0,0 +1,33 @@
> > > +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
> > > +From: James Hilliard <james.hilliard1@gmail.com>
> > > +Date: Tue, 15 Mar 2022 23:31:59 -0600
> > > +Subject: [PATCH] Search pkg-config system libs/cflags.
> > > +
> > > +We need to search the system paths as well from pkg-config for
> > > +some packages to be found properly.
> > > +
> > > +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > > +[Upstream status:
> > > +https://github.com/python-pillow/Pillow/pull/6138]
> > > +---
> > > + setup.py | 4 ++--
> > > + 1 file changed, 2 insertions(+), 2 deletions(-)
> > > +
> > > +diff --git a/setup.py b/setup.py
> > > +index 3468b260..59d65ce2 100755
> > > +--- a/setup.py
> > > ++++ b/setup.py
> > > +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
> > > + def _pkg_config(name):
> > > + try:
> > > + command = os.environ.get("PKG_CONFIG", "pkg-config")
> > > +- command_libs = [command, "--libs-only-L", name]
> > > +- command_cflags = [command, "--cflags-only-I", name]
> > > ++ command_libs = [command, "--libs-only-L",
> "--keep-system-libs", name]
> > > ++ command_cflags = [command, "--cflags-only-I",
> "--keep-system-cflags", name]
> >
> > You gave an additional explanation why this is needed in the upstream
> PR:
> >
> > Zlib and anything that has headers in the normal system libs/include
> folders
> > seems to not get their headers picked up without this change.
> >
> > I think this issue is mostly going to be something people hit when cross
> > compiling since the prefix based system libs/include folder would
> probably work
> > in most of the usual cases(searching in sys.prefix will not work when
> cross
> > compiling since it points to the host toolchain prefix rather than the
> target
> > toolchain sysroot prefix):
> >
> >
> > However, that implies that either we have to make sure that sys.prefix
> is set
> > correctly (i.e. point it to staging instead of host), or pillow is using
> > sys.prefix incorrectly.
>
> I think pillow is using sys.prefix in a way that is not really cross
> compilation
> compatible, however using pkg-config with sysm libs/cflags seems to be
> sufficient
> for it to pass its non-standard header checks.
>
I remember having added the --disable-platform-guessing exactly to overcome
this problem. All the setting should be provided by buildroot. Probably,
the logic behind this is slightly changed, and the mechanism doesn't work
anymore. I'll try to have a look.
>
> >
> > Before that, still, I don't understand how this can be an issue.
> Unless if
> > pillow also passes something like -nostdinc, our toolchain wrapper
> should make
> > sure that staging/usr/include is in the search path.
>
> It's due to pillow having custom header checks like this:
> https://github.com/python-pillow/Pillow/blob/9.0.1/setup.py#L633
>
> >
> > There also doesn't seem to be an autobuilder failure due to this...
> But maybe
> > it builds successfully, just without some optional dependency? Please
> provide
> > more details about the failure in the commit message.
>
> The failure was getting hidden by the non-standard build/install cmd
> overrides it
> would appear, it seemed pillow was getting built for the host instead
> of the target with
> those, I didn't investigate in too much detail as those custom
> build/install overrides
> are not actually needed.
>
> >
> >
> >
> > > + if not DEBUG:
> > > + command_libs.append("--silence-errors")
> > > + command_cflags.append("--silence-errors")
> > > +--
> > > +2.35.1
> > > +
> > > diff --git a/package/python-pillow/python-pillow.mk
> b/package/python-pillow/python-pillow.mk
> > > index 901876e0ee..ef677855b2 100644
> > > --- a/package/python-pillow/python-pillow.mk
> > > +++ b/package/python-pillow/python-pillow.mk
> > > @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
> > > PYTHON_PILLOW_CPE_ID_VENDOR = python
> > > PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
> > > PYTHON_PILLOW_SETUP_TYPE = setuptools
> > > -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
> > > +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
> >
> > This change isn't explained in the commit message, and seems unrelated.
> >
> > > +
> > > +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
> > > +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
> >
> > The need for these two changes is also not clear at all from the
> commit message.
> >
> >
> > I've marked the patch as Changes Requested.
> >
> > Regards,
> > Arnout
> >
> > >
> > > ifeq ($(BR2_PACKAGE_FREETYPE),y)
> > > PYTHON_PILLOW_DEPENDENCIES += freetype
> > > @@ -68,22 +71,4 @@ else
> > > PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
> > > endif
> > >
> > > -define PYTHON_PILLOW_BUILD_CMDS
> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> > > - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
> > > - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
> > > - $(PYTHON_PILLOW_BASE_BUILD_OPTS)
> $(PYTHON_PILLOW_BUILD_OPTS)
> > > -endef
> > > -
> > > -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> > > - $(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)
> > > -endef
> > > -
> > > $(eval $(python-package))
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
>
--
Angelo Compagnucci
Software Engineer
angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 (0)42 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|
[-- Attachment #1.2: Type: text/html, Size: 13974 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-21 22:10 ` Angelo Compagnucci
@ 2022-03-21 22:35 ` Angelo Compagnucci
2022-03-21 23:10 ` Angelo Compagnucci
0 siblings, 1 reply; 8+ messages in thread
From: Angelo Compagnucci @ 2022-03-21 22:35 UTC (permalink / raw)
To: James Hilliard; +Cc: Angelo Compagnucci, Asaf Kahlon, buildroot
[-- Attachment #1.1: Type: text/plain, Size: 8184 bytes --]
On Mon, Mar 21, 2022 at 11:10 PM Angelo Compagnucci <
angelo@amarulasolutions.com> wrote:
>
>
> On Mon, Mar 21, 2022 at 10:56 PM James Hilliard <james.hilliard1@gmail.com>
> wrote:
>
>> On Mon, Mar 21, 2022 at 3:32 PM Arnout Vandecappelle <arnout@mind.be>
>> wrote:
>> >
>> > Hi James,
>> >
>> > On 16/03/2022 07:02, James Hilliard wrote:
>> > > Currently pillow doesn't correctly search pkg-config system paths
>> > > for some libraries which can prevent some libraries from being
>> > > properly detected/enabled in pillow.
>> > >
>> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>> > > ---
>> > > ...Search-pkg-config-system-libs-cflags.patch | 33
>> +++++++++++++++++++
>> > > package/python-pillow/python-pillow.mk | 23 +++----------
>> > > 2 files changed, 37 insertions(+), 19 deletions(-)
>> > > create mode 100644
>> package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>> > >
>> > > diff --git
>> a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>> > > new file mode 100644
>> > > index 0000000000..9f979b048f
>> > > --- /dev/null
>> > > +++
>> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>> > > @@ -0,0 +1,33 @@
>> > > +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00
>> 2001
>> > > +From: James Hilliard <james.hilliard1@gmail.com>
>> > > +Date: Tue, 15 Mar 2022 23:31:59 -0600
>> > > +Subject: [PATCH] Search pkg-config system libs/cflags.
>> > > +
>> > > +We need to search the system paths as well from pkg-config for
>> > > +some packages to be found properly.
>> > > +
>> > > +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>> > > +[Upstream status:
>> > > +https://github.com/python-pillow/Pillow/pull/6138]
>> > > +---
>> > > + setup.py | 4 ++--
>> > > + 1 file changed, 2 insertions(+), 2 deletions(-)
>> > > +
>> > > +diff --git a/setup.py b/setup.py
>> > > +index 3468b260..59d65ce2 100755
>> > > +--- a/setup.py
>> > > ++++ b/setup.py
>> > > +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
>> > > + def _pkg_config(name):
>> > > + try:
>> > > + command = os.environ.get("PKG_CONFIG", "pkg-config")
>> > > +- command_libs = [command, "--libs-only-L", name]
>> > > +- command_cflags = [command, "--cflags-only-I", name]
>> > > ++ command_libs = [command, "--libs-only-L",
>> "--keep-system-libs", name]
>> > > ++ command_cflags = [command, "--cflags-only-I",
>> "--keep-system-cflags", name]
>> >
>> > You gave an additional explanation why this is needed in the upstream
>> PR:
>> >
>> > Zlib and anything that has headers in the normal system libs/include
>> folders
>> > seems to not get their headers picked up without this change.
>> >
>> > I think this issue is mostly going to be something people hit when cross
>> > compiling since the prefix based system libs/include folder would
>> probably work
>> > in most of the usual cases(searching in sys.prefix will not work when
>> cross
>> > compiling since it points to the host toolchain prefix rather than the
>> target
>> > toolchain sysroot prefix):
>> >
>> >
>> > However, that implies that either we have to make sure that
>> sys.prefix is set
>> > correctly (i.e. point it to staging instead of host), or pillow is using
>> > sys.prefix incorrectly.
>>
>> I think pillow is using sys.prefix in a way that is not really cross
>> compilation
>> compatible, however using pkg-config with sysm libs/cflags seems to be
>> sufficient
>> for it to pass its non-standard header checks.
>>
>
> I remember having added the --disable-platform-guessing exactly to
> overcome this problem. All the setting should be provided by buildroot.
> Probably, the logic behind this is slightly changed, and the mechanism
> doesn't work anymore. I'll try to have a look.
>
Yes, basically the paths leaks some host directories:
[...]
Checking for include file %s in %s ('libimagequant.h', '/usr/local/include')
Checking for include file %s in %s ('libimagequant.h', '/usr/include')
Looking forward for a fix.
>
>>
>> >
>> > Before that, still, I don't understand how this can be an issue.
>> Unless if
>> > pillow also passes something like -nostdinc, our toolchain wrapper
>> should make
>> > sure that staging/usr/include is in the search path.
>>
>> It's due to pillow having custom header checks like this:
>> https://github.com/python-pillow/Pillow/blob/9.0.1/setup.py#L633
>>
>> >
>> > There also doesn't seem to be an autobuilder failure due to this...
>> But maybe
>> > it builds successfully, just without some optional dependency? Please
>> provide
>> > more details about the failure in the commit message.
>>
>> The failure was getting hidden by the non-standard build/install cmd
>> overrides it
>> would appear, it seemed pillow was getting built for the host instead
>> of the target with
>> those, I didn't investigate in too much detail as those custom
>> build/install overrides
>> are not actually needed.
>>
>> >
>> >
>> >
>> > > + if not DEBUG:
>> > > + command_libs.append("--silence-errors")
>> > > + command_cflags.append("--silence-errors")
>> > > +--
>> > > +2.35.1
>> > > +
>> > > diff --git a/package/python-pillow/python-pillow.mk
>> b/package/python-pillow/python-pillow.mk
>> > > index 901876e0ee..ef677855b2 100644
>> > > --- a/package/python-pillow/python-pillow.mk
>> > > +++ b/package/python-pillow/python-pillow.mk
>> > > @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
>> > > PYTHON_PILLOW_CPE_ID_VENDOR = python
>> > > PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
>> > > PYTHON_PILLOW_SETUP_TYPE = setuptools
>> > > -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
>> > > +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
>> >
>> > This change isn't explained in the commit message, and seems
>> unrelated.
>> >
>> > > +
>> > > +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
>> > > +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
>> >
>> > The need for these two changes is also not clear at all from the
>> commit message.
>> >
>> >
>> > I've marked the patch as Changes Requested.
>> >
>> > Regards,
>> > Arnout
>> >
>> > >
>> > > ifeq ($(BR2_PACKAGE_FREETYPE),y)
>> > > PYTHON_PILLOW_DEPENDENCIES += freetype
>> > > @@ -68,22 +71,4 @@ else
>> > > PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
>> > > endif
>> > >
>> > > -define PYTHON_PILLOW_BUILD_CMDS
>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>> > > - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>> > > - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
>> > > - $(PYTHON_PILLOW_BASE_BUILD_OPTS)
>> $(PYTHON_PILLOW_BUILD_OPTS)
>> > > -endef
>> > > -
>> > > -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>> > > - $(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)
>> > > -endef
>> > > -
>> > > $(eval $(python-package))
>> _______________________________________________
>> buildroot mailing list
>> buildroot@buildroot.org
>> https://lists.buildroot.org/mailman/listinfo/buildroot
>>
>
>
> --
>
> Angelo Compagnucci
>
> Software Engineer
>
> angelo@amarulasolutions.com
> __________________________________
> Amarula Solutions SRL
>
> Via le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 (0)42 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
> [`as] https://www.amarulasolutions.com|
>
--
Angelo Compagnucci
Software Engineer
angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 (0)42 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|
[-- Attachment #1.2: Type: text/html, Size: 18959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-21 22:35 ` Angelo Compagnucci
@ 2022-03-21 23:10 ` Angelo Compagnucci
2022-03-22 18:33 ` James Hilliard
0 siblings, 1 reply; 8+ messages in thread
From: Angelo Compagnucci @ 2022-03-21 23:10 UTC (permalink / raw)
To: James Hilliard; +Cc: Angelo Compagnucci, Asaf Kahlon, buildroot
[-- Attachment #1.1: Type: text/plain, Size: 9325 bytes --]
On Mon, Mar 21, 2022 at 11:35 PM Angelo Compagnucci <
angelo@amarulasolutions.com> wrote:
>
>
> On Mon, Mar 21, 2022 at 11:10 PM Angelo Compagnucci <
> angelo@amarulasolutions.com> wrote:
>
>>
>>
>> On Mon, Mar 21, 2022 at 10:56 PM James Hilliard <
>> james.hilliard1@gmail.com> wrote:
>>
>>> On Mon, Mar 21, 2022 at 3:32 PM Arnout Vandecappelle <arnout@mind.be>
>>> wrote:
>>> >
>>> > Hi James,
>>> >
>>> > On 16/03/2022 07:02, James Hilliard wrote:
>>> > > Currently pillow doesn't correctly search pkg-config system paths
>>> > > for some libraries which can prevent some libraries from being
>>> > > properly detected/enabled in pillow.
>>> > >
>>> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>> > > ---
>>> > > ...Search-pkg-config-system-libs-cflags.patch | 33
>>> +++++++++++++++++++
>>> > > package/python-pillow/python-pillow.mk | 23 +++----------
>>> > > 2 files changed, 37 insertions(+), 19 deletions(-)
>>> > > create mode 100644
>>> package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>> > >
>>> > > diff --git
>>> a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>> > > new file mode 100644
>>> > > index 0000000000..9f979b048f
>>> > > --- /dev/null
>>> > > +++
>>> b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>> > > @@ -0,0 +1,33 @@
>>> > > +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00
>>> 2001
>>> > > +From: James Hilliard <james.hilliard1@gmail.com>
>>> > > +Date: Tue, 15 Mar 2022 23:31:59 -0600
>>> > > +Subject: [PATCH] Search pkg-config system libs/cflags.
>>> > > +
>>> > > +We need to search the system paths as well from pkg-config for
>>> > > +some packages to be found properly.
>>> > > +
>>> > > +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>> > > +[Upstream status:
>>> > > +https://github.com/python-pillow/Pillow/pull/6138]
>>> > > +---
>>> > > + setup.py | 4 ++--
>>> > > + 1 file changed, 2 insertions(+), 2 deletions(-)
>>> > > +
>>> > > +diff --git a/setup.py b/setup.py
>>> > > +index 3468b260..59d65ce2 100755
>>> > > +--- a/setup.py
>>> > > ++++ b/setup.py
>>> > > +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
>>> > > + def _pkg_config(name):
>>> > > + try:
>>> > > + command = os.environ.get("PKG_CONFIG", "pkg-config")
>>> > > +- command_libs = [command, "--libs-only-L", name]
>>> > > +- command_cflags = [command, "--cflags-only-I", name]
>>> > > ++ command_libs = [command, "--libs-only-L",
>>> "--keep-system-libs", name]
>>> > > ++ command_cflags = [command, "--cflags-only-I",
>>> "--keep-system-cflags", name]
>>> >
>>> > You gave an additional explanation why this is needed in the
>>> upstream PR:
>>> >
>>> > Zlib and anything that has headers in the normal system libs/include
>>> folders
>>> > seems to not get their headers picked up without this change.
>>> >
>>> > I think this issue is mostly going to be something people hit when
>>> cross
>>> > compiling since the prefix based system libs/include folder would
>>> probably work
>>> > in most of the usual cases(searching in sys.prefix will not work when
>>> cross
>>> > compiling since it points to the host toolchain prefix rather than the
>>> target
>>> > toolchain sysroot prefix):
>>> >
>>> >
>>> > However, that implies that either we have to make sure that
>>> sys.prefix is set
>>> > correctly (i.e. point it to staging instead of host), or pillow is
>>> using
>>> > sys.prefix incorrectly.
>>>
>>> I think pillow is using sys.prefix in a way that is not really cross
>>> compilation
>>> compatible, however using pkg-config with sysm libs/cflags seems to be
>>> sufficient
>>> for it to pass its non-standard header checks.
>>>
>>
>> I remember having added the --disable-platform-guessing exactly to
>> overcome this problem. All the setting should be provided by buildroot.
>> Probably, the logic behind this is slightly changed, and the mechanism
>> doesn't work anymore. I'll try to have a look.
>>
>
> Yes, basically the paths leaks some host directories:
>
> [...]
> Checking for include file %s in %s ('libimagequant.h',
> '/usr/local/include')
> Checking for include file %s in %s ('libimagequant.h', '/usr/include')
>
> Looking forward for a fix.
>
I came to the conclusion that James setup.py patch is necessary, pillow
doesn't use sys.prefix but builds the paths from the pkg-config output.
Withouth the patch, include and libraries directories are wrong.
It misses a piece anyway
+PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
+PYTHON_PILLOW_INSTALL_TARGET_OPTS = $(PYTHON_PILLOW_BUILD_OPTS) install
target install options should match the build ones to have a correct
library installation.
>
>>
>>>
>>> >
>>> > Before that, still, I don't understand how this can be an issue.
>>> Unless if
>>> > pillow also passes something like -nostdinc, our toolchain wrapper
>>> should make
>>> > sure that staging/usr/include is in the search path.
>>>
>>> It's due to pillow having custom header checks like this:
>>> https://github.com/python-pillow/Pillow/blob/9.0.1/setup.py#L633
>>>
>>> >
>>> > There also doesn't seem to be an autobuilder failure due to this...
>>> But maybe
>>> > it builds successfully, just without some optional dependency? Please
>>> provide
>>> > more details about the failure in the commit message.
>>>
>>> The failure was getting hidden by the non-standard build/install cmd
>>> overrides it
>>> would appear, it seemed pillow was getting built for the host instead
>>> of the target with
>>> those, I didn't investigate in too much detail as those custom
>>> build/install overrides
>>> are not actually needed.
>>>
>>> >
>>> >
>>> >
>>> > > + if not DEBUG:
>>> > > + command_libs.append("--silence-errors")
>>> > > + command_cflags.append("--silence-errors")
>>> > > +--
>>> > > +2.35.1
>>> > > +
>>> > > diff --git a/package/python-pillow/python-pillow.mk
>>> b/package/python-pillow/python-pillow.mk
>>> > > index 901876e0ee..ef677855b2 100644
>>> > > --- a/package/python-pillow/python-pillow.mk
>>> > > +++ b/package/python-pillow/python-pillow.mk
>>> > > @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
>>> > > PYTHON_PILLOW_CPE_ID_VENDOR = python
>>> > > PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
>>> > > PYTHON_PILLOW_SETUP_TYPE = setuptools
>>> > > -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
>>> > > +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
>>> >
>>> > This change isn't explained in the commit message, and seems
>>> unrelated.
>>> >
>>> > > +
>>> > > +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
>>> > > +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
>>> >
>>> > The need for these two changes is also not clear at all from the
>>> commit message.
>>> >
>>> >
>>> > I've marked the patch as Changes Requested.
>>> >
>>> > Regards,
>>> > Arnout
>>> >
>>> > >
>>> > > ifeq ($(BR2_PACKAGE_FREETYPE),y)
>>> > > PYTHON_PILLOW_DEPENDENCIES += freetype
>>> > > @@ -68,22 +71,4 @@ else
>>> > > PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
>>> > > endif
>>> > >
>>> > > -define PYTHON_PILLOW_BUILD_CMDS
>>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>>> > > - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>>> > > - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext
>>> \
>>> > > - $(PYTHON_PILLOW_BASE_BUILD_OPTS)
>>> $(PYTHON_PILLOW_BUILD_OPTS)
>>> > > -endef
>>> > > -
>>> > > -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
>>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>>> > > - $(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)
>>> > > -endef
>>> > > -
>>> > > $(eval $(python-package))
>>> _______________________________________________
>>> buildroot mailing list
>>> buildroot@buildroot.org
>>> https://lists.buildroot.org/mailman/listinfo/buildroot
>>>
>>
>>
>> --
>>
>> Angelo Compagnucci
>>
>> Software Engineer
>>
>> angelo@amarulasolutions.com
>> __________________________________
>> Amarula Solutions SRL
>>
>> Via le Canevare 30, 31100 Treviso, Veneto, IT
>>
>> T. +39 (0)42 243 5310
>> info@amarulasolutions.com
>>
>> www.amarulasolutions.com
>> [`as] https://www.amarulasolutions.com|
>>
>
>
> --
>
> Angelo Compagnucci
>
> Software Engineer
>
> angelo@amarulasolutions.com
> __________________________________
> Amarula Solutions SRL
>
> Via le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 (0)42 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
> [`as] https://www.amarulasolutions.com|
>
--
Angelo Compagnucci
Software Engineer
angelo@amarulasolutions.com
__________________________________
Amarula Solutions SRL
Via le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 (0)42 243 5310
info@amarulasolutions.com
www.amarulasolutions.com
[`as] https://www.amarulasolutions.com|
[-- Attachment #1.2: Type: text/html, Size: 24154 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths
2022-03-21 23:10 ` Angelo Compagnucci
@ 2022-03-22 18:33 ` James Hilliard
0 siblings, 0 replies; 8+ messages in thread
From: James Hilliard @ 2022-03-22 18:33 UTC (permalink / raw)
To: Angelo Compagnucci; +Cc: Angelo Compagnucci, Asaf Kahlon, buildroot
On Mon, Mar 21, 2022 at 5:10 PM Angelo Compagnucci
<angelo@amarulasolutions.com> wrote:
>
>
>
> On Mon, Mar 21, 2022 at 11:35 PM Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>>
>>
>>
>> On Mon, Mar 21, 2022 at 11:10 PM Angelo Compagnucci <angelo@amarulasolutions.com> wrote:
>>>
>>>
>>>
>>> On Mon, Mar 21, 2022 at 10:56 PM James Hilliard <james.hilliard1@gmail.com> wrote:
>>>>
>>>> On Mon, Mar 21, 2022 at 3:32 PM Arnout Vandecappelle <arnout@mind.be> wrote:
>>>> >
>>>> > Hi James,
>>>> >
>>>> > On 16/03/2022 07:02, James Hilliard wrote:
>>>> > > Currently pillow doesn't correctly search pkg-config system paths
>>>> > > for some libraries which can prevent some libraries from being
>>>> > > properly detected/enabled in pillow.
>>>> > >
>>>> > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>>> > > ---
>>>> > > ...Search-pkg-config-system-libs-cflags.patch | 33 +++++++++++++++++++
>>>> > > package/python-pillow/python-pillow.mk | 23 +++----------
>>>> > > 2 files changed, 37 insertions(+), 19 deletions(-)
>>>> > > create mode 100644 package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>>> > >
>>>> > > diff --git a/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>>> > > new file mode 100644
>>>> > > index 0000000000..9f979b048f
>>>> > > --- /dev/null
>>>> > > +++ b/package/python-pillow/0001-Search-pkg-config-system-libs-cflags.patch
>>>> > > @@ -0,0 +1,33 @@
>>>> > > +From 89c9c347bb7437d5ea559930e315f42a0236761f Mon Sep 17 00:00:00 2001
>>>> > > +From: James Hilliard <james.hilliard1@gmail.com>
>>>> > > +Date: Tue, 15 Mar 2022 23:31:59 -0600
>>>> > > +Subject: [PATCH] Search pkg-config system libs/cflags.
>>>> > > +
>>>> > > +We need to search the system paths as well from pkg-config for
>>>> > > +some packages to be found properly.
>>>> > > +
>>>> > > +Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
>>>> > > +[Upstream status:
>>>> > > +https://github.com/python-pillow/Pillow/pull/6138]
>>>> > > +---
>>>> > > + setup.py | 4 ++--
>>>> > > + 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> > > +
>>>> > > +diff --git a/setup.py b/setup.py
>>>> > > +index 3468b260..59d65ce2 100755
>>>> > > +--- a/setup.py
>>>> > > ++++ b/setup.py
>>>> > > +@@ -252,8 +252,8 @@ def _cmd_exists(cmd):
>>>> > > + def _pkg_config(name):
>>>> > > + try:
>>>> > > + command = os.environ.get("PKG_CONFIG", "pkg-config")
>>>> > > +- command_libs = [command, "--libs-only-L", name]
>>>> > > +- command_cflags = [command, "--cflags-only-I", name]
>>>> > > ++ command_libs = [command, "--libs-only-L", "--keep-system-libs", name]
>>>> > > ++ command_cflags = [command, "--cflags-only-I", "--keep-system-cflags", name]
>>>> >
>>>> > You gave an additional explanation why this is needed in the upstream PR:
>>>> >
>>>> > Zlib and anything that has headers in the normal system libs/include folders
>>>> > seems to not get their headers picked up without this change.
>>>> >
>>>> > I think this issue is mostly going to be something people hit when cross
>>>> > compiling since the prefix based system libs/include folder would probably work
>>>> > in most of the usual cases(searching in sys.prefix will not work when cross
>>>> > compiling since it points to the host toolchain prefix rather than the target
>>>> > toolchain sysroot prefix):
>>>> >
>>>> >
>>>> > However, that implies that either we have to make sure that sys.prefix is set
>>>> > correctly (i.e. point it to staging instead of host), or pillow is using
>>>> > sys.prefix incorrectly.
>>>>
>>>> I think pillow is using sys.prefix in a way that is not really cross compilation
>>>> compatible, however using pkg-config with sysm libs/cflags seems to be
>>>> sufficient
>>>> for it to pass its non-standard header checks.
>>>
>>>
>>> I remember having added the --disable-platform-guessing exactly to overcome this problem. All the setting should be provided by buildroot. Probably, the logic behind this is slightly changed, and the mechanism doesn't work anymore. I'll try to have a look.
>>
>>
>> Yes, basically the paths leaks some host directories:
>>
>> [...]
>> Checking for include file %s in %s ('libimagequant.h', '/usr/local/include')
>> Checking for include file %s in %s ('libimagequant.h', '/usr/include')
>>
>> Looking forward for a fix.
>
>
> I came to the conclusion that James setup.py patch is necessary, pillow doesn't use sys.prefix but builds the paths from the pkg-config output. Withouth the patch, include and libraries directories are wrong.
> It misses a piece anyway
>
> +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
> +PYTHON_PILLOW_INSTALL_TARGET_OPTS = $(PYTHON_PILLOW_BUILD_OPTS) install
https://patchwork.ozlabs.org/project/buildroot/patch/20220322182952.935255-1-james.hilliard1@gmail.com/
The install part isn't needed here since it will still get passed by
_BASE_INSTALL_TARGET_CMD.
See:
https://github.com/buildroot/buildroot/blob/eaa8fcf546d84e38990566e49f4730c530d2577c/package/pkg-python.mk#L199
https://github.com/buildroot/buildroot/blob/eaa8fcf546d84e38990566e49f4730c530d2577c/package/pkg-python.mk#L285
>
> target install options should match the build ones to have a correct library installation.
>
>>
>>>
>>>>
>>>>
>>>> >
>>>> > Before that, still, I don't understand how this can be an issue. Unless if
>>>> > pillow also passes something like -nostdinc, our toolchain wrapper should make
>>>> > sure that staging/usr/include is in the search path.
>>>>
>>>> It's due to pillow having custom header checks like this:
>>>> https://github.com/python-pillow/Pillow/blob/9.0.1/setup.py#L633
>>>>
>>>> >
>>>> > There also doesn't seem to be an autobuilder failure due to this... But maybe
>>>> > it builds successfully, just without some optional dependency? Please provide
>>>> > more details about the failure in the commit message.
>>>>
>>>> The failure was getting hidden by the non-standard build/install cmd
>>>> overrides it
>>>> would appear, it seemed pillow was getting built for the host instead
>>>> of the target with
>>>> those, I didn't investigate in too much detail as those custom
>>>> build/install overrides
>>>> are not actually needed.
>>>>
>>>> >
>>>> >
>>>> >
>>>> > > + if not DEBUG:
>>>> > > + command_libs.append("--silence-errors")
>>>> > > + command_cflags.append("--silence-errors")
>>>> > > +--
>>>> > > +2.35.1
>>>> > > +
>>>> > > diff --git a/package/python-pillow/python-pillow.mk b/package/python-pillow/python-pillow.mk
>>>> > > index 901876e0ee..ef677855b2 100644
>>>> > > --- a/package/python-pillow/python-pillow.mk
>>>> > > +++ b/package/python-pillow/python-pillow.mk
>>>> > > @@ -12,7 +12,10 @@ PYTHON_PILLOW_LICENSE_FILES = LICENSE
>>>> > > PYTHON_PILLOW_CPE_ID_VENDOR = python
>>>> > > PYTHON_PILLOW_CPE_ID_PRODUCT = pillow
>>>> > > PYTHON_PILLOW_SETUP_TYPE = setuptools
>>>> > > -PYTHON_PILLOW_BUILD_OPTS = --disable-platform-guessing
>>>> > > +PYTHON_PILLOW_ENV = MAX_CONCURRENCY="$(PARALLEL_JOBS)"
>>>> >
>>>> > This change isn't explained in the commit message, and seems unrelated.
>>>> >
>>>> > > +
>>>> > > +PYTHON_PILLOW_DEPENDENCIES = host-pkgconf
>>>> > > +PYTHON_PILLOW_BUILD_OPTS = build_ext --disable-platform-guessing
>>>> >
>>>> > The need for these two changes is also not clear at all from the commit message.
>>>> >
>>>> >
>>>> > I've marked the patch as Changes Requested.
>>>> >
>>>> > Regards,
>>>> > Arnout
>>>> >
>>>> > >
>>>> > > ifeq ($(BR2_PACKAGE_FREETYPE),y)
>>>> > > PYTHON_PILLOW_DEPENDENCIES += freetype
>>>> > > @@ -68,22 +71,4 @@ else
>>>> > > PYTHON_PILLOW_BUILD_OPTS += --disable-webp --disable-webpmux
>>>> > > endif
>>>> > >
>>>> > > -define PYTHON_PILLOW_BUILD_CMDS
>>>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>>>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>>>> > > - $(PYTHON_PILLOW_BASE_ENV) $(PYTHON_PILLOW_ENV) \
>>>> > > - $(PYTHON_PILLOW_PYTHON_INTERPRETER) setup.py build_ext \
>>>> > > - $(PYTHON_PILLOW_BASE_BUILD_OPTS) $(PYTHON_PILLOW_BUILD_OPTS)
>>>> > > -endef
>>>> > > -
>>>> > > -define PYTHON_PILLOW_INSTALL_TARGET_CMDS
>>>> > > - cd $(PYTHON_PILLOW_BUILDDIR); \
>>>> > > - PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
>>>> > > - $(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)
>>>> > > -endef
>>>> > > -
>>>> > > $(eval $(python-package))
>>>> _______________________________________________
>>>> buildroot mailing list
>>>> buildroot@buildroot.org
>>>> https://lists.buildroot.org/mailman/listinfo/buildroot
>>>
>>>
>>>
>>> --
>>>
>>> Angelo Compagnucci
>>>
>>> Software Engineer
>>>
>>> angelo@amarulasolutions.com
>>> __________________________________
>>> Amarula Solutions SRL
>>>
>>> Via le Canevare 30, 31100 Treviso, Veneto, IT
>>>
>>> T. +39 (0)42 243 5310
>>> info@amarulasolutions.com
>>>
>>> www.amarulasolutions.com
>>>
>>> [`as] https://www.amarulasolutions.com|
>>
>>
>>
>> --
>>
>> Angelo Compagnucci
>>
>> Software Engineer
>>
>> angelo@amarulasolutions.com
>> __________________________________
>> Amarula Solutions SRL
>>
>> Via le Canevare 30, 31100 Treviso, Veneto, IT
>>
>> T. +39 (0)42 243 5310
>> info@amarulasolutions.com
>>
>> www.amarulasolutions.com
>>
>> [`as] https://www.amarulasolutions.com|
>
>
>
> --
>
> Angelo Compagnucci
>
> Software Engineer
>
> angelo@amarulasolutions.com
> __________________________________
> Amarula Solutions SRL
>
> Via le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 (0)42 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
>
> [`as] https://www.amarulasolutions.com|
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-03-22 18:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 6:02 [Buildroot] [PATCH 1/1] package/python-pillow: fix pkg-config search paths James Hilliard
2022-03-20 13:14 ` Angelo Compagnucci
2022-03-21 21:32 ` Arnout Vandecappelle
2022-03-21 21:56 ` James Hilliard
2022-03-21 22:10 ` Angelo Compagnucci
2022-03-21 22:35 ` Angelo Compagnucci
2022-03-21 23:10 ` Angelo Compagnucci
2022-03-22 18:33 ` James Hilliard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.