Circular dependencies between core libraries

[simpkins]

Tested versions

• Current master branch

System information

• Windows 11 with mingw

Issue description

Godot's SConstruct file currently builds a separate static library for each subdirectory: core/, scene/, main/, platform/, a number of separate libraries under modules/, etc.

This is problematic, since libraries must be listed on the command line in dependency order: each library must be listed before other libraries they depend on. However, there are many circular dependencies between these libraries:

• Pretty much everything uses code from core/
• core/ depends on code from scene/, servers/, main/, and modules/
• servers/ depends on code from scene/ and modules/
• scene/ depends on code from modules/
• main/ depends on pretty much everything, including all modules (for initialize_modules()) and platform (for register_platform_apis())

The circular dependencies means that there is no correct way to list these libraries in a correct order on the command line. The code happens to work today, but is very fragile to exposing undefined symbol errors if anything is tweaked. (e.g.: this build failure from #108415.)

There are a few options for how this could be fixed:

• The most correct would be to combine everything into a single library. Since everything depends on everything else, it technically should be a single library. Unfortunately, attempting to do so currently results in build failures with Argument list too long errors trying to pass all of the source file names in a single command line. This could be worked around by making multiple separate calls to ar to incrementally append more files to the archive, but that might be awkward with SCons. Another approach might be to let SCons build the current libraries as-is, and then relink them afterwards into one combined library.
• For Linux we could hack around the problem by adding -Wl,--start-group and -Wl,--end-group around this group of libraries, to inform it that this group of libraries contains circular dependencies. However, it looks like the Apple linker may not support this functionality.

Steps to reproduce

PR #108415 makes a minor tweak to the link line (moving libtest before libmain) which triggers link failures due to these dependency problems. I'm sort of surprised that the build works as-is with these circular dependencies.

Minimal reproduction project (MRP)

Build #108415 with mingw on Windows.

[simpkins]

I was able to work around the command line to long errors on Linux and Android by updating $ARCOM to use $(TEMPFILE), so I submitted a PR to merge these libraries into one: #108426

In the future it might be possible to clean up the dependencies a little bit to split these into smaller libraries again, but we would probably also need to improve the build system to ensure that people cannot add incorrect dependencies again. One way to do this would be to set CPPPATH so that code in core/ cannot see headers in main/ or scene/, etc. Doing this today would probably require copying the header files to a separate directory so that we can have a directory with only the headers that should be visible to the current library, and adding that to CPPPATH rather than adding the repository root directory to CPPPATH.

[Repiteo]

There's absolutely a problem with the #include order in the repo as it currently exists, and it's very much dumb luck that so much of this functions at all. I'm STRONGLY in favor of a major structural overhaul if it meant ensuring these kinds of dependency issues would be sorted for good, though that very well may be 5.0 territory

[ze2j]

For argument list too long issue, one solution is to use a file as argument of ar, see here.

[Ivorforce]

We happened to be talking about this in a meeting just yesterday. We agreed it's a good goal to 'harden' dependencies, such that e.g. core cannot depend on main. We're open to suggestions for how best to achieve this.