automatic detection can still work.

*Automatic detection plus error when specified explicitly.

In D6337#118612, @yuyichao wrote:

The behavior was broken in f52f5955f2fab3cd00b2b476957e9f017fc9f5a6. I think this part should just be reverted so that automatic detection can still work.

Hmm, why am I only hearing that now, and what exactly does it break? FWIW, I do indeed always configure with the desired settings for ENABLE_GTK2, ENABLE_QT4 and ENABLE_QT5, and I'm seeing those variables show up in the CMakeCache as option variables. Which is exactly what my commit intended to introduce, IIRC.

I'm fine with any changes in principle as long as that remains possible and the toplevel CMake file continues to include the gtk2, qt4 and/or qt5 directories only when the respective ENABLE flag is set. (That was another part of the cited commit.)

Hmm, why am I only hearing that now

It's only useful for people that doesn't have all dependencies installed, which is why it's hard to notice.

and what exactly does it break?

The variables should not be in the cache. This way it's possible to give error when it's specified explicitly and disable the features if it was just enabled by default.

I'm fine with any changes in principle as long as that remains possible and the toplevel CMake file continues to include the gtk2, qt4 and/or qt5 directories only when the respective ENABLE flag is set. (That was another part of the cited commit.)

That's more or less OK though the CMake files were carefully written so that there should be no error when the disabled directories are included. This makes it easy to explicitly collect information of all directories. We are not doing that so it's fine .....

Also, what's the error mentioned in edebb86de2e2818f831cd2ac1967f12b83645b33 ? Does the find_package call errors even with QUIET specified? Conditionally running it should't cause much issue (qt4 is not the only one that's looking at kde4 settings but it probably doesn't matter too much at this point) the default value of enable_kde4/kf5 shouldn't need to depend on qt4/qt5 settings. That should remove cmake_dependent_option.

Errors like: Error at qt5/CMakeLists.txt:34 (find_package):

By not providing "FindQt5Svg.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "Qt5Svg", but
CMake did not find one.

Could not find a package configuration file provided by "Qt5Svg" with any
of the following names:

  Qt5SvgConfig.cmake
  qt5svg-config.cmake

Add the installation prefix of "Qt5Svg" to CMAKE_PREFIX_PATH or set
"Qt5Svg_DIR" to a directory containing one of the above files.  If "Qt5Svg"
provides a separate development package or SDK, be sure it has been
installed.

I'll do a part revert of the commit you said, and just change the option for set().

Indeed, I have all dependencies installed...

and what exactly does it break?

The variables should not be in the cache. This way it's possible to give error when it's specified explicitly and disable the features if it was just enabled by default.

Hmmm, option variable usually *are* stored in the cache (so that their setting persists between CMake invocations), but they indeed do have to be set to a default variable. It's possible my change didn't do that properly.

That's more or less OK though the CMake files were carefully written so that there should be no error when the disabled directories are included. This makes it easy to explicitly collect information of all directories. We are not doing that so it's fine .....

I know, but I recall having a reason not to include them at all. Maybe that's just to prevent warnings, but it can also be I added it for packagers, to help avoid spurious dependencies (building components just because their dependencies are satisfied). Should also help keeping the specific CMake files simpler.

Hmmm, option variable usually *are* stored in the cache (so that their setting persists between CMake invocations), but they indeed do have to be set to a default variable. It's possible my change didn't do that properly.

options create cached variables and so is command line flags. Options in general doesn't need to be in the cache since the absense of it in the cache is a valid and important input state. They do need to be given a value (could be empty) but they don't have to be given a default value. The old version simply make sure there is a value set for it and that should be all what's needed.

Errors like: Error at qt5/CMakeLists.txt:34 (find_package):

Sounds like the qt5 auto detection is not done properly. Should be fine for now since I doubt many people are building with qt5 disabled these days.....

Yichao Yu wrote on 20170622::12:30:25 re: "D6337: Consistently check for Qt dependencies"

The old version simply make sure there is a value set for it and that should be all what's needed.

It's a pity I didn't document why I made my modification. I can't remember now, except that I did have a valid reason. Or one that was at the time.