Replace call QT5_WRAP_UI with CMAKE_AUTOUIC

[Rossmaxx]

This PR is meant to simplify some cmake code removing the deprecated (not really sure) qt5_wrap_ui call and replace it with more supported cmake_autouic. Caught this because of a build fail with qt6 on windows.

Had to move the ui files to include to remove the build fail.

[Veratil]

removing the deprecated (not really sure) qt5_wrap_ui call

Not deprecated looking at the documentation, but with Qt6 they changed it to just qt_wrap_ui which was added in Qt 5.15. They still say people should use the CMAKE_AUTOUIC stuff though.

[Rossmaxx]

with Qt6 they changed it to just qt_wrap_ui which was added in Qt 5.15.

That's with the versionless targets from what i understand. I still use versioned targets for compat with existing qt5 stuff.

[DomClark]

You can avoid moving the .ui files by setting the AUTOUIC_SEARCH_PATHS property on the target that uses them, i.e. add set_target_properties(lmmsobjs PROPERTIES AUTOUIC_SEARCH_PATHS "${CMAKE_CURRENT_SOURCE_DIR}/gui/modals") to src/CMakeLists.txt.

However, we want to rewrite the .ui files in C++ eventually (this was done for the effect selection dialog in #7024). While this would be a bit more work, it would have more relevance to the current Qt 5 codebase.

[Rossmaxx]

@DomClark done.

[Veratil]

Sf2Player and GigPlayer have ui files, too. I guess our build_plugin function in cmake doesn't need to be changed? It still calls qt5_wrap_ui.

[Rossmaxx]

@Veratil i haven't extended this PR to plugins yet. I will soon.

[Rossmaxx]

Nvm, I am not touching plugin code for now.

[sakertooth]

Sf2Player and GigPlayer have ui files, too. I guess our build_plugin function in cmake doesn't need to be changed? It still calls qt5_wrap_ui.'

Nvm, I am not touching plugin code for now.

We should actually apply what was done here to those files as well, yeah. Luckily, it seems the only other caller for qt5_wrap_ui is in BuildPlugin.cmake, which is used for each of the plugins. It might be as simple as just removing that line. Try that.

[Rossmaxx]

I have removed both qt5_wrap_ui and qt5_wrap_cpp over there, will try something else if the ci fails.

[Rossmaxx]

Everything fails. I'll try something else.

[Rossmaxx]

What's with all these CI fails? I assume the build is ok since it succeeds with linux but if so, why does everything else fails. Looking at the logs give errors at unrelated places. Would someone please help.

[Rossmaxx]

• i remember getting the same Ci fail pattern in another PR.

[DomClark]

The issue is that you've removed the qt5_wrap_cpp command, so the MOC is no longer run, resulting in missing symbols. The Linux build succeeds anyway since the linker assumes the symbols will be available elsewhere at runtime, but the linkers for macOS and Windows builds expect the symbols to be available at link time.

[Rossmaxx]

I forgot to add the qt5_wrap_cpp command like dom said, it now works. Maybe i should open a pr for moc files as a follow up.

[Rossmaxx]

Converted to draft to block merge. Minor changes left which I will do when i get back on my laptop.

[Rossmaxx]

Ready for merge

[DomClark]

You should now be able to remove the UICFILES parameter of build_plugin.

[DomClark]

The UICFILES parameter is still mentioned in the documentation of build_plugin, and passed to the cmake_parse_arguments command. It should be removed from those too.

Newer CMake versions now generate warnings like this:

 CMake Warning (dev) in plugins/Sid/resid/CMakeLists.txt:
  Policy CMP0071 is not set: Let AUTOMOC and AUTOUIC process GENERATED files.
  Run "cmake --help-policy CMP0071" for policy details.  Use the cmake_policy
  command to set the policy and suppress this warning.

  For compatibility, CMake is excluding the GENERATED source file(s):

    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave6581_PST.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave6581_PS_.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave6581_P_T.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave6581__ST.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave8580_PST.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave8580_PS_.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave8580_P_T.h"
    "D:/a/lmms/lmms/build/plugins/Sid/resid/resid/wave8580__ST.h"

  from processing by AUTOUIC.  If any of the files should be processed, set
  CMP0071 to NEW.  If any of the files should not be processed, explicitly
  exclude them by setting the source file property SKIP_AUTOUIC:

    set_property(SOURCE file.h PROPERTY SKIP_AUTOUIC ON)

Similar ones occur for the SWH LADSPA plugins.

You can fix these warnings by setting the SKIP_AUTOUIC property like this:

diff --git a/plugins/LadspaEffect/swh/CMakeLists.txt b/plugins/LadspaEffect/swh/CMakeLists.txt
index a83001177..45210e05f 100644
--- a/plugins/LadspaEffect/swh/CMakeLists.txt
+++ b/plugins/LadspaEffect/swh/CMakeLists.txt
@@ -31,6 +31,10 @@ FOREACH(_item ${XML_SOURCES})
 		VERBATIM
 	)
 
+	set_source_files_properties("${_out_file}" PROPERTIES
+		SKIP_AUTOUIC ON
+	)
+
 	# Add a library target for this C file, which depends on success of makestup.pl
 	ADD_LIBRARY("${_plugin}" MODULE "${_out_file}")
 
diff --git a/plugins/Sid/resid/CMakeLists.txt b/plugins/Sid/resid/CMakeLists.txt
index bb39e3d16..514776390 100644
--- a/plugins/Sid/resid/CMakeLists.txt
+++ b/plugins/Sid/resid/CMakeLists.txt
@@ -56,6 +56,9 @@ foreach(WAVE_DATA IN LISTS RESID_WAVES)
 		"${WAVE_SRC_OUT}"
 	)
 	add_custom_command(OUTPUT ${WAVE_SRC_OUT} COMMAND ${WAVE_COMMAND} VERBATIM)
+	set_source_files_properties("${WAVE_SRC_OUT}" PROPERTIES
+		SKIP_AUTOUIC ON
+	)
 	target_sources(resid_objects PUBLIC ${WAVE_SRC_OUT})
 endforeach()
 

[DomClark]

Changes look good now. I would still remove UICFILES <UI_FILES_TO_COMPILE> from the documentation of build_plugin (line 4 of BuildPlugin.cmake), though.

[Rossmaxx]

I would still remove UICFILES <UI_FILES_TO_COMPILE> from the documentation of build_plugin

Done