You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2018/05/25 13:01:02 UTC
Review Request 67322: Added Python 2 check for Python bindings when
using `configure`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67322/
-----------------------------------------------------------
Review request for mesos and Benjamin Bannier.
Bugs: MESOS-8955
https://issues.apache.org/jira/browse/MESOS-8955
Repository: mesos
Description
-------
The Python bindings work with Python 2. If Python 3 is set as
the default `python` on the system, they should not be built.
This check in `configure.ac` ensures that the Python version is
not more than 2.7. This is similar to the current check we have
for the Python CLI.
CMake does not offer an option to build the Python bindings thus
we only need to add the check for autoconf users.
Diffs
-----
configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
Diff: https://reviews.apache.org/r/67322/diff/1/
Testing
-------
Thanks,
Armand Grillet
Re: Review Request 67322: Added Python 2 check for Python bindings
when using `configure`.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67322/#review204045
-----------------------------------------------------------
configure.ac
Line 2247 (original), 2247 (patched)
<https://reviews.apache.org/r/67322/#comment286438>
`AC_PYTHON_DEVEL` looks for headers for `PYTHON_VERSION`, and otherwise interprets its arg as a version selector. With the given selector we might pick a header from a version newer than the interpreter selected with `AM_PATH_PYTHON` (e.g., if a user does not set `PYTHON_VERSION`, has `PYTHON=python2.7`, and only a `Python.h` from say python3.6).
Let's restrict ourself to the exact same version used for the interpreter, i.e., use something like
AX_PYTHON_DEVEL([== $MY_PYTHON_VERSION]) ; untested
For that we'd need to safe the version on the success branch of `AM_PATH_PYTHON` with e.g.,
MY_PYTHON_VERSION=`python -c 'import sys; print(sys.version[:3])'
configure.ac
Lines 2250 (patched)
<https://reviews.apache.org/r/67322/#comment286439>
Looking at the documentation it seems none of the called functions sets this variable, and this can only be a possibly empty user variable.
Let's instead make sure this refers to the exact version of the found interpreter, see e.g., above for a sketch on how to detect it.
- Benjamin Bannier
On May 25, 2018, 3:01 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67322/
> -----------------------------------------------------------
>
> (Updated May 25, 2018, 3:01 p.m.)
>
>
> Review request for mesos and Benjamin Bannier.
>
>
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The Python bindings work with Python 2. If Python 3 is set as
> the default `python` on the system, they should not be built.
>
> This check in `configure.ac` ensures that the Python version is
> not more than 2.7. This is similar to the current check we have
> for the Python CLI.
>
> CMake does not offer an option to build the Python bindings thus
> we only need to add the check for autoconf users.
>
>
> Diffs
> -----
>
> configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
>
>
> Diff: https://reviews.apache.org/r/67322/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 67322: Added Python 2 check for Python bindings
when using `configure`.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67322/#review204050
-----------------------------------------------------------
PASS: Mesos patch 67322 was successfully built and tested.
Reviews applied: `['67322']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67322
- Mesos Reviewbot Windows
On May 25, 2018, 1:01 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67322/
> -----------------------------------------------------------
>
> (Updated May 25, 2018, 1:01 p.m.)
>
>
> Review request for mesos and Benjamin Bannier.
>
>
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The Python bindings work with Python 2. If Python 3 is set as
> the default `python` on the system, they should not be built.
>
> This check in `configure.ac` ensures that the Python version is
> not more than 2.7. This is similar to the current check we have
> for the Python CLI.
>
> CMake does not offer an option to build the Python bindings thus
> we only need to add the check for autoconf users.
>
>
> Diffs
> -----
>
> configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
>
>
> Diff: https://reviews.apache.org/r/67322/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Armand Grillet
>
>