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

Unify tools/target build type configuration to disambiguate "debug" (used for both game debugging and engine dev tools) #3371

Closed
akien-mga opened this issue Sep 30, 2021 · 13 comments
Assignees
Milestone

Comments

@akien-mga
Copy link
Member

akien-mga commented Sep 30, 2021

Describe the project you are working on

The Godot buildsystem.

Describe the problem or limitation you are having in your project

The current Godot buildsystem uses command line options named tools and target to configure a number of things. Not all combinations of tools and target are valid, and the exact meaning of the various targets (release, release_debug, and debug) is confusing to many contributors, but also especially to users who make their own custom builds.

This confusion is also furthered into other Godot-related thirdparty projects such as GDNative plugins which try to replicate the Godot buildsystem.

I'll take the Linux platform as an example for what's currently confusing:

https://github.com/godotengine/godot/blob/b8c92828146ba22e2bb205da73aecc0619581829/platform/linuxbsd/detect.py#L92-L116

The target flag governs some semantically distinct options:

  • Optimization flags (-O3 or -O2 for release and release_debug respectively)
  • Generation of debug symbols (-g3 for debug, and -g2 for release and release_debug only if debug_symbols=yes)
  • Enabling debug features in the engine itself (DEBUG_ENABLED define). This is quite confusing as it's not at all related to either optimization or debug symbols (== information in C++ debugger), this DEBUG_ENABLED define controls a lot of code in the engine which is omitted when undefined. This enables debug templates and the editor (which are both built with target=release_debug) to catch issues that would not be caught in a target=release template build.

Moreover, DEBUG_ENABLED is mandatory to build the editor (tools=yes). So tools=yes target=release is not possible, and it's prevented here:
https://github.com/godotengine/godot/blob/b8c92828146ba22e2bb205da73aecc0619581829/SConstruct#L576-L579

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Given the above problem, we have a semantic mixup in the meaning we give to "debug":

  1. Debug can refer to the features which are in the engine code to help users debug their own code (editor debugger, helpful error messages, additional validation checks which are bypassed on release/production builds).
  2. Debug can refer to the ability for engine developers to debug C++ code of the engine, and find why it crashes, set breakpoints, and compile faster without optimization.

I suggest to work this around by renaming the second usage to "dev" (for developer mode), and change the target/tools partial matrix to a single preset system (probably still using the then unique target option).

This should help better establish the relationship between the new targets and the actual binaries that we ship in official distributions (editor release binary (tools=yes target=release_debug), release template (tools=no target=release) and debug template (tools=no target=release_debug)).

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I suggest a preset system like this:

target:
    editor:
        tools: yes
        optimization: yes
        debug-features: yes
    editor-dev:
        tools: yes
        optimization: no
        debug-features: yes
    template-release:
        tools: no
        optimization: yes
        debug-features: no
    template-debug:
        tools: no
        optimization: yes
        debug-features: yes
    template-dev:
        tools: no
        optimization: no
        debug-features: yes

The default target would be target=editor, i.e. users building Godot themselves would get the optimized version of the editor by default (same as the one in official builds, minus time consuming optimizations like use_lto=yes).

Engine contributors would have to build with target=editor-dev if they want to help with engine development by fixing bugs/crashes, etc. This disables optimizations, making the editor slower, but also makes it faster to build and thus iterate when writing C++ code for the engine.

Export templates would be built with target=template-release and target=template-debug for the "Release" and "Debug" templates respectively, so what you see in your export presets matches what you use for custom builds.

The target=template-dev option would be for engine contributors who need to debug issues in export templates themselves by disabling optimizations and keeping debug symbols. This can also be used by users who need to debug e.g. a crash on Android and have a detailed stacktrace. This "dev" template can be used in place of the "debug" template in the export preset.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It will be used often. The current system works fine, but this should improve user-friendliness significantly.

Is there a reason why this should be core and not an add-on in the asset library?

This is the engine buildsystem, can't be third-party.

@akien-mga
Copy link
Member Author

akien-mga commented Sep 30, 2021

For the record, this is related to #1416 and would likely need to be done at the same time, as refactoring the target system also impacts the weird binary suffixes that we currently use: https://github.com/godotengine/godot/blob/ac7505e27730a800c9167ab940216c3980257b5b/SConstruct#L576-L597

Edit: Though realistically I'd probably start with this proposal before tackling #1416, which still needs to be further defined.

@YuriSizov
Copy link
Contributor

YuriSizov commented Sep 30, 2021

I suggest a preset system like this:

It's not clear to me, but do we keep the individual options or do we only expose presets and those options are listed here just for explanation purpose?

I think it would be useful to have individual flags available, but only active if you do not specify any preset. I.e. you either use target or you use some combination of tools, optimization, debug-features, and we don't allow mix and matching.

@akien-mga
Copy link
Member Author

It's not clear to me, but do we keep the individual options or do we only expose presets and those options are listed here just for explanation purpose?

For the record, I think it would be useful to have individual flags available, but only active if you do not specify any preset. I.e. you either use target or you use some combination of tools, optimization, debug-features, and we don't allow mix and matching.

In essence for this proposal these may just be internal booleans that will be registered in the Environment and can be queried where relevant in the buildsystem.

We could also keep them as command line options that users can provide, but then we need to make sure that the aforementioned Environment config is distinct from those (i.e. you should have a single bool to query to know if you're building with debug-features, without having to check both target and debug-features to know it based on user input).

In other words, whatever we do for the command line interface, the first step for the buildsystem would be to parse the options, error if they're mutually exclusive indeed, and then if they're good, register them separately in booleans or a dict or whatever that would be the reference on the build config throughout the buildsystem.

@akien-mga akien-mga added this to the 4.0 milestone Sep 30, 2021
@akien-mga akien-mga self-assigned this Sep 30, 2021
@lawnjelly
Copy link
Member

This also paves the way for something like a DEV_ENABLED define, which is sorely needed imo and isn't possible at the moment afaik because of DEBUG_ENABLED being set in the editor in debug and release_debug.

@aaronfranke
Copy link
Member

Why does "template-release" have a "-release" suffix, but "editor" does not?

Would optimization being enabled include the current production=yes?

@akien-mga
Copy link
Member Author

akien-mga commented Sep 30, 2021

Why does "template-release" have a "-release" suffix, but "editor" does not?

Because "editor-release" calls for "editor-debug", and this is the whole problem that this proposal aims to address. And "editor-release" would actually build with the same settings as "template-debug", which again is the whole problem here. There's only two types of builds for the editor: the main, production one, and dev builds for contributors.

Would optimization being enabled include the current production=yes?

Partly. production=yes currently sets use_static_cpp=yes (which is now default too), use_lto=yes (still very time consuming) and debug_symbols=no.

I would probably make optimized builds disable debug symbols, but also strip the binaries as there's more unnecessary symbols for production use than just what -g2 adds.

use_static_cpp is more about portability than optimization. Since it's already enabled by default I'd leave it out of the proposed target matrix.

use_lto is tricky. That can be considered but it's a topic in itself, not necessarily something that should be decided as part of this proposal.

@aaronfranke

This comment was marked as outdated.

@akien-mga
Copy link
Member Author

akien-mga commented Oct 4, 2021

@aaronfranke Your example reintroduces the exact ambiguity that this proposal aims to solve. And you lack a preset for what we currently build as Debug templates, which are optimized templates with debug features.

The whole point of this proposal is to make things clear and match what users and contributors are familiar with:

  • Editor (editor)
  • Template Release (template-release)
  • Template Debug (template-debug)

The other two presets are not intended for users and are only relevant for contributors who want to work on the engine itself:

  • Editor Dev build (editor-dev)
  • Template Dev build (template-dev)

Edit: One option however could be to keep only editor, template-release, and template-debug, and make the "dev" part (i.e. no optimizations, debug symbols) a separate flag (e.g. dev=yes). That means engine contributors would have to build by default with target=editor dev=yes.

@lawnjelly
Copy link
Member

lawnjelly commented Oct 4, 2021

I'm not sure why we necessarily need a distinction between debug and dev, since I usually want both or neither

The distinction is because they are two completely different things, that are currently being confused by having the same name.

  1. c++ debug build (engine developers)
    There currently is no c++ debug build. c++ debug features are currently added by the ide-debug flag.
  2. Godot IDE debug features (users)
    This is what the debug represents in the current builds

These two things need different names, as you may need different combinations of these. Users will typically want user debug features but NOT engine developer debugging. Thus the editor release build is built with user debugging features.

The distinction is important because we also need the ability to add extra code that will only run in c++ debug builds (DEV), to add extra verification, tests etc that we don't want in the shipping builds.

This can be confusing because mostly in c++ you are producing an app, you have an obvious debug build, and a release build (final product). If for example, your final product is a debugger, you then have a naming problem. You then have a debug-debugger, and a release-debugger. The same thing is happening in Godot, because we are essentially building a debugger.

akien-mga added a commit to akien-mga/godot that referenced this issue Oct 4, 2021
This will allow adding developer checks which will be fully compiled out in
user builds, unlike `DEBUG_ENABLED` which is included in debug tempates and
the editor builds.

This define is not used yet, but we'll soon add code that uses it, and change
some existing `DEBUG_ENABLED` checks to be performed only in dev builds.

Related to godotengine/godot-proposals#3371.
@akien-mga
Copy link
Member Author

This also paves the way for something like a DEV_ENABLED define, which is sorely needed imo and isn't possible at the moment afaik because of DEBUG_ENABLED being set in the editor in debug and release_debug.

I've added this to the 3.x branch in a way that doesn't change the buildsystem flags, and doesn't impact user builds: godotengine/godot#53383

It does not solve the ambiguity described in this proposal, which is why it's only for 3.x (kept compatible). For master we'll refactor everything.

akien-mga added a commit to akien-mga/godot that referenced this issue Oct 14, 2021
This will allow adding developer checks which will be fully compiled out in
user builds, unlike `DEBUG_ENABLED` which is included in debug tempates and
the editor builds.

This define is not used yet, but we'll soon add code that uses it, and change
some existing `DEBUG_ENABLED` checks to be performed only in dev builds.

Related to godotengine/godot-proposals#3371.
sairam4123 pushed a commit to sairam4123/godot that referenced this issue Nov 10, 2021
This will allow adding developer checks which will be fully compiled out in
user builds, unlike `DEBUG_ENABLED` which is included in debug tempates and
the editor builds.

This define is not used yet, but we'll soon add code that uses it, and change
some existing `DEBUG_ENABLED` checks to be performed only in dev builds.

Related to godotengine/godot-proposals#3371.
lekoder pushed a commit to KoderaSoftwareUnlimited/godot that referenced this issue Dec 18, 2021
This will allow adding developer checks which will be fully compiled out in
user builds, unlike `DEBUG_ENABLED` which is included in debug tempates and
the editor builds.

This define is not used yet, but we'll soon add code that uses it, and change
some existing `DEBUG_ENABLED` checks to be performed only in dev builds.

Related to godotengine/godot-proposals#3371.
@akien-mga
Copy link
Member Author

For optimizations, we'll likely want to provide more flexibility than just the optimization=yes/no that I originally mentioned.

Something along the lines of what godotengine/godot-cpp#835 implemented for godot-cpp would likely be necessary to support the full range of options we already use in the codebase (and which users may want to use as override).

akien-mga added a commit to akien-mga/godot that referenced this issue Sep 26, 2022
Implements godotengine/godot-proposals#3371.

New `target` presets
====================

The `tools` option is removed and `target` changes to use three new presets,
which match the builds users are familiar with. These targets control the
default optimization level and enable editor-specific and debugging code:

- `editor`: Replaces `tools=yes target=release_debug`.
  * Defines: `TOOLS_ENABLED`, `DEBUG_ENABLED`, `-O2`/`/O2`
- `template_debug`: Replaces `tools=no target=release_debug`.
  * Defines: `DEBUG_ENABLED`, `-O2`/`/O2`
- `template_release`: Replaces `tools=no target=release`.
  * Defines: `-O3`/`/O2`

New `dev_build` option
======================

The previous `target=debug` is now replaced by a separate `dev_build=yes`
option, which can be used in combination with either of the three targets,
and changes the following:

- `dev_build`: Defines `DEV_ENABLED`, disables optimization (`-O0`/`/0d`),
  enables generating debug symbols, does not define `NDEBUG` so `assert()`
  works in thirdparty libraries, adds a `.dev` suffix to the binary name.

Note: Unlike previously, `dev_build` defaults to off so that users who
compile Godot from source get an optimized and small build by default.
Engine contributors should now set `dev_build=yes` in their build scripts or
IDE configuration manually.

Changed binary names
====================

The name of generated binaries and object files are changed too, to follow
this format:

`godot.<platform>.<target>[.dev][.double].<arch>[.<extra_suffix>][.<ext>]`

For example:
- `godot.linuxbsd.editor.dev.arm64`
- `godot.windows.template_release.double.x86_64.mono.exe`

Be sure to update your links/scripts/IDE config accordingly.

More flexible `optimize` and `debug_symbols` options
====================================================

The optimization level and whether to generate debug symbols can be further
specified with the `optimize` and `debug_symbols` options. So the default
values listed above for the various `target` and `dev_build` combinations
are indicative and can be replaced when compiling, e.g.:

`scons p=linuxbsd target=template_debug dev_build=yes optimize=debug`
will make a "debug" export template with dev-only code enabled, `-Og`
optimization level for GCC/Clang, and debug symbols. Perfect for debugging
complex crashes at runtime in an exported project.
@dzil123
Copy link

dzil123 commented Nov 21, 2022

With the PR merged, should this issue be closed?

@aaronfranke
Copy link
Member

Yes, this proposal has been fully implemented in godotengine/godot#66242.

Meptl added a commit to Meptl/goost that referenced this issue May 9, 2023
See godot commit 3d2dd79ecd2c8456ba9401f6b12333d01f61e13e
godotengine/godot-proposals#3371
godotengine/godot#60723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Implemented
Development

No branches or pull requests

5 participants