Skip to content

Use modern CMake syntax.#1273

Merged
danmar merged 2 commits into
cppcheck-opensource:masterfrom
susilehtola:qt5cmake
Jun 2, 2018
Merged

Use modern CMake syntax.#1273
danmar merged 2 commits into
cppcheck-opensource:masterfrom
susilehtola:qt5cmake

Conversation

@susilehtola
Copy link
Copy Markdown
Contributor

The qt5_use_modules syntax has been obsoleted and no longer works in up-to-date CMake (Fedora rawhide). This PR makes the program build again using standard CMake syntax.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jun 2, 2018

so we drop support for cmake 2.8.4 but 2.8.11 still works... I guess that is fine.

@danmar danmar merged commit f2fc38a into cppcheck-opensource:master Jun 2, 2018
if (BUILD_GUI)
set(GUI_QT_COMPONENTS Core Gui Widgets PrintSupport)
find_package(Qt5 COMPONENTS ${GUI_QT_COMPONENTS})
find_package(Qt5Core)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have you got other programming opinions than Marcus D. Hanwell?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Hanwell's code does not work in new CMake.

Hanwell's post is two years old.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is recent discussion in this in the blog post. Hanwell writes

The FindQt5.cmake is the “old way”, and it is still searched for. Qt 5 distributes CMake config files, such as Qt5Config.cmake that contain paths, macros, functions, etc. They are unfortunately a little deeper in the tree, so you would have to set Qt5_DIR to “/usr/lib64/cmake/Qt5” in my case.

So, why replace one line of code with two, especially if the two lines of code doesn't even work by default? 🤣

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Do you interpret a bit of information in the wrong direction?
  • Would you like to recheck the involved software versions?
  • I am using the command “find_package(Qt5 COMPONENTS Core Widgets PrintSupport Sql REQUIRED)” based on the software “CMake 3.12.0-2.1” and “Qt 5.11.2-1.1” without build error messages for my recent development experiments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* Do you interpret a bit of information in the wrong direction?

Looks like you have. Old version has two lines of code that didn't work, my version has one line line of code that works without tuning the build system at all.

* Would you like to recheck the involved software versions?

DEBUG util.py:439: cmake x86_64 3.11.2-1.fc28 build 7.7 M
DEBUG util.py:439: qt5-devel noarch 5.10.1-1.fc28 build 9.9 k

* I am using the command “`find_package(Qt5 COMPONENTS Core Widgets PrintSupport Sql REQUIRED)`” based on the software “[CMake 3.12.0-2.1](https://cmake.org/cmake/help/v3.13/command/find_package.html#basic-signature-and-module-mode)” and “[Qt 5.11.2-1.1](https://doc.qt.io/qt-5/cmake-manual.html)” without build error messages for my recent development experiments.

This literally holds no information. You're presumably not using a vanilla system.

Picking a fight over A SINGLE LINE OF CODE just makes no sense. Does my version not work for you? Open up a ticket.

If you want to see for yourself that the old version did not work, install Fedora and do it.

I have no time for this sort of time waste.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old version has two lines of code that didn't work,

I am curious to clarify your different observations from other run time environments a bit more.
Should it usually work if the needed software dependencies were fulfilled?

my version has one line line of code

You split a list of components in a CMake variable into separate calls of the command “find_package”, didn't you?
Will a single function call become nicer in the script for Cppcheck's graphical user interface?

cmake x86_64 3.11.2-1.fc28

The package source from Fedora 28 should be sufficient while I can share experiences from openSUSE Tumbleweed.

This literally holds no information.

This command example indicates that I got the Qt SQL support working for my development system.

Open up a ticket.

Another clarification is evolving for these build scripts.

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.

3 participants