qmlformat: Probe first the most common Qt6 path #4681
Conversation
|
By the way, I have a qtchooser port that supports QT6 as well: |
tools/qmlformat.py
Outdated
| qmlformat_executable = shutil.which("qmlformat") | ||
| # First look up at the most common location for QT6 which is not in PATH | ||
| qmlformat_executable = shutil.which( | ||
| "qmlformat", os.F_OK, "/usr/lib/qt6/bin" |
There was a problem hiding this comment.
This does not work in Fedora where the path is "/usr/lib64/qt6/bin"
There was a problem hiding this comment.
The executable is in the PATH and named qmlformat-qt6.
These are actually 2 independent copies of the executable:
/usr/bin/qmlformat-qt6/usr/lib64/qt6/bin/qmlformat
There was a problem hiding this comment.
And also at "/usr/lib/qt6/bin" depending on the architecture: https://fedora.pkgs.org/34/fedora-updates-x86_64/qt6-qtdeclarative-devel-6.2.2-1.fc34.i686.rpm.html
I will add /usr/bin/qmlformat-qt6 to the lookup paths.
There was a problem hiding this comment.
32-bit is no longer supported.
tools/qmlformat.py
Outdated
| # First look up at the most common location for QT6 which is not in PATH | ||
| # This applies to Debian, Arch and Fedora (i686) | ||
| qmlformat_executable = shutil.which( | ||
| "qmlformat", os.F_OK, "/usr/lib/qt6/bin" |
There was a problem hiding this comment.
The mode argument is wrong, now it will find files that exist but aren't executable. Please just remove it and use a keyword argument for path:
"qmlformat", path="/usr/lib/qt6/bin"Also, please create a list with possible installation paths, so that isn't not redundant when checking for qmlformat-qt6 or qmlformat:
unix_paths = (
"/usr/lib/qt6/bin",
"/usr/lib64/qt6/bin",
)
path = os.pathsep.join(unix_paths) if sys.platform != "win32" else None
tools/qmlformat.py
Outdated
| for path in unix_paths: | ||
| qmlformat_executable = shutil.which("qmlformat", path=path) |
There was a problem hiding this comment.
This loop is unnecessary, path is equivalent to $PATH, i.e. may contain multiple paths separated by os.pathsep (: on unix, ; on windows).
What we do need a loop for is both executable name variants:
| for path in unix_paths: | |
| qmlformat_executable = shutil.which("qmlformat", path=path) | |
| path = os.pathsep.join(unix_paths) | |
| for executable_name in ("qmlformat-qt6", "qmlformat"): | |
| qmlformat_executable = shutil.which(executable_name, path=path) |
|
OK I have removed the loop. There is no need to look up qmlformat-qt6, because we will have found qmlformat already in the native location. The -qt6 suffix seems to be an Fedora only workaround. Upstream uses just "6" but only for user facing "usr/bin/qmake6" probably as common entry point? Unfortunately Fedora uses "/usr/bin/qmake-qt6" So there is a chance to use But since this is already working, I am done here. |
tools/qmlformat.py
Outdated
| qmlformat_executable = shutil.which( | ||
| "qmlformat", path="/usr/lib/qt6/bin:/usr/lib64/qt6/bin" | ||
| ) | ||
| # Than look at PATH |
There was a problem hiding this comment.
| # Than look at PATH | |
| # Then look in PATH |
|
Done. |
|
@Holzhaus any further notes? |
No, I already approved 18 days ago, just forgot to merge. |
Normally the Qt6 tools are not at PATH, because the location is known by CMake.
The qtchooser that puts QT5 to PATH is no longer maintained for QT6
That is why the pre-commit hook finds the QT5 version. However we have dropped the QT5 support for the qml gui and it should prefer the QT6 version. This PR looks at the Qt6 binary path first and falls back to the old behavior.