|
Notes |
|
|
(0035145)
|
|
Clinton Stimpson
|
|
2014-02-18 23:12
|
|
Thanks for the patch.
Can you modify it to keep the code for the static library case? |
|
|
|
(0035146)
|
|
Orion Poplawski
|
|
2014-02-18 23:43
|
|
|
I'll try - what's the proper test for dynamic/static - ENABLE_SHARED? |
|
|
|
(0035147)
|
|
Stephen Kelly
|
|
2014-02-19 03:49
|
|
|
This is surprising. For example, you need to use QtCore if you use QtGui, but your patch removes that dependency. Does it exist somewhere else? |
|
|
|
(0035149)
|
|
Clinton Stimpson
|
|
2014-02-19 09:19
|
|
Stephen, thanks for jumping in. The UseQt4.cmake file doesn't add include directories for the dependencies, but it does add the dependencies to the link. So, if using QtGui required the include directories for QtCore, the dependency would exist the user's CMakeLists.txt file.
Do you think it makes sense to add the link dependency without adding the include directories for shared Qt libraries?
Orion, FindQt4.cmake sets a QT_IS_STATIC variable for the static Qt case. |
|
|
|
(0035150)
|
|
Stephen Kelly
|
|
2014-02-19 09:32
|
|
Clinton, I think I need to understand the code before I understand your question. The foreach loop at the bottom of UseQt4.cmake seems to add includes and defines for dependencies. How do I map that to what you wrote?
> So, if using QtGui required the include directories for QtCore, the
> dependency would exist the user's CMakeLists.txt file.
How would that exist in the users CMakeLists.txt file? Please post a snippet. |
|
|
|
(0035151)
|
|
Clinton Stimpson
|
|
2014-02-19 11:16
|
|
The for loop adds include directories only if QT_USE_${module} is set.
It adds the link library if QT_USE_${module} is set or if it is a dependency for another module with that variable set.
For example:
cmake_minimum_required(VERSION 2.8)
find_package(Qt4 REQUIRED QtGui)
include(${QT_USE_FILE})
add_executable(testqt main.cpp)
target_link_libraries(testqt ${QT_LIBRARIES})
That will end up with an include directory with QtGui, but not QtCore, but will link with both QtGui and QtCore.
If this is used instead
find_package(Qt4 REQUIRED QtGui QtCore)
then the include directory for QtCore is added. |
|
|
|
(0035152)
|
|
Stephen Kelly
|
|
2014-02-19 11:50
|
|
That looks like a bug to my eyes. However, the better bug fix is to use the imported targets anyway.
I have no further comment :). |
|
|
|
(0035154)
|
|
Orion Poplawski
|
|
2014-02-19 15:29
|
|
How about this version - it's a lot simpler :).
--- cmake-2.8.12.2/Modules/UseQt4.cmake.qtdeps 2014-01-16 10:15:08.000000000 -0700
+++ cmake-2.8.12.2/Modules/UseQt4.cmake 2014-02-19 13:27:21.780219091 -0700
@@ -99,7 +99,9 @@ foreach(module QT3SUPPORT QTOPENGL QTASS
include_directories(SYSTEM ${QT_${module}_INCLUDE_DIR})
endif(QT_INCLUDE_DIRS_NO_SYSTEM)
endif()
- set(QT_LIBRARIES ${QT_LIBRARIES} ${QT_${module}_LIBRARY})
+ if(QT_USE_${module} OR QT_IS_STATIC)
+ set(QT_LIBRARIES ${QT_LIBRARIES} ${QT_${module}_LIBRARY})
+ endif()
set(QT_LIBRARIES_PLUGINS ${QT_LIBRARIES_PLUGINS} ${QT_${module}_PLUGINS})
if(QT_IS_STATIC)
set(QT_LIBRARIES ${QT_LIBRARIES} ${QT_${module}_LIB_DEPENDENCIES}) |
|
|
|
(0035155)
|
|
Clinton Stimpson
|
|
2014-02-19 15:48
|
|
Thanks, but that doesn't look quite correct.
I think we can just move the part that does set(*_DEPENDS 1) to inside the QT_IS_STATIC case. Does this work for you?
@@ -102,10 +102,10 @@ foreach(module QT3SUPPORT QTOPENGL QTASSISTANT QTDESIGNER QTMOTIF QTNSPLUGIN
set(QT_LIBRARIES_PLUGINS ${QT_LIBRARIES_PLUGINS} ${QT_${module}_PLUGINS})
if(QT_IS_STATIC)
set(QT_LIBRARIES ${QT_LIBRARIES} ${QT_${module}_LIB_DEPENDENCIES})
+ foreach(depend_module ${QT_${module}_MODULE_DEPENDS})
+ set(QT_USE_${depend_module}_DEPENDS 1)
+ endforeach()
endif()
- foreach(depend_module ${QT_${module}_MODULE_DEPENDS})
- set(QT_USE_${depend_module}_DEPENDS 1)
- endforeach()
else ()
message("Qt ${module} library not found.")
endif () |
|
|
|
(0035156)
|
|
Orion Poplawski
|
|
2014-02-19 16:00
|
|
|
Well, I think Stephan was correct in that we always want to add in any needed includes and defines. And I'm really just interested in preventing over linkage. So my version (in the note, not attachment - that was a mistaken reposting of the original) only links if we specifically ask for the module or we are building statically. |
|
|
|
(0035210)
|
|
Clinton Stimpson
|
|
2014-02-26 13:05
|
|
|
|
|
(0036918)
|
|
Robert Maynard
|
|
2014-10-06 10:32
|
|
|
Closing resolved issues that have not been updated in more than 4 months. |
|
cmake-qtdeps.patch (2,403) 2014-02-11 10:53