Skip to content

Make build_library=no usable at godot-cpp's root#903

Merged
Faless merged 1 commit into
godotengine:masterfrom
PapyChacal:build_library
Dec 19, 2022
Merged

Make build_library=no usable at godot-cpp's root#903
Faless merged 1 commit into
godotengine:masterfrom
PapyChacal:build_library

Conversation

@PapyChacal

Copy link
Copy Markdown
Contributor

Hello,

Currently, build_library=no does not work at the repository's root. (It generates a dependency cycle and scons refuses to generate anything).

To reproduce:
scons generate_bindings=yes build_library=no in godot-cpp
Or rm gen -r before scons build_library=no in godot-cpp
Should give something like:

scons: *** Found dependency cycle(s):
  gen -> gen/include/godot_cpp/classes/text_server_manager.hpp -> gen/include/godot_cpp/classes -> gen/include/godot_cpp -> gen/include -> gen

This PR solves this, by not stating that gen/ is a target itself.
Also invert arguments to env.Clean, to call it as is documented in https://scons.org/doc/4.4.0/HTML/scons-user/apd.html.
I added a CI step to check this does not regress. I clean the generated sources before the actual build, to make sure the build step includes generation so the dependency handling should not regress either.

@akien-mga
akien-mga requested a review from Faless November 11, 2022 12:14
@akien-mga akien-mga added bug This has been identified as a bug topic:buildsystem Related to the buildsystem or CI setup labels Nov 11, 2022

@Faless Faless left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

…_library=no.

Use env variable to pass the output dir instead.
Add a CI step to guard regression on this. Clean the generated sources to avoid interfering with next steps.
@PapyChacal

Copy link
Copy Markdown
Contributor Author

Sure, done!

@Faless
Faless merged commit c0eaada into godotengine:master Dec 19, 2022
@Faless

Faless commented Dec 19, 2022

Copy link
Copy Markdown
Contributor

Thanks!

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

Labels

bug This has been identified as a bug topic:buildsystem Related to the buildsystem or CI setup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants