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

CMake: GodotCPPModule.cmake #1707

Merged
merged 1 commit into from
Feb 17, 2025
Merged

Conversation

enetheru
Copy link
Collaborator

After @Naros pointed out that for orchestrator the doc_source.cpp file was not being generated it made me think that a more predictable and obvious method is desirable.

Changes:

  • Renamed python_callouts.cmake to GodotCPPModule.cmake
  • Moved the find_package(Python3 3.4 REQUIRED) to top of GodotCPPModule.cmake to guarantee its availability.
  • Added target_doc_sources CMake function

It's typical of CMake projects to APPEND to the CMAKE_MODULES_PATH to include scripts from dependencies.
Renaming the python_callouts.cmake to GodotCPPModule.cmake creates a very clear location to place any CMake functionality which is public facing.

In consumer CMakeLists.txt it would be recommended that after including the godot-cpp project to:

list(APPEND CMAKE_MODULE_PATH "${godot-cpp_SOURCE_DIR}/cmake")
include( GodotCPPModule )

To simpify the addition of documentation to a gdextension library a new function target_doc_sources has been created
which adds a dependency to generate the files on build. It also adds a new target doc_gen which will perform the doc_source.cpp generation on demand. (I can add this behind a flag, or leave it exposed by default or customise it to include the name of the target.)

This reduces the complexity of including documentation sources to a project by relieving the user of having to explicitly put the OUTPUT_PATH of the generate_doc_source function in the sources list of their target.

add_library( my_extension SHARED )

target_sources( my_extension
        PRIVATE
        src/my_source.cpp
        src/my_source.h
        src/...
)

file( GLOB_RECURSE XML_FILES_LIST LIST_DIRECTORIES NO CONFIGURE_DEPENDS
    "${CMAKE_CURRENT_SOURCE_DIR}/doc_classes/*.xml" )

target_doc_sources( my_extension ${XML_FILES_LIST} )

@enetheru enetheru force-pushed the cmake_module branch 2 times, most recently from b488c72 to e7654df Compare February 13, 2025 07:18
@enetheru enetheru marked this pull request as ready for review February 13, 2025 07:59
@enetheru enetheru requested a review from a team as a code owner February 13, 2025 07:59
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks!

At a high-level this makes sense to me, and is definitely the sort of thing we want to get in before Godot 4.4 (since it affects the "developer interface" to the cmake config).

I just have one question about the changes...

@enetheru
Copy link
Collaborator Author

Thanks!

At a high-level this makes sense to me, and is definitely the sort of thing we want to get in before Godot 4.4 (since it affects the "developer interface" to the cmake config).

I just have one question about the changes...

I'll revert that sorry. I accidentally resolved the comment.

Move the find_package for python into it.
Recommend adding godot-cpp/cmake to CMAKE_MODULE_PATH and using include( GodotCPPModule ) to use functions.
Add target_doc_sources function to simplify the addition of documentation to a binary.
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks!

@dsnopek dsnopek merged commit 3f54a86 into godotengine:master Feb 17, 2025
11 checks passed
@enetheru enetheru deleted the cmake_module branch February 17, 2025 20:47
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