Bug 240343 - x11-themes/plasma5-breeze-gtk: Fails to build if lang/python37 is installed
Summary: x11-themes/plasma5-breeze-gtk: Fails to build if lang/python37 is installed
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Graham Perrin
URL: https://www.freshports.org/lang/pytho...
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-05 04:44 UTC by Ting-Wei Lan
Modified: 2023-09-28 18:12 UTC (History)
7 users (show)

See Also:
tcberner: maintainer-feedback+
grahamperrin: maintainer-feedback-


Attachments
Build log (3.59 KB, text/plain)
2019-09-05 04:44 UTC, Ting-Wei Lan
no flags Details
Patch (670 bytes, patch)
2019-09-05 13:16 UTC, Ting-Wei Lan
no flags Details | Diff
Patch for Mk/Uses/python.mk (511 bytes, patch)
2019-11-30 16:56 UTC, Ting-Wei Lan
no flags Details | Diff
Patch for Mk/Uses/python.mk (619 bytes, patch)
2019-11-30 16:57 UTC, Ting-Wei Lan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ting-Wei Lan 2019-09-05 04:44:25 UTC
Created attachment 207202 [details]
Build log

It seems that CMake picks the latest version instead of respecting what python3 symlink points to. Since cairo module is only installed for the default version of Python 3, which is currently 3.6, it can't find cairo module for Python 3.7.

-- Found Python3: /usr/local/bin/python3.7 (found version "3.7.4") found components:  Interpreter
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'cairo'
Comment 1 Ting-Wei Lan 2019-09-05 04:45:11 UTC
This problem was reproduced on ports r511099.
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-05 11:40:52 UTC
While we dont use symlinks to determine which Python version a port should build with, ports are required to build explicitly and only with the Python version chosen by the user (via DEFAULT_VERSIONS) or the DEFAULT_VERSIONS default.

Can you check upstreams documentation and/or CMakeLists.txt for how to explicit set the Python version
Comment 3 Ting-Wei Lan 2019-09-05 13:16:17 UTC
Created attachment 207213 [details]
Patch

I just found a solution for the problem. It needs -DPython3_FIND_STRATEGY=LOCATION to prefer the first one it finds instead of finding the latest version in all paths, and -DPython3_EXECUTABLE=${PYTHON_CMD} is also needed to tell find_program to skip the search. USES=gnome is added to fix a warning because the port defines USE_GNOME.
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 03:31:22 UTC
(In reply to Ting-Wei Lan from comment #3)

Is -DPython3_EXECUTABLE=${PYTHON_CMD} *sufficient* on its own to explicit set the path to Python we want?
Comment 5 Ting-Wei Lan 2019-09-06 03:38:29 UTC
(In reply to Kubilay Kocak from comment #4)
No, the default is -DPython3_FIND_STRATEGY=VERSION, and CMake will try to find the highest version even if -DPython3_EXECUTABLE=${PYTHON_CMD} is used.
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 03:43:15 UTC
(In reply to Ting-Wei Lan from comment #5)

That sounds broken. If a specific version is specified, that should preclude a search, no? 

Almost all other CMake projects I've come across either don't have an additional 'strategy' mechanism for Python, or skip 'searches' entirely when PYTHON_INTERPRETER is provided (not the internal Python*_EXECUTABLE variable)

The above is just with most just using CMake's default/upstream python module to provide the variables/functionality

See Also: https://gitlab.kitware.com/cmake/cmake/issues/19492
Comment 7 Ting-Wei Lan 2019-09-06 04:36:44 UTC
(In reply to Kubilay Kocak from comment #6)
plasma5-breeze-gtk does nothing special here. It just uses find_package(Python3) provided by CMake upstream with two lines of code.

  find_package(Python3 COMPONENTS Interpreter REQUIRED)
  set(PYTHON_EXECUTABLE "${Python3_EXECUTABLE}")

The find stratagy is provided by CMake upstream. It can be found in /usr/local/share/cmake/Modules/FindPython/Support.cmake, and it is documented at https://cmake.org/cmake/help/v3.15/module/FindPython3.html. After adding  message() calls to debug the module, I think the reason that setting Python3_EXECUTABLE doesn't work for Python3_FIND_STRATEGY=VERSION is that it always iterates through the list of known python versions, which starts at 3.8. Since ${PYTHON_CMD} (3.6) specified with Python3_EXECUTABLE doesn't match the requirement (3.8) exactly, _python_validate_interpreter resets Python3_EXECUTABLE to Python3_EXECUTABLE-NOTFOUND. Therefore, all find_program calls after the first _python_validate_interpreter don't see Python3_EXECUTABLE set on the command line, and they just use its own way to find python.
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 05:15:30 UTC
(In reply to Ting-Wei Lan from comment #7)

Thanks for the analysis, and yeh, that sounds like the linked upstream issue in comment 6.

What I'm not quite sure about is why most projects up until now using CMake have just been able to set PYTHON_EXECUTABLE [1]

Did upstream CMake change at some point?

[1] find /usr/ports -name Makefile -exec grep -H PYTHON_EXECUTABLE {} +
Comment 9 Ting-Wei Lan 2019-09-06 05:56:28 UTC
(In reply to Kubilay Kocak from comment #8)
PYTHON_EXECUTABLE seems to come from the older and deprecated PythonInterp module: https://cmake.org/cmake/help/v3.15/module/FindPythonInterp.html.
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-06 06:17:42 UTC
In particular from the CMake policy:

This policy was introduced in CMake version 3.15. Use the cmake_policy() command to set it to OLD or NEW explicitly. Unlike many policies, CMake version 3.15.3 does not warn when this policy is not set and simply uses the OLD behavior.

Does that mean this ports upstream is setting cmake_policy NEW ?
Comment 12 Ting-Wei Lan 2019-09-07 07:31:02 UTC
(In reply to Kubilay Kocak from comment #11)
I didn't find any cmake_policy command there: https://cgit.kde.org/breeze-gtk.git.
Comment 13 Kubilay Kocak freebsd_committer freebsd_triage 2019-09-16 12:01:09 UTC
^Triage: Assign to maintainer of port with the issue.

Issue has been analyzed, patch appears appropriate and sufficient to explicitly set the python version that must be used (without fallback, or ordering issues)

This issue is very likely going to hit other ports that use cmake.
Comment 14 commit-hook freebsd_committer freebsd_triage 2019-10-13 15:09:07 UTC
A commit references this bug:

Author: rakuco
Date: Sun Oct 13 15:08:32 UTC 2019
New revision: 514401
URL: https://svnweb.freebsd.org/changeset/ports/514401

Log:
  Add USES=gnome.

  Fix a DEV_WARNING, we were setting USE_GNOME without setting USES=gnome. This
  is part of the patch in bug 240343.

  PR:		240343
  Submitted by:	Ting-Wei Lan <lantw44@gmail.com>

Changes:
  head/x11-themes/plasma5-breeze-gtk/Makefile
Comment 15 Raphael Kubo da Costa freebsd_committer freebsd_triage 2019-10-13 15:23:42 UTC
(In reply to Kubilay Kocak from comment #13)
> This issue is very likely going to hit other ports that use cmake.

In this case, isn't it better to always add this to CMAKE_ARGS in Uses/python.mk? I see that there's something there that seems to attempt to work around the same issue.
Comment 16 Ting-Wei Lan 2019-11-30 16:56:33 UTC
Created attachment 209559 [details]
Patch for Mk/Uses/python.mk
Comment 17 Ting-Wei Lan 2019-11-30 16:57:28 UTC
Created attachment 209560 [details]
Patch for Mk/Uses/python.mk

Sorry, I attached the wrong patch.
Comment 18 Ting-Wei Lan 2019-11-30 16:59:54 UTC
(In reply to Raphael Kubo da Costa from comment #15)
I moved it to Mk/Uses/python.mk in the new patch.
Comment 19 Adriaan de Groot freebsd_committer freebsd_triage 2020-01-03 20:31:29 UTC
The latest patch in this issue affects python.mk, so changing the assignee to the list responsible for that, rather than kde@ who are responsible for the port experiencing the problem.

If python@ rejects this, we can pick it up again in the affected port, although that might not scale if other ports are affected as well. We might consider messing around with CMake itself, setting the default search approach to LOCATION instead of VERSION (matching an earlier iteration of this patch).
Comment 20 Tobias C. Berner freebsd_committer freebsd_triage 2020-04-12 21:04:34 UTC
Moin moin 

Any news from python@ here?

mfg Tobias
Comment 21 Gleb Popov freebsd_committer freebsd_triage 2021-09-16 12:32:36 UTC
Bumping the PR. python@ team, we need a feedback from you on the python.ml patch.
Comment 22 Li-Wen Hsu freebsd_committer freebsd_triage 2022-06-22 17:04:09 UTC
A question here:

Now in x11-themes/plasma5-breeze-gtk/Makefile nwe have:

  -DPython3_EXECUTABLE:PATH=${PYTHON_CMD}

But the python.mk in this ticket is:

  -DPython3_FIND_STRATEGY=LOCATION \
  -DPython3_EXECUTABLE=${PYTHON_CMD}

Should we add :PATH here? Or that's not needed because of the previous line?
Comment 23 Graham Perrin 2023-09-28 18:12:20 UTC
lang/python37 was removed three months ago. 

Thanks to Mina Galić for observing obsolescence.