Sure - I’m talking about Python here, “build” is never an issue. I just want to make sure no one decides to just rip out the QRegExp code without testing it. In Python there is no compiler to catch those things, the code has to actually be exercised.
So far I have replaced most of the occurrences of QRegExp with QRegularExpression. The class is still used in the class DlgCreateNewPreferencePackImp and the Path, AddonManager and TechDraw modules.
Btw, my PySide2 package for XUbuntu 18.04 (taken from the PPA) already provides QRegularExpression. So, I don’t think it’s a problem to also port the Python code.
It wasn’t QRegularExpression itself that gave me the problems – it was various “client” classes that only supported the old QRegExp class, and it wasn’t well-documented which versions of PySide actually implemented them properly. The one that springs immediately to mind is QRegularExpressionValidator – can you check if your version of PySide has that now?
Too bad. This class is indeed missing. It’s supposed to be part of QtGui. But an option is that we implement it in Python and put the code to here:
https://github.com/FreeCAD/FreeCAD/blob/master/cMake/FreeCAD_Helpers/SetupShibokenAndPyside.cmake#L68
Can you also check QSortFilterProxyModel to see if it now supports QRegularExpression? I have a note that says it was added to PySide in Qt 5.12, but I’m not sure of my source for that comment. I’m just checking whether it has setFilterRegularExpression() to see if I can use it or not, rather than pegging to a specific Qt version.
I don’t think that is really necessary: the current code already works around the missing class. When used with Qt6 it ought to just work, the QRegExp code path will never get executed if the necessary QRegularExpression functionality exists. It’s not as clean as with C++, where we are able to literally “delete” code paths via the preprocessor, but I believe I implemented both instances of QRegExp so that that class will never get instantiated in the Qt6 code paths.
It’s missing. The Qt docs also say it has been added with 5.12:
New roablock is xml stuff in TechDraw.
Thanks to prior work by StSt94 I have been making some progress but anyone with knowledge of the matter would be much quicker!
As an aside, several people each did a bunch of work on QT6 migration. There was also overwhelming support for collaboration. Pity all were rejected out of hand.
wmayer when we migrated to PySide2, how did we communicate from cMake to the Python code that it should use the new import statements? Was there a variable in FreeCAD or FreeCADGui?
You mean the way to still use
from PySide import QtCore
instead of
from PySide2 import QtCore
in Python code? That’s achieved by the file SetupShibokenAndPyside.cmake. At configure time it creates some files under Ext/PySide and inside that files the PySide2 modules are loaded.
Perfect, thanks. To try to make some headway on this project I’m thinking of adding a BUILD_QT6 flag to cMake – at first of course it won’t compile, but my theory is that then developers can start to submit PRs without worrying that they don’t have a “finished product” yet. I presume we want to keep the ability for developers to explicitly choose whether to use Qt5 or Qt6, in cases where their systems have both installed?
I presume we want to keep the ability for developers to explicitly choose whether to use Qt5 or Qt6, in cases where their systems have both installed?
Yes. This makes it easier to test things because it allows to run two FreeCAD versions in parallel that are linked to the different Qt versions.
Thank you for this change of heart !
If devs can lodge (not too big, and not total disaster) QT6 related commits knowing they will be merged or assisted into shape promptly, without bias (complicated for the reviewer if also a QT6 dev), this becomes an exciting experiment (after recent big discussions) in encouraging positive dev experience.
And what of imperfect commits? “Perfect is the enemy of good”.
Does the switch to QT6, which depends on Pyside6, requires handling/rewrite of FC source code to conform to Pythons Py_Limited_API, as Pyside6 / Shiboken6 will set this as a default?
Cross referencing here the discussion on the macOS and CPython API thread: Making sure you're not a bot!
According to my understanding these are two completely different topics. The fact that PySide/shiboken conform to the limited API does not mean that FreeCAD MUST conform to it, too. Actually FreeCAD is free not to conform to the limited API and this doesn’t affect PySide/shiboken at all because FreeCAD depends on them and not the other way around. However, as said in the other thread we should give it a try to make FreeCAD conform to the limited API.
According to my understanding these are two completely different topics. The fact that PySide/shiboken conform to the limited API does not mean that FreeCAD MUST conform to it, too. Actually FreeCAD is free not to conform to the limited API and this doesn’t affect PySide/shiboken at all because FreeCAD depends on them and not the other way around
I’m confused. FC does not respect LimitedAPI (this we know), how does this not become a problem if PySide/Shiboken are built for LimitedAPI? I’m seeing non-LimitedAPI stuff simply not found during build. If this is a confused question it is because I am lost in space.
It feels like the answer should be simple, yet is difficult to pin down. Perhaps a building block description of the relationships involved?
As wmayer rightly pointed out, if Pyside/Shiboken is compiled or set to use the Py_Limited_API, this should not affect the FC source code. This setting should be restricted to the Pyside/Shiboken package.
Why are there errors with a build on macOS? As I mentioned in the other thread, I suspect the Py_Limited_API setting somehow spills over from Shiboken to the consuming code or somehow pollutes the ENV variables of the system, so that the Python interpreter picks this up and enforces this setting for the whole project.
It needs to be investigated, if this is A) a bug within the Pyside/Shiboken package, B) the Homebrew setup (as apparently we don’t se this problem on Linux/Windows), C) some Cmake file within the FC source which picks up the default setting from Pyside/Shiboken and sets it for the FC build or D) any other possible reason.
If PySide/shiboken decides to use the limited Python API then this is OK. But an application that uses the C++ part of PySide/shiboken shouldn’t be forced by the library to also conform to the limited API. If it does then this is a bug IMO or do you think it’s OK that a library is allowed to reduce the possibilities of the application that uses this library? After all using the limited Python API is an implementation detail of PySide/shiboken and its C++ API shouldn’t be affected by this. Or do I see something fundamentally wrong?
So, what needs to be checked is whether it’s really PySide/shiboken that pulls in a pre-processor define (e.g. Py_Limited_API) and if yes how it does it. It can be done via CMake or by defining it in a PySide/shiboken header file.
Now in the FreeCAD code the one and only file that includes PySide/shibken headers is PythonWrapper.cpp. But because the build failure was reported for Application.cpp it’s actually impossible that a PySide/shiboken header is the culprit for that because it won’t be included there.
There are 442 occurrences of Qt5: 169 unclassified, 33 in comments and 240 in string constants.
There are 78 occurrences of Qt4: 62 unclassified, 10 in comments and 6 in string constants.
Given that only one version can be in use at a time, can now be the time to normalise these, removing much complexity and repetition. A version of DRY.
BUILD_QT5 / BUILD_QT6 / BUILD_QT4 are misleading names in that they give no clue that they are in fact mutually exclusive.
So, introduce BUILD_QT_VERSION. Unspecified/invalid = Do not build Qt.
-removed obsolete example-
There are a lot of cmake generated derivatives e.g. Qt5xx_FOUND etc. I suggest normalise these as well by making making appropriate copies back where they are generated. At least the ones that are used later. Maybe the copying can be avoided - but I don’t know better.
e.g. Qt5xx_FOUND → Qt_FOUND
cmake macros/functions could handle more complex things.
Without considering the implementation cost, is there any fundamental flaw in the logic?
Qt6 will have a big implementation cost anyway, so would it be that much worse?
Thanks for helping clear my thinking a bit. I need to study a lot more to understand the C++ API you mention.