godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.74k stars 575 forks source link

Add SCons variant_dir support #1439

Open Cedev opened 7 months ago

Cedev commented 7 months ago

Adds support for using SCons's variant_dir feature to specify the build path for where to build a variant of the the library.

Correct paths in SConstruct and godotcpp tool for the following, even when the current directory is not the root of the repository:

dsnopek commented 6 months ago

Thanks!

Can you show how to use the functionality added in this PR?

Cedev commented 6 months ago

Can you show how to use the functionality added in this PR?

If you pass variant_dir to SConscript it will build files in that directory instead of in the source tree. You can use this to separate out builds for different variants like platform and architecture/target/debug/precision

For example, the following SConstruct file for something conscripting godot-cpp will build it twice, once for env["precision"]="single" and once for env["precision"]="double".

env = Environment()
env["precision"] = "single"
env = SConscript(
  "godot-cpp/SConstruct",
  exports=['env'],
  variant_dir="#build/libgodot-cpp-single",
  duplicate=False)

env = Environment()
env["precision"] = "double"
env = SConscript(
  "godot-cpp/SConstruct",
  exports=['env'],
  variant_dir="#build/libgodot-cpp-double",
  duplicate=False)

Because of the variant_dir options the builds will be done in two separate directories, "#build/libgodot-cpp-single" and "#build/libgodot-cpp-double"

The generated binding header files depend on both the env["precision"] and env["arch"], changing either would result in generating different include files and rebuilding everything that depends on them with each build, but with the results of the single and double precision variant builds in different directories, subsequent builds can reuse the portion of each variant that didn't change (which for a project depending on godot-cpp is usually everything).

Here's the output from the second run of that build on my machine.

Cedric@ELIDED-COMPUTER-NAME MINGW64 /e/elided-project-path (master)
$ scons
scons: Reading SConscript files ...
Auto-detected 4 CPU cores available for build parallelism. Using 4 cores by default. You can override it with the -j argument.
Building for architecture x86_64 on platform windows
Auto-detected 4 CPU cores available for build parallelism. Using 4 cores by default. You can override it with the -j argument.
Building for architecture x86_64 on platform windows

scons: warning: Two different environments were specified for target E:\elided-project-path\compile_commands.json,
        but they appear to have the same action: write_compilation_db(target, source, env)
File "E:\elided-project-path\godot-cpp\tools\godotcpp.py", line 444, in generate
scons: done reading SConscript files.
scons: Building targets ...
scons: `build\libgodot-cpp-single\bin\libgodot-cpp.windows.template_debug.x86_64.lib' is up to date.
scons: `build\libgodot-cpp-double\bin\libgodot-cpp.windows.template_debug.double.x86_64.lib' is up to date.
scons: done building targets.
dsnopek commented 6 months ago

Ah, ok, thanks! That sounds really cool :-)

@faless Do you have any concerns with these changes?

Faless commented 6 months ago

Ah, ok, thanks! That sounds really cool :-)

@Faless Do you have any concerns with these changes?

Indeed this seems pretty nice, I've been investigating this on the godot side a couple of times, it's nice to have it at least in godot-cpp.

The only note I have is about the warning with the provided example:

scons: warning: Two different environments were specified for target E:\elided-project-path\compile_commands.json, but they appear to have the same action: write_compilation_db(target, source, env)

I wonder if the default compile_command.json should be generated in the variant dir when not specified. Not a blocker in any case, since the problem is pretty specific to the synthetic example.

Not sure why CI tests are failing...

Cedev commented 6 months ago

I wonder if the default compile_command.json should be generated in the variant dir when not specified. Not a blocker in any case, since the problem is pretty specific to the synthetic example.

I couldn't decide whether to touch that, well eventually I decided not to. Usually you only invoke SConscript once in an scons script, env["compiledb_file"] already provides a way to control that, and by default env["compiledb"] = False and the file isn't generated.

To keep from potentially breaking things I only changed where godotcpp and SConstruct are looking in their own source directories for their own files, and not when the location of a file is provided by a user. (I think that's the scope of changes, whether or not that's true would be relevant review).

Cedev commented 6 months ago

...the CI tests are failing with the same error on this other recent PR too: https://github.com/godotengine/godot-cpp/actions/runs/8852986336/job/24312834460?pr=1450

For the tiniest of bifurcation, the checks were successful against this PR when the base was b7661a60a4788cc7d5ce690cf8a5815b0aab511, but it looks like there's way more recent PRs than that that they were passing against.

dsnopek commented 6 months ago

...the CI tests are failing with the same error on this other recent PR too: https://github.com/godotengine/godot-cpp/actions/runs/8852986336/job/24312834460?pr=1450

Try rebasing on master. The changes from PR https://github.com/godotengine/godot-cpp/pull/1461 may have fixed the CI issues (but we'll only know after enough CI builds run successfully)

dsnopek commented 4 months ago

Thanks! Unfortunately, it looks like this needs another rebase to address conflicts in SConstruct