| Message ID | 20200414102621.18804-1-asafka7@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] package/python-argon2-cffi: use -msse2 only when supported | expand |
Hello Asaf, On Tue, 14 Apr 2020 13:26:21 +0300 Asaf Kahlon <asafka7@gmail.com> wrote: > The package adds the '-msse2' flag according to the result of the > pythonic "platform.machine()" statement, which runs on the host > and doesn't necessarily reflects the existence of this flag on > the target compiler. > Hence, we'll set the 'optimzed' variable in setup.py only when optimzed -> optimized > +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG optimized > + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py > +endef > + > +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG optimized But can't we do better here ? Check if the compiler supports the -msse2 flag ? See https://github.com/pybind/python_example/blob/master/setup.py#L38 for an example on how to do that. Thanks! Thomas
On Wed, Apr 15, 2020 at 2:10 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Asaf, > > On Tue, 14 Apr 2020 13:26:21 +0300 > Asaf Kahlon <asafka7@gmail.com> wrote: > > > The package adds the '-msse2' flag according to the result of the > > pythonic "platform.machine()" statement, which runs on the host > > and doesn't necessarily reflects the existence of this flag on > > the target compiler. > > Hence, we'll set the 'optimzed' variable in setup.py only when > > optimzed -> optimized > > > +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > optimized > > > + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py > > +endef > > + > > +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > optimized > > But can't we do better here ? Check if the compiler supports the -msse2 > flag ? See > https://github.com/pybind/python_example/blob/master/setup.py#L38 for > an example on how to do that. Ah, that looks better, I had submitted https://github.com/hynek/argon2-cffi/pull/59 to avoid having to patch setup.py, I'll see if I can get that working. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Wed, Apr 15, 2020 at 2:20 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > On Wed, Apr 15, 2020 at 2:10 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > > > Hello Asaf, > > > > On Tue, 14 Apr 2020 13:26:21 +0300 > > Asaf Kahlon <asafka7@gmail.com> wrote: > > > > > The package adds the '-msse2' flag according to the result of the > > > pythonic "platform.machine()" statement, which runs on the host > > > and doesn't necessarily reflects the existence of this flag on > > > the target compiler. > > > Hence, we'll set the 'optimzed' variable in setup.py only when > > > > optimzed -> optimized > > > > > +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > > > optimized > > > > > + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py > > > +endef > > > + > > > +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > > > optimized > > > > But can't we do better here ? Check if the compiler supports the -msse2 > > flag ? See > > https://github.com/pybind/python_example/blob/master/setup.py#L38 for > > an example on how to do that. > Ah, that looks better, I had submitted > https://github.com/hynek/argon2-cffi/pull/59 > to avoid having to patch setup.py, I'll see if I can get that working. Oh, just noticed we'll probably still need both since argon2-cffi supports python2. > > > > Thanks! > > > > Thomas > > -- > > Thomas Petazzoni, CTO, Bootlin > > Embedded Linux and Kernel engineering > > https://bootlin.com
On Wed, Apr 15, 2020 at 2:22 PM James Hilliard <james.hilliard1@gmail.com> wrote: > > On Wed, Apr 15, 2020 at 2:20 PM James Hilliard > <james.hilliard1@gmail.com> wrote: > > > > On Wed, Apr 15, 2020 at 2:10 PM Thomas Petazzoni > > <thomas.petazzoni@bootlin.com> wrote: > > > > > > Hello Asaf, > > > > > > On Tue, 14 Apr 2020 13:26:21 +0300 > > > Asaf Kahlon <asafka7@gmail.com> wrote: > > > > > > > The package adds the '-msse2' flag according to the result of the > > > > pythonic "platform.machine()" statement, which runs on the host > > > > and doesn't necessarily reflects the existence of this flag on > > > > the target compiler. > > > > Hence, we'll set the 'optimzed' variable in setup.py only when > > > > > > optimzed -> optimized > > > > > > > +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > > > > > optimized > > > > > > > + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py > > > > +endef > > > > + > > > > +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > > > > > > optimized > > > > > > But can't we do better here ? Check if the compiler supports the -msse2 > > > flag ? See > > > https://github.com/pybind/python_example/blob/master/setup.py#L38 for > > > an example on how to do that. > > Ah, that looks better, I had submitted > > https://github.com/hynek/argon2-cffi/pull/59 > > to avoid having to patch setup.py, I'll see if I can get that working. > Oh, just noticed we'll probably still need both since argon2-cffi > supports python2. Nevermind, that comment appears to be totally inaccurate as Python 3.6's CCompiler does not actually have a has_flag method at all, however we can just include the function which seems to work. I've updated my pull request with a reworked version of has_flag which seems to work. > > > > > > Thanks! > > > > > > Thomas > > > -- > > > Thomas Petazzoni, CTO, Bootlin > > > Embedded Linux and Kernel engineering > > > https://bootlin.com
Hello, On Wed, 15 Apr 2020 16:00:58 -0600 James Hilliard <james.hilliard1@gmail.com> wrote: > Nevermind, that comment appears to be totally inaccurate as Python > 3.6's CCompiler > does not actually have a has_flag method at all, however we can just include the > function which seems to work. I've updated my pull request with a reworked > version of has_flag which seems to work. Do you have some news about the upstream patch to fix this issue ? It would be good to fix it in Buildroot, as the build continues to fail. Thanks! Thomas
On Sun, Apr 19, 2020 at 8:00 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Wed, 15 Apr 2020 16:00:58 -0600 > James Hilliard <james.hilliard1@gmail.com> wrote: > > > Nevermind, that comment appears to be totally inaccurate as Python > > 3.6's CCompiler > > does not actually have a has_flag method at all, however we can just include the > > function which seems to work. I've updated my pull request with a reworked > > version of has_flag which seems to work. > > Do you have some news about the upstream patch to fix this issue ? It > would be good to fix it in Buildroot, as the build continues to fail. I was waiting on upstream to merge the patch I sent but it should fix this. https://github.com/hynek/argon2-cffi/pull/59 > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
I think we should maybe just merge this for now so that autobuilders stop failing, once my upstream PR is merged and the package is updated we can then remove this workaround. On Tue, Apr 14, 2020 at 4:27 AM Asaf Kahlon <asafka7@gmail.com> wrote: > > The package adds the '-msse2' flag according to the result of the > pythonic "platform.machine()" statement, which runs on the host > and doesn't necessarily reflects the existence of this flag on > the target compiler. > Hence, we'll set the 'optimzed' variable in setup.py only when > we know SSE2 is supported. > > Fixes: > http://autobuild.buildroot.net/results/8c8aee8c8a0062575f489042bb11cc8515cbe48c/ > > Signed-off-by: Asaf Kahlon <asafka7@gmail.com> > --- > package/python-argon2-cffi/python-argon2-cffi.mk | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/package/python-argon2-cffi/python-argon2-cffi.mk b/package/python-argon2-cffi/python-argon2-cffi.mk > index 099574e9c3..9f73e75c1e 100644 > --- a/package/python-argon2-cffi/python-argon2-cffi.mk > +++ b/package/python-argon2-cffi/python-argon2-cffi.mk > @@ -12,4 +12,10 @@ PYTHON_ARGON2_CFFI_LICENSE = MIT > PYTHON_ARGON2_CFFI_LICENSE_FILES = LICENSE > PYTHON_ARGON2_CFFI_DEPENDENCIES = host-python-cffi > > +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py > +endef > + > +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG > + > $(eval $(python-package)) > -- > 2.26.0 >
diff --git a/package/python-argon2-cffi/python-argon2-cffi.mk b/package/python-argon2-cffi/python-argon2-cffi.mk index 099574e9c3..9f73e75c1e 100644 --- a/package/python-argon2-cffi/python-argon2-cffi.mk +++ b/package/python-argon2-cffi/python-argon2-cffi.mk @@ -12,4 +12,10 @@ PYTHON_ARGON2_CFFI_LICENSE = MIT PYTHON_ARGON2_CFFI_LICENSE_FILES = LICENSE PYTHON_ARGON2_CFFI_DEPENDENCIES = host-python-cffi +define PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG + $(SED) 's;^\(optimized = *\)\(.*\);\1$(if $(BR2_X86_CPU_HAS_SSE2),True,False);' $(@D)/setup.py +endef + +PYTHON_ARGON2_CFFI_POST_PATCH_HOOKS += PYTHON_ARGON2_CFFI_SET_OPTIMZED_FLAG + $(eval $(python-package))
The package adds the '-msse2' flag according to the result of the pythonic "platform.machine()" statement, which runs on the host and doesn't necessarily reflects the existence of this flag on the target compiler. Hence, we'll set the 'optimzed' variable in setup.py only when we know SSE2 is supported. Fixes: http://autobuild.buildroot.net/results/8c8aee8c8a0062575f489042bb11cc8515cbe48c/ Signed-off-by: Asaf Kahlon <asafka7@gmail.com> --- package/python-argon2-cffi/python-argon2-cffi.mk | 6 ++++++ 1 file changed, 6 insertions(+)