Qt6 + ubuntu #4679

daschuer · 2022-02-15T14:58:40Z

This allows to build and run Mixxx on Ubuntu using https://launchpad.net/~daschuer/+archive/ubuntu/qt6-backports

The main issue was that CMake < 3.21 requires to use qt_finalize_target(mixxx) to ensure that the resources are all linked at the beginning of the mixxx binary.

uklotzde · 2022-02-15T17:07:56Z

CMakeLists.txt

@@ -1256,7 +1256,12 @@ elseif(UNIX)
 endif()

 # The mixxx executable
-add_executable(mixxx WIN32 src/main.cpp)
+if(QT7 AND CMAKE_VERSION VERSION_LESS "3.21")


Ups, that is a debug leftover.

I think it is the best to rebase this out.

Holzhaus · 2022-02-15T18:34:12Z

CMakeLists.txt

-add_executable(mixxx WIN32 src/main.cpp)
+if(QT7 AND CMAKE_VERSION VERSION_LESS "3.21")
+  find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions
+  qt_add_executable(mixxx WIN32 src/main.cpp MANUAL_FINALIZATION)


Please add a comment that explains why this is necessary.

In my test with cmake 3.16 it was actually not necessary.
It is just mentioned as the counterpart to the required qt-finalize-target which takes care of object order during linking. See https://doc.qt.io/qt-6/qt-finalize-target.html#qt6-finalize-target
So I think it is better to add it and follow the docs.

From the docs I don't see any indication why this is only necessary on cmake < 3.21?

"This is especially important if using a statically built Qt or a CMake version earlier than 3.21."

There is also a pending PR that removes the related tests in the QT CI for cmake > 3.21. Unfortunately I do not have saved the link.

I read that too, but in my understanding it means "It's always important, but especially important on CMake < 3.21". So I don't think we should use a version guard here. Less code paths = less chances we run into weird bugs that CI doesn't detect or that we can't reproduce.

I tested it with this patch on CMake 3.22.2 and it works fine:

diff --git a/CMakeLists.txt b/CMakeLists.txt index c7c7743072..f0f31773ea 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1265,12 +1265,10 @@ elseif(UNIX) endif() # The mixxx executable -if(QT6 AND CMAKE_VERSION VERSION_LESS "3.21") +if(QT6) find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions # qt_add_executable() is the recommended initial call for qt_finalize_target() # below that takes care of the correct object order in the resulting binary - # According to https://doc.qt.io/qt-6/qt-finalize-target.html it is importand for - # builds with Qt < 3.21 qt_add_executable(mixxx WIN32 src/main.cpp MANUAL_FINALIZATION) else() add_executable(mixxx WIN32 src/main.cpp) @@ -2208,11 +2206,9 @@ if(QT6) ) target_link_libraries(mixxx-lib PRIVATE mixxx-qml-mixxxcontrolsplugin) - if(CMAKE_VERSION VERSION_LESS "3.21") - # qt_finalize_target takes care that the resources :/mixxx.org/imports/Mixxx/ - # and :/mixxx.org/imports/Mixxx/Controls are placed into beginning of the binary - qt_finalize_target(mixxx) - endif() + # qt_finalize_target takes care that the resources :/mixxx.org/imports/Mixxx/ + # and :/mixxx.org/imports/Mixxx/Controls are placed into beginning of the binary + qt_finalize_target(mixxx) endif() target_compile_definitions(mixxx-lib PUBLIC QT_TABLET_SUPPORT QT_USE_QSTRINGBUILDER)

daschuer · 2022-02-15T19:16:20Z

Done

ywwg · 2022-02-15T19:45:35Z

CMakeLists.txt

-add_executable(mixxx WIN32 src/main.cpp)
+if(QT6 AND CMAKE_VERSION VERSION_LESS "3.21")
+  find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions
+  # qt_add_executable() is the recomendet initial call for qt_finalize_target()


typo

Suggested change

# qt_add_executable() is the recomendet initial call for qt_finalize_target()

# qt_add_executable() is the recomended initial call for qt_finalize_target()

ywwg · 2022-02-15T19:47:34Z

I get a bunch of this warning on my system for /usr/bin/cmake .. -DCMAKE_BUILD_TYPE=Debug -DOPTIMIZE=native -DQT6=on

CMake Warning (dev) in CMakeLists.txt:
  Policy CMP0099 is not set: Link properties are transitive over private
  dependency on static libraries.  Run "cmake --help-policy CMP0099" for
  policy details.  Use the cmake_policy command to set the policy and
  suppress this warning.
This warning is for project developers.  Use -Wno-dev to suppress it.

daschuer · 2022-02-15T21:35:04Z

Regarding CMP0099, Qt deals with it if available and works around if it is not, so we can safely enable it since the old behavior is deprecated.

Holzhaus · 2022-02-16T00:08:23Z

CMakeLists.txt

@@ -25,6 +25,14 @@ if(POLICY CMP0071)
  cmake_policy(SET CMP0071 NEW)
 endif()

+# propagate interface link properties
+if(POLICY CMP0099)
+  # this avoids a warning when qt deals with it see


Deals with what?

Holzhaus · 2022-02-16T00:12:44Z

CMakeLists.txt

@@ -2178,7 +2194,7 @@ if(QT6)
    URI Mixxx.Controls
    VERSION 0.1
    RESOURCE_PREFIX /mixxx.org/imports
-    OPTIONAL_IMPORTS Mixxx
+    IMPORTS Mixxx


This looks wrong, not every type in that module imports Mixxx. Also, why is this even part of this PR? It's not necessary to fix Mixxx on Ubuntu, right?

It was necessary at one point during debugging. Now it still works even if I ommit the line entirely so it can be reverted.

Holzhaus · 2022-02-16T00:15:11Z

CMakeLists.txt

-add_executable(mixxx WIN32 src/main.cpp)
+if(QT7 AND CMAKE_VERSION VERSION_LESS "3.21")
+  find_package(Qt6 REQUIRED COMPONENTS Core) # For Qt Core cmake functions
+  qt_add_executable(mixxx WIN32 src/main.cpp MANUAL_FINALIZATION)


From the docs I don't see any indication why this is only necessary on cmake < 3.21?

…se of it

daschuer · 2022-02-16T07:21:42Z

Done.

ywwg · 2022-02-16T15:03:26Z

works on my machine, thanks!

… if it is really no longer required with cmake >= 3.21

Holzhaus

LGTM, thank you. Waiting for CI.

Adjust qml libraries import statement.

7e61781

github-actions bot added the build label Feb 15, 2022

uklotzde reviewed Feb 15, 2022

View reviewed changes

Holzhaus requested changes Feb 15, 2022

View reviewed changes

daschuer added 3 commits February 15, 2022 19:58

Finalize mixxx binary manually for cmake < 3.21

0ce2ec9

Import MixxxPlugin

4e1d0e7

Add missing virtual destructor overload

fd954c6

daschuer force-pushed the qt6_ubuntu branch from a7dc558 to 12d8325 Compare February 15, 2022 19:15

daschuer mentioned this pull request Feb 15, 2022

QML: Disable automatic qmldir generation to work around QTBUG-100326 #4673

Merged

ywwg reviewed Feb 15, 2022

View reviewed changes

Added a comment why we use qt_add_executable()

67dde53

daschuer force-pushed the qt6_ubuntu branch from 12d8325 to 67dde53 Compare February 15, 2022 20:26

SET CMP0099 NEW to avoid a warning

224aa13

Holzhaus requested changes Feb 16, 2022

View reviewed changes

daschuer added 2 commits February 16, 2022 07:54

Import Mixxx Module als OPTIONAL_IMPORTS because not all types make u…

6e3ea5b

…se of it

Improve comments around finalizing

77125f4

daschuer added 3 commits February 16, 2022 18:52

Merge remote-tracking branch 'upstream/main' into qt6_ubuntu

1cda499

Update res/qml/Mixxx/qmldir from auto-generated version

83c710a

Remove version guard around qt_add_executable because we are not sure…

1989638

… if it is really no longer required with cmake >= 3.21

Holzhaus approved these changes Feb 16, 2022

View reviewed changes

Holzhaus merged commit d67b057 into mixxxdj:main Feb 16, 2022

daschuer deleted the qt6_ubuntu branch April 14, 2022 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qt6 + ubuntu #4679

Qt6 + ubuntu #4679

daschuer commented Feb 15, 2022

uklotzde Feb 15, 2022

daschuer Feb 15, 2022

daschuer Feb 15, 2022

Holzhaus Feb 15, 2022

daschuer Feb 15, 2022

Holzhaus Feb 16, 2022 •

edited

Loading

daschuer Feb 16, 2022

Holzhaus Feb 16, 2022

daschuer commented Feb 15, 2022

ywwg Feb 15, 2022

ywwg commented Feb 15, 2022

daschuer commented Feb 15, 2022

Holzhaus Feb 16, 2022

Holzhaus Feb 16, 2022

daschuer Feb 16, 2022

Holzhaus Feb 16, 2022 •

edited

Loading

daschuer commented Feb 16, 2022

ywwg commented Feb 16, 2022

Holzhaus left a comment

	# qt_add_executable() is the recomendet initial call for qt_finalize_target()
	# qt_add_executable() is the recomended initial call for qt_finalize_target()

Qt6 + ubuntu #4679

Qt6 + ubuntu #4679

Conversation

daschuer commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Holzhaus Feb 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Feb 15, 2022

Choose a reason for hiding this comment

ywwg commented Feb 15, 2022

daschuer commented Feb 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Holzhaus Feb 16, 2022 • edited Loading

Choose a reason for hiding this comment

daschuer commented Feb 16, 2022

ywwg commented Feb 16, 2022

Holzhaus left a comment

Choose a reason for hiding this comment

Holzhaus Feb 16, 2022 •

edited

Loading

Holzhaus Feb 16, 2022 •

edited

Loading