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

SCons: Warn when passing unknown variables #91213

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Apr 26, 2024

Lifted largely from godot-cpp's implementation1, albeit with formatting tweaked to be more in-line with recent SConstruct adjustments.

Example output:

Executing task: scons dev_build=yes compiledb=yes optimize=debug verbose=no tests=yes warnings=extra d3d12=yes module_text_server_fb_enabled=yes cpp_standard=23 ninja=no 

scons: Reading SConscript files ...
Automatically detected platform: windows
WARNING: Unknown SCons variables were passed and will be ignored:
        cpp_standard = 23
Auto-detected 32 CPU cores available for build parallelism. Using 31 cores by default. You can override it with the -j argument.

Footnotes

  1. https://github.com/godotengine/godot-cpp/blob/e23b117ac32fdbeb0920f234e193e6d31307c0ad/SConstruct#L41-L46

@Repiteo Repiteo force-pushed the scons/warn-unknown-variables branch from 40d6019 to 646757a Compare April 26, 2024 18:04
@Repiteo
Copy link
Contributor Author

Repiteo commented Apr 26, 2024

Double-checked to make sure the regressions from #56551 are no longer applicable. The mono-stuff is no longer relevant as that logic appears to have been removed entirely, or otherwise relocated to not use the argument system. The only regression I could replicate was the profile option, and that came down to it simply… Not being an option. By adding that as a proper variable, everything works as expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants