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

Updates and fixes to build_configs #11397

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Lzard
Copy link
Contributor

@Lzard Lzard commented Nov 2, 2024

Adds missing macro definitions to SDL_build_config.h.cmake, removes unused ones, changes unneeded @VAR@ replacements to 1s, and fixes a few little things.

Those are issues I came across while working with SDL_build_config.h.cmake for a personal project that uses CMake configuration headers, but not CMake itself.

Lzard added 10 commits November 2, 2024 19:57
Those are only used in build_config files that define them themselves, or not used at all.
Those are defined in other build_configs files and used elsewhere in SDL.
Currently, `SDL_DEFAULT_ASSERT_LEVEL` is commented out by CMake when its value is 0, setting the assertions level to the default value instead of disabling them.
This change:
- defines `SDL_DEFAULT_ASSERT_LEVEL_CONFIGURED` when its value is non-zero.
- defines `SDL_DEFAULT_ASSERT_LEVEL`, regardless of its value, when `SDL_DEFAULT_ASSERT_LEVEL_CONFIGURED` is defined.
Makes all macros only used in `#ifdef`s defined as `1` when they exist, instead of the CMake value of the corresponding variable.
I messed up some spacing, so I thought I might as well strip all those unnecessary spaces.
...and that `SDL_VENDOR_INFO` is included in `SDL_REVISION`.
@Lzard Lzard force-pushed the build-config-updates branch from 402151a to 44ae21d Compare November 2, 2024 21:13
@slouken slouken requested a review from madebr November 3, 2024 02:23
@slouken slouken added this to the 3.2.0 milestone Nov 3, 2024
@slouken
Copy link
Collaborator

slouken commented Dec 4, 2024

@madebr, can you take a look at this? It mostly looks good to me, I just want a second pair of eyes on this.

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.

2 participants