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

Privatise and Tidy #1665

Closed
wants to merge 2 commits into from
Closed

Privatise and Tidy #1665

wants to merge 2 commits into from

Conversation

enetheru
Copy link
Contributor

@enetheru enetheru commented Dec 7, 2024

One of the benefits of CMake which is not currently being realised is that target options can be selectively propagated to consumers. For expediency, I had previously made all the flags PUBLIC, which propagates all options to consumers. However, after some investigation, It appears that we can make the majority of them PRIVATE.
The compile_definitions must remain public as they are needed in consumer projects.
The compile_options and link_options can be made private and achieve a successful compile, but investigation is required for what should be propagated.

There are also flags and properties that can be tidied up to be more correct.

Remove RPATH_USE_ORIGIN from static libs
add INTERFACE_POSITION_INDEPENDENT_CODE ON
reset GODOT_ARCH property to SYSTEM_ARCH for macos because universal add MACOSX_RPATH to test target
remove redundant properties from test target
remove redundant and PUBLIC cxx_std_17 compile feature.
Move MSVC flag utf-8 to godot-cpp-test where it is required.

@enetheru enetheru force-pushed the tidy branch 2 times, most recently from fe20f36 to af8f04c Compare December 9, 2024 21:14
Simplify <platform>_generate cmake function signature, as TARGET_ALIAS and TARGET_NAME are still in scope and do not need to be passed.
Add USE_FOLDERS, and FOLDER properties to group targets in VS and XCode
Guard test target with GODOT_ENABLE_TESTING
Generate all three variants in the form godot-cpp.test.<target> to remove the -DTEST_TARGET option clutter.
Update the docs/cmake.rst with information about the testing target
Update the Github CI
@enetheru enetheru force-pushed the tidy branch 2 times, most recently from 4d5b9ef to 13f6a93 Compare December 11, 2024 00:23
There are tiny issues that don't effect the build that can be cleaned.
And flags that can be made private to not effect consumers.

Remove RPATH_USE_ORIGIN from static libs
add INTERFACE_POSITION_INDEPENDENT_CODE ON
reset GODOT_ARCH property to SYSTEM_ARCH for macos because universal
add MACOSX_RPATH to test target
remove redundant properties from test target
remove redundant and PUBLIC cxx_std_17 compile feature
Change compiler options to PRIVATE, tested on godot-cpp-test, and orchestrator
Transform BITS if statement into simple math expression
Transform binding parameters if statements into a genex
@enetheru enetheru closed this Dec 11, 2024
@Calinou Calinou added archived enhancement This is an enhancement on the current functionality labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archived enhancement This is an enhancement on the current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants