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

Godot build system breaks with SCons 4.8.0 release #94410

Closed
TCROC opened this issue Jul 15, 2024 · 7 comments
Closed

Godot build system breaks with SCons 4.8.0 release #94410

TCROC opened this issue Jul 15, 2024 · 7 comments

Comments

@TCROC
Copy link
Contributor

TCROC commented Jul 15, 2024

Tested versions

Godot v4.3.beta.mono (ba7c5b0)

System information

Godot v4.3.beta.mono (ba7c5b0) - Windows 10.0.19045 - d3d12 (Forward+) - dedicated AMD Radeon RX 6600 (Advanced Micro Devices, Inc.; 31.0.12027.9001) - AMD Ryzen 7 5800X 8-Core Processor (16 Threads)

Issue description

Godot can no longer be built with the SCons 4.8.0 update. It appears that wildcard imports no longer work. So things like

from SCons.Variables import *

Won't import BoolVariable and such.

This has been recently discovered on this StackOverflow question: https://stackoverflow.com/questions/78746956/error-name-boolvariable-is-not-defined-when-using-scons-on-godot-with-c

And this blog: https://devcodef1.com/news/1330890/godot-c-error-undefined-boolvariable-in-scons

Note that the only thing in the blog that discusses this error is the title. Then it rambles on about booleans, gdextension, and c++ bindings. I'm guessing it was probably written by AI or something. Impressive that it somehow caught the issue tho! Even tho the proposed answer is wildly incorrect.

The solution is 1 of 2 things:

  1. Require users to be on SCons 2.7.0 or earlier. (I don't reccomend this approach. In my experience, locking in on old versions of software eventually creates future and more challenging headaches).
  2. Change all wildcard imports to explicit imports.

Here are some areas that are affected:

https://github.com/godotengine/godot-nir-static/blob/27a49f8601f6af6da93d5950d80764827d2b3442/godot-tools/windows.py#L6

https://github.com/godotengine/godot-nir-static/blob/27a49f8601f6af6da93d5950d80764827d2b3442/godot-tools/targets.py#L4

Also, os now needs to be explicitly imported.

EDIT

Okay literally as I was about to submit this, I realized this may be isolated to the godot-nir-static repo: https://github.com/godotengine/godot-nir-static and the mesa submodule that godot-nir-static uses.

I'm still going to submit here so godot devs are aware. Maybe we should add a note to contribution guidlines? Or maybe just confirm that all pipelines still pass with 4.8.0 version of SCons? Feel free to close it once you have done what you need. I'll go copy this issue over to godot-nir-static as well since that is where I for sure know it to be.

Steps to reproduce

Download scons 4.8.0
Try to build godot-nir-static: https://github.com/godotengine/godot-nir-static

Minimal reproduction project (MRP)

N/A

@Calinou
Copy link
Member

Calinou commented Jul 16, 2024

Do you mean SCons 4.8.0? Godot requires SCons 3.1.2 or later.

Change all wildcard imports to explicit imports.

While this is more annoying to do, explicit imports also benefit static analyzers in my experience.

@TCROC
Copy link
Contributor Author

TCROC commented Jul 16, 2024

Do you mean SCons 4.8.0? Godot requires SCons 3.1.2 or later.

Change all wildcard imports to explicit imports.

While this is more annoying to do, explicit imports also benefit static analyzers in my experience.

Yes I did mean 4.8.0. I'll correct that in the morning.

@kus04e4ek
Copy link
Contributor

Can't reproduce in godotengine/godot, cause it seems like there are no wildcard imports left

@TCROC TCROC changed the title Godot build system breaks with SCons 2.8.0 release Godot build system breaks with SCons 4.8.0 release Jul 16, 2024
@DylanCheetah
Copy link

It builds fine for me with SCons 4.8.0 as well.

@TCROC
Copy link
Contributor Author

TCROC commented Jul 16, 2024

Good. Hopefully this is just specific to godot-nir-static. Which there appears to be a pr in to fix.

@akien-mga
Copy link
Member

Moving to godotengine/godot-nir-static#17.

@akien-mga
Copy link
Member

This also affected godot-cpp in the 4.2, 4.1, and 4.0 branches, but not master. godotengine/godot-cpp#1526 fixes it.

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

No branches or pull requests

5 participants