Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs build: Prefer CMake targets for build tools #794

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented May 19, 2023

Instead of using find_program() to discover tools like qdoc, qhelpgenerator, and qtattributionsscanner, default to picking up those tools' locations from the relevant CMake IMPORTED executable targets.

Fall back to find_program() for any targets that don't exist, to preserve compatibility with Qt 5.12 and earlier.

I've successfully used the code on this branch to build GammaRay, complete with all documentation including the collections file, on:

  • Ubuntu 20.04, using no options at all to configure for Qt 5.12:
    cmake -B _build -S .
    cmake --build _build

Because of the fallbacks, this build actually configures identically to the current master branch code.

  • Fedora 38, setting only GAMMARAY_QT6_BUILD and QDOC_INDEX_DIR for Qt 6.5. (Because nothing can change the fact that Fedora doesn't ship a qtcore.index with their Qt6 documentation packages.)
    cmake -B _build -S . -DGAMMARAY_QT6_BUILD=1 -DQDOC_INDEX_DIR=/usr/share/doc/qt6
    cmake --build _build

Normally on Fedora 38, (using the master branch), at least -DQT_INSTALL_BINS=/usr/lib64/qt6/libexec would also have to be set, because qtattributionsscanner is not on the $PATH. But the target created by Qt6ToolsTools allows it to be referenced in CMake as Qt::qtattributionsscanner.

Whereas for qdoc, (again on Fedora) /usr/bin/qdoc is the Qt5 version; the Qt6 version is named /usr/bin/qdoc-qt6. Using a find_program()-discovered /usr/bin/qdoc in a Qt6 build will cause it to fail. The Qt::qdoc CMake target, however, references the correct version of the tool for any Qt release in which it's available (which is Qt 5.13+).

Comment on lines 18 to 35
if (TARGET Qt::qdoc)
set(QDOC_EXECUTABLE "$<TARGET_FILE:Qt::qdoc>")
else()
find_program(QDOC_EXECUTABLE qdoc HINTS ${QT_INSTALL_BINS})
endif()

if (TARGET Qt::qhelpgenerator)
# Required for Doxygen
get_target_property(QHELPGEN_EXECUTABLE Qt::qhelpgenerator IMPORTED_LOCATION)
else()
find_program(QHELPGEN_EXECUTABLE qhelpgenerator HINTS ${QT_INSTALL_BINS})
endif()

if (TARGET Qt::qtattributionsscanner)
set(QTATTRIBUTIONSSCANNER_EXECUTABLE Qt::qtattributionsscanner)
else()
find_program(QTATTRIBUTIONSSCANNER_EXECUTABLE qtattributionsscanner HINTS ${QT_INSTALL_BINS})
endif()
Copy link
Contributor Author

@ferdnyc ferdnyc May 19, 2023

Choose a reason for hiding this comment

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

Three different tools, three different ways of consuming the target. Because:

  1. qdoc is used in add_custom_command() as an argument to ${CMAKE_COMMAND} -E env ... commands, meaning that the actual path to the binary has to be supplied. $<TARGET_FILE:Qt::qdoc> is the recommended method of doing so, according to the CMake docs for add_custom_command.

    Arguments to COMMAND may use generator expressions. Use the TARGET_FILE generator expression to refer to the location of a target later in the command line (i.e. as a command argument rather than as the command to execute).

  2. Qt::qtattributionsscanner is used directly as the COMMAND supplied to add_custom_command(), which means that the target name can be used as the command to execute.

  3. The Qt::qhelpgenerator binary path needs to be supplied directly in the QHELPGEN_EXECUTABLE variable, as it'll be substituted into the generated Doxyfile for the API doc generation.

Comment on lines -47 to +81
message(STATUS "qdoc template dir: ${QDOC_TEMPLATE_DIR}")
message(STATUS "qdoc index dir: ${QDOC_INDEX_DIR}")
message(STATUS "qdoc template: ${QDOC_TEMPLATE}")
message(STATUS "qdoc index: ${QDOC_INDEX}")
Copy link
Contributor Author

@ferdnyc ferdnyc May 19, 2023

Choose a reason for hiding this comment

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

I slightly went back on my previous change, here, and switched back to displaying the QDOC_TEMPLATE and QDOC_INDEX (renamed from QDOC_QTCORE_INDEX for symmetry) variables rather than the directory variables. That's so that, if the find_file() command is used and fails to find a suitable file to infer directory paths from, QDOC_TEMPLATE-NOTFOUND or QDOC_INDEX-NOTFOUND will be displayed as the value, rather than an empty string.

The overly-complicated bit of code following "# Expand imported target data for status reporting", above, ensures that the variables being reported have sensible values, in the cases where targets or command line arguments are used instead of fallback find_program() and find_file() calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of this only ever runs in cases where some part of the help-tool-discovery logic fails, so hopefully most of the time it won't even be triggered.

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2023

CLA assistant check
All committers have signed the CLA.

Instead of using `find_program()` to discover tools like `qdoc`,
`qhelpgenerator`, and `qtattributionsscanner`, default to picking up
those tools' locations from the relevant CMake IMPORTED executable
targets.

Fall back to `find_program()` for any targets that don't exist, to
preserve compatibility with Qt 5.12 and earlier.
@dantti dantti merged commit c7af0fb into KDAB:master Aug 25, 2023
@ferdnyc ferdnyc deleted the help-modernization branch October 2, 2023 20:52
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