CMake consistency fixes #41
Conversation
Changed install path on Debian from dist-packages Contributes to CURA-8640
Will only be present in development environment Contributes to CURA-8640
Cura 8640 PyQt6 upgrade
| find_package(Python3 ${CURA_PYTHON_VERSION} EXACT REQUIRED COMPONENTS Interpreter) | ||
| if(NOT DEFINED Python_VERSION) | ||
| set(Python_VERSION | ||
| 3.10 |
There was a problem hiding this comment.
we have 3.7 on the printer, anything above that won't work for firmware.
There was a problem hiding this comment.
Hm, I'm assuming libCharon needs 3.10 when used with/for Cura?
If so, to be on the safe side: do not use a default value, but require Python_VERSION to be specified instead?
In order to prevent 'silent' mistakes.
| if(NOT DEFINED Python_SITELIB_LOCAL) | ||
| set(Python_SITELIB_LOCAL | ||
| "${Python_SITELIB}" | ||
| CACHE PATH "Local alternative site-package location to install Cura" FORCE) | ||
| endif() | ||
|
|
||
| option(INSTALL_SERVICE "Install the Charon DBus-service" ON) | ||
| option(INSTALL_CLIENT "Install the Charon Client library" ON) |
There was a problem hiding this comment.
With the exclusions removed, do these still matter?
INSTALL_CLIENT seems to be unused.
There was a problem hiding this comment.
Yes I think these could be removed as well then, if they are indeed unused now.
I'm not sure why those excludes were removed because this is code for the firmware-side.
| set(CHARON_INSTALL_PATH lib${LIB_SUFFIX}/python${Python3_VERSION_MAJOR}.${Python3_VERSION_MINOR}/site-packages) | ||
| endif() | ||
|
|
||
| set(_excludes PATTERN __pycache__ EXCLUDE) |
There was a problem hiding this comment.
No longer necessary?
| endif() | ||
| find_package(Python ${Python_VERSION} EXACT REQUIRED COMPONENTS Interpreter) | ||
| message(STATUS "Linking and building ${project_name} against Python ${Python_VERSION}") | ||
| if(NOT DEFINED Python_SITELIB_LOCAL) |
There was a problem hiding this comment.
Where/how's this expected to be set now?
There was a problem hiding this comment.
Cura-build sets this variable upon building. The reason is that all other packages are installed to site-packages on all platforms (Windows, MacOS and Linux) but on Debian-based platforms like our Ubuntu build server the default for Python_SITELIB is dist-packages.
We added this so that from cura-build we could override the installation directory to be consistent so that we would have to make fewer exceptions.
The default remains the Python_SITELIB that becomes dist-packages on Debian-based systems, like it was before.
There was a problem hiding this comment.
Since these changes are aimed at changes/requirements from the cura-build, is it stil possible to use this for a cura-build-less build of libCharon? As in: does Team Styx now need to use the cura-build to release/create a new pkg for libCharon?
There was a problem hiding this comment.
No, that's not our intention. That would be very unworkable for you!
If Python_SITELIB_LOCAL is not specified it should default to the old behaviour.
Would it be acceptable if we add documentation to this about that this parameter exists and leave the behaviour as-is, then?
These are some changes that we made to the CMake build system to prepare for Cura 5.0.
From the original PR:
I'd suggest that you take a look at these changes to see how they can be adjusted for the build environment of the firmware. I don't expect the changes here to have a big impact on the way you're working though.
Contributes to issue CURA-8640.