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: Sync targets.py fully with upstream Godot #1204

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Aug 10, 2023

  • Reorders existing code to match Godot.
  • Adds NDEBUG for non-dev builds.
  • Adds -gdwarf-4 for Clang debug symbols.
  • Adds strip link flag for GCC/Clang builds without debug symbols.

Follow-up to #1203.

The additions are up to discussion, if some seem unwanted in the context of GDExtension, I can remove them and document why we differ from upstream.

This syncs with:

https://github.com/godotengine/godot/blob/013e8e3afb982d4b230f0039b6dc248b48794ab9/SConstruct#L402-L437

and:

https://github.com/godotengine/godot/blob/013e8e3afb982d4b230f0039b6dc248b48794ab9/SConstruct#L574-L619

@akien-mga akien-mga added enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup cherrypick:4.1 labels Aug 10, 2023
@akien-mga akien-mga added this to the 4.2 milestone Aug 10, 2023
@akien-mga akien-mga requested review from dsnopek and Faless August 10, 2023 07:22
@akien-mga akien-mga requested a review from a team as a code owner August 10, 2023 07:22
- Reorders existing code to match Godot.
- Adds `NDEBUG` for non-dev builds.
- Adds `-gdwarf-4` for Clang debug symbols.
- Adds strip link flag for GCC/Clang builds without debug symbols.
@akien-mga akien-mga force-pushed the scons-targets-sync-with-godot branch from dac4c7b to 600e749 Compare August 10, 2023 07:23
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!

I can't speak so much for the functionality of what the changes do here, but this does look like a faithful copy-paste of the Godot code with only minor modifications:

  • env_base -> env
  • The addition of the DEBUG_METHODS_ENABLED define
  • using_clang(env) -> methods.using_clang(env) and the same for is_vanilla_clang()

And since it successfully built for the all the platforms on CI, I think we can say it probably didn't break anything.

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

The addition of the DEBUG_METHODS_ENABLED define

It was defined and used (in the binder_common.hpp) before, so nothing changes.

@dsnopek
Copy link
Collaborator

dsnopek commented Aug 16, 2023

Thanks!

@dsnopek dsnopek merged commit 112e41b into godotengine:master Aug 16, 2023
@akien-mga akien-mga deleted the scons-targets-sync-with-godot branch August 16, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants