-
-
Notifications
You must be signed in to change notification settings - Fork 890
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
Cura 8640 PyQt6 upgrade #1639
Cura 8640 PyQt6 upgrade #1639
Conversation
Contains a handy function for rpath setting Contributes to CURA-8640
Contributes to CURA-8640
Contributes to CURA-8640
Contributes to CURA-8640
This allows CuraEngine to be compiled with Visual Studio boostorg/polygon#40 Contributes to CURA-8640
This commit should allow for the usage of Conan to manage our dependencies simply do a `conan install . -if <cmake_build_folder> -pr:h default --build=missing` and configure the CMake project using the toolchain `-DCMAKE_TOOLCHAIN_FILE=<cmake_build_folder>/conan_toolchain.cmake` I did my best to make this change backwards compatible and allow falling back to shipped third-party sourcecode e.q. clipper and rapidjson. This third party sourcecode will be removed in the near future. Contributes to CURA-8640
# Conflicts: # CMakeLists.txt
Contributes to CURA-8640
Contributes to CURA-8640
…RA-8640_PyQt6_upgrade
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.
Only one minor remark that I'd point out in case you agree that it could be better, but the rest of the changes are good and I wouldn't block it.
In the libs folder I think all of the whitespace has changed to include a carriage return (Windows-style). This may be a weirdness in your Git config. I verified that no real changes were made with the -w
option in git diff
. The only actual change was the way clipper.hpp
is imported, which is pointed out here on Github as well.
$<$<CONFIG:Debug>:DEBUG> | ||
$<$<CONFIG:RelWithDebInfo>:ASSERT_INSANE_OUTPUT> | ||
$<$<CONFIG:RelWithDebInfo>:USE_CPU_TIME> | ||
$<$<CONFIG:RelWithDebInfo>:DEBUG> |
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 think perhaps just if(Debug OR RelWithDebInfo)
is easier syntax to read than this.
A bit of a matter of style though, so I'm fine either way.
In order to get this to work on our build-system and working for all three OSes we did a shit tons of boy scouting in our cmake. We removed old methods with variables and try to be consisted in a target-based approach. The idea is that we don't patch stuff down the line, but that the install should place everything in the correct path in a uniform way across all of Cura's dependencies.
Most of the changes had to do with how dependency targets were named and to make sure that external projects weren't downloaded automatically or that we prefer the usage over an external dependency instead of the shipped once. That means moving the third-party libs, such as clipper to another structure. I tried to be backward compatible with these changes but they are untested on my machine. I believe @Ghostkeeper ran into some issues recently.
Because of this target based approach and how the flags and settings are now set we can drop support for MinGW on Windows: CuraEngine (and its dependencies protobuf and Arcus) can now compile using the x64 MSVC toolchain on Windows. This is tested with the Visual Studio 2019 and 2022 version.
The question is if we are going put time into doing it properly for both scenario's: supporting shipped libs and external libs or are we saying don't waste the energy since these shipped libs will be removed anyhow in CURA-8830 . In fact people can install the third-party dependencies now anyhow using Conan. I myself am in favor of removing the third-party shipped dependencies t.b.h.
To setup Conan:
To use Conan:
This should download, (build if needed,) en install the dependencies, create a cmake toolchain containing the correct paths top dependencies and proper settings. You can also use make or nmake instead of Ninja but I advise Ninja since that can be used on all three systems.
There were also some changes needed in the namespace when using enums and the renaming of the Python modules Arcus and Savitar to their counterparts pyArcus and pySavitar. This was needed to ensure no naming conflicts downstream.
Part of
Fixes
Todo