-
Notifications
You must be signed in to change notification settings - Fork 81
CURA-6187 Fixes for building in docker #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4621109 to
eabd935
Compare
|
CURA-6188 Also added GitLab CI files. |
|
(Regarding your question whether I can test the PR 😉) Sure! Excuse that I was not replying recently. |
|
Up and running: https://thopiekar.eu:5443/cura/cdk/pipelines/314
However, I never came to the point, where software distributions are done, eg. AppImage for Linux, NSIS for Windows, etc. |
CMakeLists.txt
Outdated
|
|
||
| # FIXME: Remove the code for CMake <3.12 once we have switched over completely. | ||
| # FindPython3 is a new module since CMake 3.12. It deprecates FindPythonInterp and FindPythonLibs. The FindPython3 | ||
| # module is copied from the CMake repository here so in CMake <3.12 we can still use it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I(/we) assume this is still 'fixme' because we don't want to rely on the copy once we're over to 3.12+?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our current (old) systems are still using CMake < 3.12, but in the docker image, it's CMake >= 3.12. This FIXME is to keep compatibility for now. Once the old systems are retired, we can remove this FIXME.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments are not entirely correct. I've updated them in e15df83.
|
Any chance I can force the Python module to be installed to I've tried with: But it always ends up in |
|
@hroncok You can try EDIT: |
|
Doesn't help: |
|
@hroncok I think this has to do with where the python3.7 libraries are installed. Because the python modules are installed in |
|
I'm afraid I don't understand. This is an extension module and hence need to be installed in sitearch. sitearch is determined by The
How does that help? I can "trick" it by |
|
I think the bug in this case lies in the FindPython script that we're using. This detects the installation folder that you need changed. On Cura's side we can only make hacks around that script, but the real bug is in CMake itself here. |
|
That macro should give you proper Python_SITEARCH. Where do you instruct cmake to install the Python extension? I don't really know cmake, but maybe you need to tell it it belongs to Python_SITEARCH? |
|
The code that causes the plug-in to be installed is this line: Line 98 in 63f3f00
This calls upon a CMake function in the Sip import of CMake. The only documentation of this function that I found online was here: https://api.kde.org/cmake/modules.html (search for |
|
This looks suspicious: libArcus/cmake/SIPMacros.cmake Line 127 in 63f3f00
|
|
Changing that to Python3_SITEARCH fixes the problem. I'd report that to the upstream of those macros, but I have no idea where is that. |
|
Aha, good find. I thought that The upstream of those macros is @sedwards2009 who was since hired by Ultimaker (by sheer coincidence)... But I doubt that he's still involved in this. What exact change did you make, and perhaps you'd like to get recognition of your change by making a PR? 😄 |
|
I can definitively send a PR that just changes it here (and possible in libsavitar), I just assumed those are vendored/bundled/copied from somewhere else. |
The sitearch directory is used for compiled code, such as this extension module. See Ultimaker#94 (comment)
|
🎉 #99 🎉 |
The sitearch directory is used for compiled code, such as this extension module. See Ultimaker/libArcus#94 (comment)
This commit should work in both docker (using centos7 with cmake 3.13) and the current build/CI servers (with cmake <3.12).
CMAKE_PREFIX_PATH