Re: Review Request 67322: Added Python 2 check for Python bindings when using `configure`.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67322/#review204045
---




configure.ac
Line 2247 (original), 2247 (patched)


`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)


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`.

[Mesos Reviewbot Windows]

---
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
> 
>