Don't assume qdbus is in PATH #676
Conversation
CMakeLists.txt
Outdated
| @@ -74,7 +74,7 @@ if(WIN32) | |||
| add_subdirectory(src/qgittag) | |||
| endif() | |||
|
|
|||
| find_package(Qt5 REQUIRED COMPONENTS Widgets LinguistTools Concurrent) | |||
| find_package(Qt5 REQUIRED COMPONENTS Core Widgets LinguistTools Concurrent) | |||
There was a problem hiding this comment.
Why Core is required? If you require Widgets, Core is automatically included.
There was a problem hiding this comment.
Because Qt5::qmake is defined in Qt5CoreConfigExtras.cmake
There was a problem hiding this comment.
I assume it should work becaue Widgets already imports Core. I tried it on my machine and it works.
|
Looks like build on old Ubuntu fails. Maybe we could provide a temporary callback if ECM version is older? |
No need, ECM also has a solution for this case |
|
Great! Could you also try to remove |
|
I'm not fond of relying on transitive dependencies. A build system should require what it uses. |
|
I prefer to keep PRs minimal and change other things only if it's absolutely necessary. |
Since I'm not a developer, I'm a bit lost with all this stuff. So I want to ask if this PR can be merged? |
|
Tried to pushed the changes, but for some reason GitHub closed the PR. I suspect it's because it was done from |
We'll use ECM's ecm_query_qt macro to find where Qt executables are installed.
Fixes: #675