Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix QT_MAJOR_VERSION to QT_VERSION_MAJOR #916

Merged
merged 1 commit into from
Mar 21, 2024
Merged

Conversation

dantti
Copy link
Member

@dantti dantti commented Jan 12, 2024

No description provided.

@ferdnyc
Copy link
Contributor

ferdnyc commented Jan 14, 2024

In #920 (comment) @dantti wrote:

This is a duplicate of #916 but was not commited due internal conversation that ECM uses the current form, so far both options work, maybe due qt versionless targets or that this doesn't matter, still CI is happy so I would merge as other files use this form.

Agreed, both options work only because ${QT_MAJOR_VERSION} ends up undefined in the GammaRay build, and CMake replaces undefined vars with the empty string. (The line could be written as simply if(Qt::Quick), as that's what it's effectively been all along.)

The larger conversation about using ECM to probe Qt versions is sort of tangential, because if that's going to be done it would make sense to change lots of ${QT_VERSION_MAJOR} to ${QT_MAJOR_VERSION}. Having this one outlier makes no sense regardless.

(IMHO, re: that larger question, it looks like the only thing from ECM that GammaRay actually uses anymore is cmake/ECM/modules/ECMGeneratePri.cmake,1 which is written to use its ECMQueryQt.cmake iff KDE_INSTALL_USE_QT_SYS_PATHS is set true, or not defined at all. The main GammaRay CMakeLists.txt explicitly does a set(KDE_INSTALL_USE_QT_SYS_PATHS OFF) to avoid triggering use of ECMQueryQt.cmake. Conclusion: The ECM Qt support logic isn't used or needed by GammaRay at all. Integrating it would be more trouble than it's worth.)

  1. (And cmake/ECM/find-modules/FindWayland.cmake, which isn't related to Qt.)

@ferdnyc
Copy link
Contributor

ferdnyc commented Jan 14, 2024

(And, actually it's not true that both options currently work. On my system, with both Qt5 and Qt6 installed to /usr/lib64/, if I specify only QT_MAJOR_VERSION=5, I still end up with Qt6:)

$ cmake -B _qt_major5 -S . -DQT_MAJOR_VERSION=5
-- The CXX compiler identification is GNU 13.2.1
-- The C compiler identification is GNU 13.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/lib64/ccache/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/lib64/ccache/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Setting build type to Debug as none was specified.
-- Found libdw: /usr/lib64/libdw.so  
-- Found libbfd: /usr/lib64/libbfd.so  
-- Could NOT find libdwarf (missing: LIBDWARF_LIBRARY LIBDWARF_INCLUDE_DIR) 
-- Found Backward: /home/ferd/rpmbuild/REPOS/GammaRay/worktrees/master/3rdparty/backward-cpp  
-- Found Git: /usr/bin/git (found version "2.43.0") 
-- Building GammaRay 3.0.95 (revision: a5f0b92b5) in Debug mode
-- Performing Test C_SUPPORTS_UNUSED_BUT_SET
-- Performing Test C_SUPPORTS_UNUSED_BUT_SET - Success
-- Performing Test CXX_SUPPORTS_UNUSED_BUT_SET
-- Performing Test CXX_SUPPORTS_UNUSED_BUT_SET - Success
-- Performing Test C_SUPPORTS_LOGICAL_OP
-- Performing Test C_SUPPORTS_LOGICAL_OP - Success
-- Performing Test CXX_SUPPORTS_LOGICAL_OP
-- Performing Test CXX_SUPPORTS_LOGICAL_OP - Success
-- Performing Test C_SUPPORTS_POINTER_MEMACCESS
-- Performing Test C_SUPPORTS_POINTER_MEMACCESS - Success
-- Performing Test CXX_SUPPORTS_POINTER_MEMACCESS
-- Performing Test CXX_SUPPORTS_POINTER_MEMACCESS - Success
-- Performing Test C_SUPPORTS_REORDER
-- Performing Test C_SUPPORTS_REORDER - Failed
-- Performing Test CXX_SUPPORTS_REORDER
-- Performing Test CXX_SUPPORTS_REORDER - Success
-- Performing Test C_SUPPORTS_FORMAT_SECURITY
-- Performing Test C_SUPPORTS_FORMAT_SECURITY - Failed
-- Performing Test CXX_SUPPORTS_FORMAT_SECURITY
-- Performing Test CXX_SUPPORTS_FORMAT_SECURITY - Failed
-- Performing Test C_SUPPORTS_SUGGEST_OVERRIDE
-- Performing Test C_SUPPORTS_SUGGEST_OVERRIDE - Failed
-- Performing Test CXX_SUPPORTS_SUGGEST_OVERRIDE
-- Performing Test CXX_SUPPORTS_SUGGEST_OVERRIDE - Success
-- Performing Test HAVE_GXX_GNUXX11
-- Performing Test HAVE_GXX_GNUXX11 - Success
-- Performing Test HAVE_GXX_CXX11
-- Performing Test HAVE_GXX_CXX11 - Success
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- Found OpenGL: /usr/lib64/libOpenGL.so   
-- Found WrapOpenGL: TRUE  
-- Found XKB: /usr/lib64/libxkbcommon.so (found suitable version "1.6.0", minimum required is "0.5.0") 
-- Found WrapVulkanHeaders: /usr/include  
-- Performing Test HAVE_STDATOMIC
-- Performing Test HAVE_STDATOMIC - Success
-- Found WrapAtomic: TRUE  
-- Found Wayland_Client: /usr/lib64/libwayland-client.so (found version "1.22.0") 
-- Found Wayland_Server: /usr/lib64/libwayland-server.so (found version "1.22.0") 
-- Found Wayland_Cursor: /usr/lib64/libwayland-cursor.so (found version "1.22.0") 
-- Found Wayland_Egl: /usr/lib64/libwayland-egl.so (found version "18.1.0") 
-- Performing Test C_SUPPORTS_ZERO_AS_NULL_POINTER_CONSTANT
-- Performing Test C_SUPPORTS_ZERO_AS_NULL_POINTER_CONSTANT - Failed
-- Performing Test CXX_SUPPORTS_ZERO_AS_NULL_POINTER_CONSTANT
-- Performing Test CXX_SUPPORTS_ZERO_AS_NULL_POINTER_CONSTANT - Success
-- Looking for QT_NO_OPENGL
-- Looking for QT_NO_OPENGL - not found
-- Looking for include file stdint.h
-- Looking for include file stdint.h - found
-- Looking for include file unistd.h
-- Looking for include file unistd.h - found
-- Looking for backtrace
-- Looking for backtrace - found
-- Looking for abi::__cxa_demangle
-- Looking for abi::__cxa_demangle - found
-- Building probe for ABI: qt6_6-x86_64 (Debug)
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_VISIBILITY - Success
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY
-- Performing Test COMPILER_HAS_HIDDEN_INLINE_VISIBILITY - Success
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
-- Performing Test HAVE_NO_CXX_ZERO_AS_NULL_POINTER_CON
-- Performing Test HAVE_NO_CXX_ZERO_AS_NULL_POINTER_CON - Success
-- Performing Test HAVE_NO_CXX_UNNEEDED_INTERNAL_DEC
-- Performing Test HAVE_NO_CXX_UNNEEDED_INTERNAL_DEC - Success
-- Performing Test HAVE_NO_CXX_NULL_CONVERSION
-- Performing Test HAVE_NO_CXX_NULL_CONVERSION - Success
-- WARNING: Skipping the translatortest since the translations are not installed.
manual/shadereffect.vert -> manual/shadereffect.vert.qsb exposed as ://manual/shadereffect.vert.qsb
manual/shadereffect.frag -> manual/shadereffect.frag.qsb exposed as ://manual/shadereffect.frag.qsb
manual/shadereffect.vert -> manual/shadereffect.vert.qsb exposed as ://manual/shadereffect.vert.qsb
manual/shadereffect.frag -> manual/shadereffect.frag.qsb exposed as ://manual/shadereffect.frag.qsb
-- Found Wayland: /usr/lib64/libwayland-client.so;/usr/lib64/libwayland-server.so;/usr/lib64/libwayland-cursor.so;/usr/lib64/libwayland-egl.so (found suitable version "1.22.0", minimum required is "1.12") found components: Server 
-- Couldn't find index dir, manual won't link to system documentation
-- Found Doxygen: /usr/bin/doxygen (found version "1.9.7") found components: doxygen dot 
-- The following features have been enabled:

 * Option GAMMARAY_BUILD_UI, Build the GammaRay client and in-process UI.
 * Option GAMMARAY_MULTI_BUILD, Build multiple applicable probe configurations.
 * Option GAMMARAY_BUILD_CLI_INJECTOR, Build command line injector on Windows.
 * Option GAMMARAY_BUILD_DOCS, Build GammaRay documentation.
 * Widget .ui file export, Requires QtDesigner library.
 * ELF ABI detection, Automatic probe ABI detection on ELF-based systems. Requires elf.h.
 * User Manual creation in qch format, Requires qdoc, qhelpgenerator, qtattributionsscanner, the qdoc templates and index files

-- The following OPTIONAL packages have been found:

 * Git
   Determine exact build version.
 * OpenGL
 * XKB (required version >= 0.5.0), XKB API common to servers and clients., <http://xkbcommon.org>
 * Vulkan
 * Qt6GuiTools (required version >= 6.6.0)
 * Qt6CoreTools (required version >= 6.6.0)
 * Qt6DBusTools (required version >= 6.6.0)
 * Qt6Gui
 * Qt6Network
 * WrapVulkanHeaders
 * Qt63DAnimation
 * Qt63DExtras
 * Qt6QmlTools (required version >= 6.6.0)
 * Qt63DQuick
 * Qt6Bluetooth
 * Qt6WidgetsTools (required version >= 6.6.0)
 * Qt6Designer
 * Qt6Positioning
 * Qt6QuickWidgets
 * Qt6Svg
   Required for widget SVG export.
 * Qt6Test
 * Qt6WebEngineCoreTools (required version >= 6.6.0)
 * Qt6WebEngineWidgets
 * Qt6WaylandScannerTools (required version >= 6.6.0)
 * Qt6WaylandCompositor
 * Qt6ScxmlTools (required version >= 6.6.0)
 * Qt6Scxml (required version == 6.6.0)
 * Qt6StateMachine (required version == 6.6.0)
 * Qt6Help
 * Qt6LinguistTools
 * QmlLint, <https://qt.io>
   Validate QML code.
 * Qt6ShaderToolsTools (required version >= 6.6.0)
 * Qt6ShaderTools
 * PkgConfig
 * Wayland (required version >= 1.12), C library implementation of the Wayland protocol: a protocol for a compositor to talk to its clients, <https://wayland.freedesktop.org/>
   Needed for the QtWayland compositor inspector plug-in.
 * Qt6ToolsTools
 * Doxygen, API Documentation system, <https://www.doxygen.org>
   Needed to build the API documentation.

-- The following RECOMMENDED packages have been found:

 * pod2man, Man page generator
   Generate GammaRay man pages.

-- The following REQUIRED packages have been found:

 * QT (required version >= 5.15)
 * Qt6, <https://qt.io/>

-- The following features have been disabled:

 * Option GAMMARAY_PROBE_ONLY_BUILD, Build only an additional probe configuration for an already existing launcher.
 * Option GAMMARAY_CLIENT_ONLY_BUILD, Build the client part only.
 * Option GAMMARAY_INSTALL_QT_LAYOUT, Install into Qt directory layout.
 * Option GAMMARAY_STATIC_PROBE, Build the probe as static library for compile-time injection.
 * Option GAMMARAY_ENFORCE_QT_ASSERTS, Force QT_ASSERT in all builds.
 * Option GAMMARAY_DISABLE_FEEDBACK, Disable user feedback support.
 * Option GAMMARAY_USE_PCH, Enable Precompiled Headers support
 * Option ENABLE_GOLD_LINKER, Use GNU gold linker
 * Option GAMMARAY_CORE_ONLY_LAUNCHER, Only use QtCore in the CLI launcher (breaks style injector, but is needed for Boot2Qt compatibility)
 * QtScript debugger, Requires QtScript and QtScriptTools.

-- The following OPTIONAL packages have not been found:

 * Qt6QmlCompilerPlusPrivate
 * Qt6Location
 * Qt6Script
 * Qt6ScriptTools
 * Qt6IviCore (required version >= 1.1)
 * Qt6IviVehicleFunctions (required version >= 1.1)
 * Qt6IviMedia (required version >= 1.1)
 * KF6CoreAddons, KDE KCoreAddons Framework, <https://www.kde.org/>
   Required for the KJob tracker plug-in.

-- The following RECOMMENDED packages have not been found:

 * KF6SyntaxHighlighting (required version >= 5.28), <https://www.kde.org/>
   Syntax highlighting for code editor.
 * KDSME-qt6 (required version >= 1.2), KDAB State Machine Editor framework, <https://github.com/KDAB/KDStateMachineEditor>
   Graphical state machine debugging.

-- Configuring done (10.5s)
-- Generating done (0.7s)
-- Build files have been written to: /home/ferd/rpmbuild/REPOS/GammaRay/worktrees/master/_qt_major5

If I were to build that, I'd presumably get the Qt6 quickinspector without quickimplicitbindingdependencyprovider.cpp compiled in, because the if(TARGET Qt5::Quick) test would fail.

@redstrate
Copy link
Contributor

I think this makes sense right? QT_MAJOR_VERSION is being set by the user (if needed at all) but here it should be QT_VERSION_MAJOR because that's coming from the target. If it found Qt 6.5.0, QT_VERSION_MAJOR should be 6 and for something like 5.15.0 it would be 5.

@winterz
Copy link
Contributor

winterz commented Mar 21, 2024

Yes, this patch is correct.

@redstrate could you look into removing any unnecessary cmake/ECM/foo files?

@winterz winterz merged commit b240401 into master Mar 21, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants