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

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

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

PapyChacal
Copy link
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
Copy link
Contributor

@Faless Faless left a comment

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
Contributor Author

Sure, done!

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

Faless commented Dec 19, 2022

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