[gruenich]

QT_CREATE_TRANSLATION is deprecated.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[firewave]

To avoid the clang-tidy warnings we need to disable all checks. See at the top of gui/CMakeFiles.txt (that should be moved into a shared function since it already used in another place).

[gruenich]

Thanks for you explanation. I don't get which folder needs the additional clang-tidy configuration file. My change does not add generated files in a new directory, does it?

[gruenich]

I see no reason to hold this back. Please consider merging.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[danmar]

I don't know much about cmake and asked AI to review this. In my experience AI reviews are often mostly wrong - take this AI-review below with a mountain of salt:

The deprecation claim checks out. Qt's own docs for qt_create_translation state it's deprecated in favor of the target-based qt6_add_lupdate/qt6_add_translations (Qt docs
(https://doc.qt.io/qt-6/qtlinguist-cmake-qt-create-translation.html)). (Note: the PR title/body says QT_CREATE_TRANSLATION, not QT_TRANSLATE_TRANSLATION — I assume that's what you meant.)
qt_add_translations needs Qt ≥6.2, and this repo already requires Qt6 everywhere in CI (including the "oldest supported" ubuntu-22.04 job, which already has
qt6-l10n-tools/qt6-tools-dev-tools installed), so there's no version-compatibility issue.

But the patch drops something it doesn't replace. The old code did:
QT_CREATE_TRANSLATION(qms ${CMAKE_CURRENT_SOURCE_DIR} ${tss})
...
install(FILES ${qms} DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT applications)
The PR removes the QT_CREATE_TRANSLATION(qms ...) call but leaves install(FILES ${qms} ...) untouched (gui/CMakeLists.txt:75 in the PR). Nothing sets qms anymore, so that install line now
installs an empty file list — the built package ships with zero .qm translation files.

This isn't cosmetic: gui/translationhandler.cpp loads translations from disk at runtime (QFile::exists(datadir/lang/...), mTranslator->load(...)), not from a Qt resource.
qt_add_translations by default embeds generated .qm files into a compiled-in Qt resource (prefix /i18n) instead of leaving them as loose files — getting standalone .qm paths back requires
explicitly passing QM_FILES_OUTPUT_VARIABLE, which the PR doesn't do.

There's also direct corroborating evidence in the repo: .github/workflows/release-windows.yml does
move build\gui*.qm win_installer\files\translations
expecting loose .qm files under build/gui/, matching the old mechanism. That workflow only runs on release tags / nightly schedule / manual dispatch — not on this PR's own CI — so this
breakage wouldn't be caught before a release.

Minor secondary note: the PR deletes the comment # TODO: passing "-no-obsolete" here breaks the translations and then passes LUPDATE_OPTIONS "-no-obsolete" anyway. That's probably fine in
practice since qt_add_translations only runs lupdate via an opt-in update_translations target (not part of the default build, unlike the old always-on macro) — but the reasoning that
flag was avoided for is no longer documented anywhere, so it could resurface if someone runs that target to refresh translations later.

Verdict: moving off QT_CREATE_TRANSLATION is the right direction, but this PR isn't a correct/complete fix — it needs to wire qms/QM_FILES_OUTPUT_VARIABLE back into the install() call and
verify where qt_add_translations actually places .qm files relative to what release-windows.yml expects, before merging.

[dmcppcheck]

AI review

I found no new problems. The change only touches gui/CMakeLists.txt. Qt 6 isn't installed in the container, so I couldn't configure or build the GUI and checked the change by reading it.

• The main concern in the AI review that C6 quoted doesn't apply to this version. The PR passes QM_FILES_OUTPUT_VARIABLE qms to qt_add_translations. That sets qms, so install(FILES ${qms} ...) still gets the loose .qm files, and the files are not embedded as a resource.
• qt_add_lrelease writes the .qm files to CMAKE_CURRENT_BINARY_DIR by default, which is build/gui, the same folder as before. That is the folder release-windows.yml moves them from.
• As I understand Qt's macros, they make cppcheck-gui depend on its _lrelease target, so cmake --build build --target cppcheck-gui in the release workflow should still generate the files. Only a real Qt build can confirm this.

The point C6 made about the deleted -no-obsolete TODO is still open.

Earlier comments

#	Comment	By	Where	Status
1	Generated files may trigger clang-tidy warnings; all checks need to be disabled as at the top of gui/CMakeLists.txt	@firewave	general	💬 Answered: Author says the change adds no generated files in a new directory and asked for clarification; no reply
2	qms is no longer set, so install(FILES ${qms}) installs nothing and .qm files are embedded instead of loose	@danmar	general	✅ Fixed
3	TODO saying -no-obsolete breaks translations was deleted, yet LUPDATE_OPTIONS "-no-obsolete" is now passed	@danmar	general	⚠️ Open

@gruenich, please fix, resolve or reply to the open comments: #3.

Testing

• cmake ../../pr without the GUI (in /work/scratch/b): configures fine. The changed code is inside if (BUILD_GUI), so this only shows that the rest of the configure still works.
• cmake -DBUILD_GUI=ON ../../pr: fails at find_package(Qt6 ...) because Qt 6 is not installed in the container. So I could not configure or build the GUI, check whether the .qm files are generated and installed, or run the update_translations target.
• I did not build or run cppcheck/testrunner. The PR changes no C++ code or tests, so building them would not test anything that changed.
• What I checked by reading:
  • qms is now set through QM_FILES_OUTPUT_VARIABLE and is still used by install(FILES ${qms} ...).
  • LinguistTools is already among the Qt components in cmake/findDependencies.cmake.
  • gui/translationhandler.cpp loads loose .qm files from disk at runtime, which matches files that are not embedded.
  • release-windows.yml builds --target cppcheck-gui and moves build\gui\*.qm.

---

Automated review of 5d0e4c5 by Claude. The pull request was built and tested locally.

[danmar]

AI review (second opinion on the earlier AI review in #8062 (comment))

I built 5d0e4c5 with -DBUILD_GUI=ON -DBUILD_TESTING=ON and Qt 6.10.2 and ran ctest: all 91 tests pass, including test-translationhandler.

The earlier review, point by point

• qms not set, so install() installs no .qm files: fixed by the latest commit (QM_FILES_OUTPUT_VARIABLE qms). cmake --install installs all 14 .qm files.
• qt_add_translations embeds the .qm files as a resource: correct in general, but QM_FILES_OUTPUT_VARIABLE turns that off, so the .qm files stay separate files in build/gui/.
• release-windows.yml expects build\gui\*.qm: the location is fine, but there is another problem, see below.
• -no-obsolete: fine now. lupdate no longer runs in a normal build, only when the update_translations target is built.

Problem: the release-windows build gets no .qm files

release-windows.yml builds only --target cppcheck-gui and then does move build\gui\*.qm .... With Qt >= 6.7 (the release uses 6.10.0), qt_add_translations creates the .qm files in a separate <project>_lrelease target. That target is part of all, but cppcheck-gui does not depend on it. Before, the .qm files were sources of cppcheck-gui.

Tested with Qt 6.10.2, after deleting build/gui/*.qm:

cmake --build build --target cppcheck-gui

• main: 14 .qm files
• this PR: 0 .qm files

This could be the same problem that made #8047 be reverted.

Adding this after the qt_add_translations(...) call fixed it for me (14 .qm files, and configuring with -Werror=dev --warn-uninitialized as in the release workflow gives no new warnings):

add_dependencies(cppcheck-gui release_translations)

I only tested this with Qt 6.10, so CI should confirm that it works with the oldest Qt too.

Other notes

• Nice side effect: on main a normal build rewrites all gui/*.ts files in the source tree (trac #14169). With this PR it does not. Maybe mention the ticket in the title.
• Minor: without SOURCE_TARGETS cppcheck-gui, Qt >= 6.7 makes update_translations scan all the targets of the project (lib, cli, test, generated files). Running it added no new strings, but the 342 .ui locations in each .ts file became paths like ../build/gui/ui_mainwindow.h, and the 31 obsolete/vanished entries were removed (expected with -no-obsolete). This only matters if someone uses that target instead of lupdate gui.pro.