diff mbox

package/qpid-proton: needs host-python2

Message ID 1436873894-1080-1-git-send-email-yann.morin.1998@free.fr
State Changes Requested
Headers show

Commit Message

Yann E. MORIN July 14, 2015, 11:38 a.m. UTC
qpid-proton is using constructs that are not valid in python3.
qpid-proton is hardcoding calls to /usr/bin/python, which can be
whatever version.

So, fix the scripts to use '/usr/bin/env python' so it finds it in the
PATH, and add a dependency to host-python.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 package/qpid-proton/0002-fix-python.patch | 22 ++++++++++++++++++++++
 package/qpid-proton/qpid-proton.mk        |  2 ++
 2 files changed, 24 insertions(+)
 create mode 100644 package/qpid-proton/0002-fix-python.patch

Comments

Baruch Siach July 14, 2015, 11:48 a.m. UTC | #1
Hi Yann,

On Tue, Jul 14, 2015 at 01:38:14PM +0200, Yann E. MORIN wrote:
> qpid-proton is using constructs that are not valid in python3.
> qpid-proton is hardcoding calls to /usr/bin/python, which can be
> whatever version.

The manual, under "Mandatory packages", currently lists "python (version 2.6 
or 2.7)". Should we change that to "python (version 2.6 or any later)"?

> So, fix the scripts to use '/usr/bin/env python' so it finds it in the
> PATH, and add a dependency to host-python.

My interpretation of the cmake log is that cmake uses the python interpreter 
it detects to run the scripts. In the failed cases cmake detected host-python3 
because host-python3 happened to build before qpid-proton. cmake did not use 
the distro installed host python even though it's python2 (most likely). So 
I'm not sure patching the scripts is necessary.

baruch

> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Baruch Siach <baruch@tkos.co.il>
> ---
>  package/qpid-proton/0002-fix-python.patch | 22 ++++++++++++++++++++++
>  package/qpid-proton/qpid-proton.mk        |  2 ++
>  2 files changed, 24 insertions(+)
>  create mode 100644 package/qpid-proton/0002-fix-python.patch
> 
> diff --git a/package/qpid-proton/0002-fix-python.patch b/package/qpid-proton/0002-fix-python.patch
> new file mode 100644
> index 0000000..c6f57e8
> --- /dev/null
> +++ b/package/qpid-proton/0002-fix-python.patch
> @@ -0,0 +1,22 @@
> +Those scripts are not pytrhon3-compliant, use python2
> +
> +Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> +
> +diff -durN qpid-proton-0.9.1.orig/proton-c/src/codec/encodings.h.py qpid-proton-0.9.1/proton-c/src/codec/encodings.h.py
> +--- qpid-proton-0.9.1.orig/proton-c/src/codec/encodings.h.py	2015-04-29 21:17:49.000000000 +0200
> ++++ qpid-proton-0.9.1/proton-c/src/codec/encodings.h.py	2015-07-14 13:20:40.798356160 +0200
> +@@ -1,4 +1,4 @@
> +-#!/usr/bin/python
> ++#!/usr/bin/env python2
> + #
> + # Licensed to the Apache Software Foundation (ASF) under one
> + # or more contributor license agreements.  See the NOTICE file
> +diff -durN qpid-proton-0.9.1.orig/proton-c/src/protocol.h.py qpid-proton-0.9.1/proton-c/src/protocol.h.py
> +--- qpid-proton-0.9.1.orig/proton-c/src/protocol.h.py	2015-04-29 21:17:49.000000000 +0200
> ++++ qpid-proton-0.9.1/proton-c/src/protocol.h.py	2015-07-14 13:21:19.750824925 +0200
> +@@ -1,4 +1,4 @@
> +-#!/usr/bin/python
> ++#!/usr/bin/env python2
> + #
> + # Licensed to the Apache Software Foundation (ASF) under one
> + # or more contributor license agreements.  See the NOTICE file
> diff --git a/package/qpid-proton/qpid-proton.mk b/package/qpid-proton/qpid-proton.mk
> index eae4589..cb06d97 100644
> --- a/package/qpid-proton/qpid-proton.mk
> +++ b/package/qpid-proton/qpid-proton.mk
> @@ -11,8 +11,10 @@ QPID_PROTON_LICENSE = Apache-2.0
>  QPID_PROTON_LICENSE_FILES = LICENSE
>  QPID_PROTON_INSTALL_STAGING = YES
>  QPID_PROTON_DEPENDENCIES = \
> +	host-python \
>  	util-linux \
>  	$(if $(BR2_PACKAGE_OPENSSL),openssl)
> +
>  QPID_PROTON_CONF_OPTS = \
>  	-DBUILD_JAVA=OFF \
>  	-DENABLE_VALGRIND=OFF \
Thomas Petazzoni July 14, 2015, 11:56 a.m. UTC | #2
Dear Baruch Siach,

On Tue, 14 Jul 2015 14:48:04 +0300, Baruch Siach wrote:

> My interpretation of the cmake log is that cmake uses the python interpreter 
> it detects to run the scripts. In the failed cases cmake detected host-python3 
> because host-python3 happened to build before qpid-proton. cmake did not use 
> the distro installed host python even though it's python2 (most likely). So 
> I'm not sure patching the scripts is necessary.

It is necessary if your host machine has a Python 3 interpreter and not
a Python 2 interpreter. In this case the scripts will
run /usr/bin/python (because it's hardcoded in their source code), and
this will run Python 3.

So Yann's solution looks good. Am I missing something?

Thanks,

Thomas
Baruch Siach July 14, 2015, 12:03 p.m. UTC | #3
Hi Thomas,

On Tue, Jul 14, 2015 at 01:56:27PM +0200, Thomas Petazzoni wrote:
> On Tue, 14 Jul 2015 14:48:04 +0300, Baruch Siach wrote:
> > My interpretation of the cmake log is that cmake uses the python 
> > interpreter it detects to run the scripts. In the failed cases cmake 
> > detected host-python3 because host-python3 happened to build before 
> > qpid-proton. cmake did not use the distro installed host python even 
> > though it's python2 (most likely). So I'm not sure patching the scripts is 
> > necessary.
> 
> It is necessary if your host machine has a Python 3 interpreter and not
> a Python 2 interpreter. In this case the scripts will
> run /usr/bin/python (because it's hardcoded in their source code), and
> this will run Python 3.
> 
> So Yann's solution looks good. Am I missing something?

My point is that in the failed cases /usr/bin/python was not used, otherwise 
the build would have succeeded. It seems that cmake uses the python 
interpreter it detects to run the scripts. So patching the scripts doesn't 
change anything by itself, IIUC.

Related to this, what do you thing about changing the host python requirement 
in the manual?

baruch
Yann E. MORIN July 14, 2015, 12:42 p.m. UTC | #4
Baruch, All,

On 2015-07-14 15:03 +0300, Baruch Siach spake thusly:
> On Tue, Jul 14, 2015 at 01:56:27PM +0200, Thomas Petazzoni wrote:
> > On Tue, 14 Jul 2015 14:48:04 +0300, Baruch Siach wrote:
> > > My interpretation of the cmake log is that cmake uses the python 
> > > interpreter it detects to run the scripts. In the failed cases cmake 
> > > detected host-python3 because host-python3 happened to build before 
> > > qpid-proton. cmake did not use the distro installed host python even 
> > > though it's python2 (most likely). So I'm not sure patching the scripts is 
> > > necessary.
> > 
> > It is necessary if your host machine has a Python 3 interpreter and not
> > a Python 2 interpreter. In this case the scripts will
> > run /usr/bin/python (because it's hardcoded in their source code), and
> > this will run Python 3.
> > 
> > So Yann's solution looks good. Am I missing something?
> 
> My point is that in the failed cases /usr/bin/python was not used, otherwise 
> the build would have succeeded. It seems that cmake uses the python 
> interpreter it detects to run the scripts. So patching the scripts doesn't 
> change anything by itself, IIUC.

OK, I know understansd what you meant. I'll check that. Thanks for the
explanations! ;-)

Regards,
Yann E. MORIN.
Yann E. MORIN July 14, 2015, 1:59 p.m. UTC | #5
Baruch, All,

On 2015-07-14 15:03 +0300, Baruch Siach spake thusly:
> On Tue, Jul 14, 2015 at 01:56:27PM +0200, Thomas Petazzoni wrote:
> > On Tue, 14 Jul 2015 14:48:04 +0300, Baruch Siach wrote:
> > > My interpretation of the cmake log is that cmake uses the python 
> > > interpreter it detects to run the scripts. In the failed cases cmake 
> > > detected host-python3 because host-python3 happened to build before 
> > > qpid-proton. cmake did not use the distro installed host python even 
> > > though it's python2 (most likely). So I'm not sure patching the scripts is 
> > > necessary.
> > 
> > It is necessary if your host machine has a Python 3 interpreter and not
> > a Python 2 interpreter. In this case the scripts will
> > run /usr/bin/python (because it's hardcoded in their source code), and
> > this will run Python 3.
> > 
> > So Yann's solution looks good. Am I missing something?
> 
> My point is that in the failed cases /usr/bin/python was not used, otherwise 
> the build would have succeeded. It seems that cmake uses the python 
> interpreter it detects to run the scripts. So patching the scripts doesn't 
> change anything by itself, IIUC.

OK, so you were right and wrong at the same time! ;-)

In fact, if we just add host-python as a dependency, and we do not have
a target python3 package, then we have no $(HOST_DIR)/usr/bin/python ,
only $(HOST_DIR)/usr/bin/python2 so cmake won't find it, and will
fallback to using the host's python, and we're back to square-one.

So, we have to tell cmake what python to use, with something like
  -DPYTHON_EXECUTABLE=$(HOST_DIR)/usr/bin/python2

And now, it works! :-)   (patch incoming shortly)

Thanks for directing me in the right direction! :-)

Regards,
Yann E. MORIN.
Thomas Petazzoni July 14, 2015, 3:43 p.m. UTC | #6
Dear Baruch Siach,

On Tue, 14 Jul 2015 15:03:25 +0300, Baruch Siach wrote:

> Related to this, what do you thing about changing the host python requirement 
> in the manual?

Yes, sounds good. Samuel is using Buildroot with both Python 2 and
Python 3 installed on his machine, but Python 3 being the default one.
However, I don't know if anyone has tested with a pure Python 3
installation. But it _should_ work. If not that's a bug that we should
fix.

Thomas
diff mbox

Patch

diff --git a/package/qpid-proton/0002-fix-python.patch b/package/qpid-proton/0002-fix-python.patch
new file mode 100644
index 0000000..c6f57e8
--- /dev/null
+++ b/package/qpid-proton/0002-fix-python.patch
@@ -0,0 +1,22 @@ 
+Those scripts are not pytrhon3-compliant, use python2
+
+Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
+
+diff -durN qpid-proton-0.9.1.orig/proton-c/src/codec/encodings.h.py qpid-proton-0.9.1/proton-c/src/codec/encodings.h.py
+--- qpid-proton-0.9.1.orig/proton-c/src/codec/encodings.h.py	2015-04-29 21:17:49.000000000 +0200
++++ qpid-proton-0.9.1/proton-c/src/codec/encodings.h.py	2015-07-14 13:20:40.798356160 +0200
+@@ -1,4 +1,4 @@
+-#!/usr/bin/python
++#!/usr/bin/env python2
+ #
+ # Licensed to the Apache Software Foundation (ASF) under one
+ # or more contributor license agreements.  See the NOTICE file
+diff -durN qpid-proton-0.9.1.orig/proton-c/src/protocol.h.py qpid-proton-0.9.1/proton-c/src/protocol.h.py
+--- qpid-proton-0.9.1.orig/proton-c/src/protocol.h.py	2015-04-29 21:17:49.000000000 +0200
++++ qpid-proton-0.9.1/proton-c/src/protocol.h.py	2015-07-14 13:21:19.750824925 +0200
+@@ -1,4 +1,4 @@
+-#!/usr/bin/python
++#!/usr/bin/env python2
+ #
+ # Licensed to the Apache Software Foundation (ASF) under one
+ # or more contributor license agreements.  See the NOTICE file
diff --git a/package/qpid-proton/qpid-proton.mk b/package/qpid-proton/qpid-proton.mk
index eae4589..cb06d97 100644
--- a/package/qpid-proton/qpid-proton.mk
+++ b/package/qpid-proton/qpid-proton.mk
@@ -11,8 +11,10 @@  QPID_PROTON_LICENSE = Apache-2.0
 QPID_PROTON_LICENSE_FILES = LICENSE
 QPID_PROTON_INSTALL_STAGING = YES
 QPID_PROTON_DEPENDENCIES = \
+	host-python \
 	util-linux \
 	$(if $(BR2_PACKAGE_OPENSSL),openssl)
+
 QPID_PROTON_CONF_OPTS = \
 	-DBUILD_JAVA=OFF \
 	-DENABLE_VALGRIND=OFF \