coin3d / quarter

Coin GUI binding for Qt
BSD 3-Clause "New" or "Revised" License
39 stars 28 forks source link

Build issue of qthelp documentation if Qt binaries are not in PATH #57

Open waebbl opened 2 years ago

waebbl commented 2 years ago

We encountered an issue with building the QtHelp documentation, if the Qt related binaries are not available in PATH[1]:

-- Found Doxygen: /usr/bin/doxygen (found version "1.9.3") found components: doxygen missing components: dot
CMake Error at CMakeLists.txt:213 (message):
  Missing program Qt qhelpgenerator

-- Configuring incomplete, errors occurred!

As a patch[2], I'm using qmake to get the location of Qt binaries and successfully configure the package.

WDYT?

[1] https://bugs.gentoo.org/836340 [2] https://github.com/gentoo/gentoo/pull/24866/files#diff-09795807d0be82f2cc9b7d3c5c6fc1e66ff899bbdfd580bd56490579c4bfb055

VolkerEnderlein commented 2 years ago

Looks good to me but we would then tie the CMakeLists.txt to Qt5 as we only check for Qt5:qmake. We need to check also for Qt::qmake as well as Qt6::qmake depending on the Qt version used. Will have a closer look on this

VolkerEnderlein commented 2 years ago

I would propose following fragment in Quarter's top level CMakeLists.txt file. This avoids the find_program call at all for Qt5 and Qt6 and uses the information from the Qt's CMake config files. Only in case of Qt4 one needs to query the path of QMake and try to find the qhelpgenerator program.

if(QUARTER_USE_QT6)
  find_package(Qt6 COMPONENTS Widgets UiTools OpenGL OpenGLWidgets QUIET)
  if(QUARTER_BUILD_PLUGIN)
    find_package(Qt6 COMPONENTS Designer QUIET)
  endif()
  if(QUARTER_BUILD_DOC_QTHELP)
    find_package(Qt6 COMPONENTS Help QUIET)
    if(TARGET Qt6::qhelpgenerator)
      get_target_property(QHG_LOCATION Qt6::qhelpgenerator LOCATION)
    endif()
  endif()
endif()

if(NOT Qt6_FOUND AND QUARTER_USE_QT5)
  find_package(Qt5 COMPONENTS Widgets UiTools OpenGL QUIET)
  if(QUARTER_BUILD_PLUGIN)
    find_package(Qt5 COMPONENTS Designer QUIET)
  endif()
  if(QUARTER_BUILD_DOC_QTHELP)
    find_package(Qt5 COMPONENTS Help QUIET)
    if(TARGET Qt5::qhelpgenerator)
      get_target_property(QHG_LOCATION Qt5::qhelpgenerator LOCATION)
    endif()
  endif()
endif()

if(NOT Qt6_FOUND AND NOT Qt5_FOUND)
  set(QT_USE_IMPORTED_TARGETS ON)
  find_package(Qt4 COMPONENTS QtGui QtUiTools QtOpenGL REQUIRED)
  if(QUARTER_BUILD_PLUGIN)
    find_package(Qt4 COMPONENTS QtDesigner REQUIRED)
  endif()
  if(QUARTER_BUILD_DOC_QTHELP)
    find_package(Qt4 COMPONENTS QtHelp REQUIRED)
    get_filename_component(_qt_bin_dir "${QT_QMAKE_EXECUTABLE}" PATH)
    find_program(QHG_LOCATION NAMES qhelpgenerator DOC "Qt qhelpgenerator" HINTS "${_qt_bin_dir}")
  endif()
endif()

What do you think about this?

waebbl commented 2 years ago

I like the idea. It looks more robust than my patch. However, don't the lines

if(QUARTER_BUILD_DOC_QTHELP)
    find_package(Qt6 COMPONENTS Help QUIET)
    if(TARGET Qt6::qhelpgenerator)
      get_target_property(QHG_LOCATION Qt6::qhelpgenerator LOCATION)
    endif()
endif()

(similar for Qt{5,4}) need to be guarded by an additional if(QUARTER_BUILD_DOCUMENTATION), like it is in the current cmake file (line 206 and 241)? The symbol QUARTER_BUILD_DOC_QTHELP depends on the QUARTER_BUILD_DOCUMENTATION symbol to be defined (line 68 of the CMakeLists.txt file).

We could also think of a variable like QUARTER_QT_VERSION and use it to reduce the redudancy of the code:

if(QUARTER_USE_QT${QUARTER_QT_VERSION})
  find_package(Qt${QUARTER_QT_VERSION} COMPONENTS Widgets UiTools OpenGL OpenGLWidgets QUIET)
  if(QUARTER_BUILD_PLUGIN)
    find_package(Qt${QUARTER_QT_VERSION} COMPONENTS Designer QUIET)
  endif()
  if(QUARTER_BUILD_DOC_QTHELP)
    find_package(Qt${QUARTER_QT_VERSION} COMPONENTS Help QUIET)
    if(TARGET Qt${QUARTER_QT_VERSION}::qhelpgenerator)
      get_target_property(QHG_LOCATION Qt${QUARTER_QT_VERSION}::qhelpgenerator LOCATION)
    endif()
  endif()
endif()

It can be passed to cmake like -DQUARTER_QT_VERSION="6" with values 4, 5, 6 and auto. I've seen such an approach in the vtk package (https://github.com/Kitware/VTK/blob/master/CMake/vtkQt.cmake). We would still need some code to handle the basic componentes, as they differ between the Qt versions (OpenGLWidgets only in Qt6 and Qt4 having different names than Qt5 and Qt6)

VolkerEnderlein commented 2 years ago

Many thanks for your feedback.

QUARTER_BUILD_DOC_QTHELP is realized as cmake_dependent_option that is available only if QUARTER_BUILD_DOCUMENTATION is enabled. So this check should be sufficient IMHO.

I fully agree that we should simplify/generalize the boiler plate code for Qt5 and Qt6 detection. I'll have a closer look on the VTK solution.