Bug 1084281 - [cmake] feature-suse-python-interp-search-order.patch breaks cantor
[cmake] feature-suse-python-interp-search-order.patch breaks cantor
Status: RESOLVED UPSTREAM
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Basesystem
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: Simon Lees
E-mail List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-03-07 11:10 UTC by Christophe Giboudeaux
Modified: 2019-03-04 23:57 UTC (History)
4 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christophe Giboudeaux 2018-03-07 11:10:10 UTC
feature-suse-python-interp-search-order.patch causes an issue on the KDE CI hosts.

The python2 backend in Cantor no longer builds as shown here:
https://build.kde.org/job/Applications%20cantor%20kf5-qt5%20SUSEQt5.9/29/consoleFull

The relevant part is here:
06:03:01 -- Found PythonLibs: /usr/lib64/libpython3.6m.so (found suitable version "3.6.4", minimum required is "2.7") 

Cantor explicitly tries to find python 2 but due to the patch, CMake ignores the version.

Reverting the patch locally fixes the issue.
Comment 1 Simon Lees 2018-03-08 01:29:34 UTC
(In reply to Christophe Giboudeaux from comment #0)
> feature-suse-python-interp-search-order.patch causes an issue on the KDE CI
> hosts.
> 
> The python2 backend in Cantor no longer builds as shown here:
> https://build.kde.org/job/Applications%20cantor%20kf5-qt5%20SUSEQt5.9/29/
> consoleFull
> 
> The relevant part is here:
> 06:03:01 -- Found PythonLibs: /usr/lib64/libpython3.6m.so (found suitable
> version "3.6.4", minimum required is "2.7") 
> 
> Cantor explicitly tries to find python 2 but due to the patch, CMake ignores
> the version.
> 
> Reverting the patch locally fixes the issue.

Can you give me a link to your CMakeLists.txt, all this patch does is removes the assumption that  /usr/bin/python is the default python interpreter and instead looks for a python version that meets the constraints from newest to oldest. The reason behind this is in openSUSE, python3 is the default but we have decided not to change /usr/bin/python to python3 for reasons instead we encorage everyone to either use /usr/bin/python2 /usr/bin/python3 rather then /usr/bin/python, reverting the patch fixes it for you because then cmake looks for /usr/bin/python first which is python2 rather then python3. It looks like cmake rightly presumes that 3.6.4 is a higher number then 2.7.

From reading FindPythonInterp I recommend you look at the following.

# The Python_ADDITIONAL_VERSIONS variable can be used to specify a list
# of version numbers that should be taken into account when searching
# for Python.  You need to set this variable before calling
# find_package(PythonInterp).

I suspect you just want to set that variable to 2.7 (you could list additional python2 versions here as well if people are actually still using any others)
Comment 2 Christophe Giboudeaux 2018-03-08 09:01:51 UTC
(In reply to Simon Lees from comment #1)
> Can you give me a link to your CMakeLists.txt, all this patch does is
> removes the assumption that  /usr/bin/python is the default python
> interpreter and instead looks for a python version that meets the
> constraints from newest to oldest. The reason behind this is in openSUSE,
> python3 is the default but we have decided not to change /usr/bin/python to
> python3 for reasons instead we encorage everyone to either use
> /usr/bin/python2 /usr/bin/python3 rather then /usr/bin/python, reverting the
> patch fixes it for you because then cmake looks for /usr/bin/python first
> which is python2 rather then python3. It looks like cmake rightly presumes
> that 3.6.4 is a higher number then 2.7.

Indeed. Here's the CMakeLists.txt :
https://cgit.kde.org/cantor.git/tree/src/backends/CMakeLists.txt

> 
> From reading FindPythonInterp I recommend you look at the following.
> 
> # The Python_ADDITIONAL_VERSIONS variable can be used to specify a list
> # of version numbers that should be taken into account when searching
> # for Python.  You need to set this variable before calling
> # find_package(PythonInterp).

That's what cantor does already:
set(Python_ADDITIONAL_VERSIONS 2.7)
find_package(PythonLibs 2.7)

> 
> I suspect you just want to set that variable to 2.7 (you could list
> additional python2 versions here as well if people are actually still using
> any others)

The patch causes another issue: if python2 isn't installed (it's optional) but python3 is, CMake will assume both were found.
Comment 3 Christophe Giboudeaux 2018-03-08 09:11:59 UTC
(In reply to Christophe Giboudeaux from comment #2)
> > From reading FindPythonInterp I recommend you look at the following.
> > 
> > # The Python_ADDITIONAL_VERSIONS variable can be used to specify a list
> > # of version numbers that should be taken into account when searching
> > # for Python.  You need to set this variable before calling
> > # find_package(PythonInterp).
> 
> That's what cantor does already:
> set(Python_ADDITIONAL_VERSIONS 2.7)
> find_package(PythonLibs 2.7)
> 

Note that this won't help, FindPythonInterp is already called by a build dependency. (but that gives me an idea to work around the regression)
Comment 4 Christophe Giboudeaux 2018-03-13 10:00:35 UTC
https://phabricator.kde.org/D11176 should take care of this issue.
Waiting for feedback
Comment 5 Simon Lees 2018-03-13 10:19:46 UTC
(In reply to Christophe Giboudeaux from comment #4)
> https://phabricator.kde.org/D11176 should take care of this issue.
> Waiting for feedback

Thanks, there might be some other issues that I found in testing (or I broke my local install as well) so i'll keep looking at those as well, Monday was a holiday here.
Comment 6 Simon Lees 2018-03-16 02:16:13 UTC
(In reply to Christophe Giboudeaux from comment #4)
> https://phabricator.kde.org/D11176 should take care of this issue.
> Waiting for feedback

Just thought I'd check, as the Qt / KDE maintainer are you happy with this change Fabian?
Comment 7 Fabian Vogt 2018-03-16 07:52:18 UTC
(In reply to Simon Lees from comment #6)
> (In reply to Christophe Giboudeaux from comment #4)
> > https://phabricator.kde.org/D11176 should take care of this issue.
> > Waiting for feedback
> 
> Just thought I'd check, as the Qt / KDE maintainer are you happy with this
> change Fabian?

I don't know much about CMake - I'm glad that Christophe takes care of that.
Comment 8 Simon Lees 2019-03-04 23:57:35 UTC
I think this can be closed now, reopen if there is still something required in the cmake package