Problems with BoolVariable import in SCons 4.8.0

[mwichmann]

Tested versions

SCons version 4.8.0 is relevant, Godot version does not matter.

System information

any

Issue description

This is a complementary issue filing from the SCons project, so there's some findable information for folks that run into this problem - it's not a Godot problem but seems to affect some developers using Godot.

The build engine used by Godot, SCons, has a Variables mechanism for describing more sophisticated command line argument usage of the key=value form, along the lines of the various Python argument parsing routines like argparse, optparse (which SCons happens to use for --foo type arguments), getopt and many more. The Godot project uses this mechanism a fair bit - you'll find some or all of BoolVariable, EnumVariable, ListVariable, PackageVariable and PathVariable used in various places.

For files which SCons itself brings in when processing (via an SConscript() call), conventionally named SConscript, or, specific to Godot, SCsub - though any name is allowed, SCons can fix up the module global scope such that all the SCons symbols are in scope. But files brought in via the Python import statement provide no hook to do that, so those files must bring in the SCons context explicitly. The SCons project normally recommends doing this via from SCons.Script import *, which is specially crafted to bring the entire SCons API into scope, without extras.

In the 4.8.0 revision of SCons, a cleanup of Variables added use of the special __all__ attribute, which constrains the symbols made available by "star imports" (in this case, from SCons.Variables import *), so as not to pollute the scope of the module doing the import with extra symbols. Unfortunately, we did not anticipate that external projects were importing only the Variables subset of the API using a star import.

From a quick examination, Godot itself always imports in a way that doesn't run afoul of this change. Some examples (a subset):

platform/android/detect.py:    from SCons.Variables import BoolVariable
platform/ios/detect.py:    from SCons.Variables import BoolVariable
platform/linuxbsd/detect.py:    from SCons.Variables import BoolVariable, EnumVariable
platform/macos/detect.py:    from SCons.Variables import BoolVariable, EnumVariable
platform/web/detect.py:    from SCons.Variables import BoolVariable
platform/windows/detect.py:    from SCons.Variables import BoolVariable, EnumVariable

Importing the variable by name does not run into this "star import" situation.

Presumably there are examples in the wild that show the star import form, since we've already heard of a few cases on SCons forums and on StackOverflow.

The five variable types listed above are proposed for addition to the __all__ attribute in an SCons PR (SCons/scons#4576), and if accepted will appear in a future release.

The workaround is to either import the variable types by name, as in the snip from the platform code above, or use from SCons.Script import *.

Steps to reproduce

Inclusion of a Python file via import.

Minimal reproduction project (MRP)

not applicable

[akien-mga]

Hi Mats, thanks for reaching out downstream!

We noticed this indeed, and has this prior report: #94410.

The godotengine/godot repo wasn't affected but we had to fix it up in godotengine/godot-cpp's stable branches (which is where the StackOverflow issue came from) and godotengine/godot-nir-static.

I'm fine switching to from SCons.Script import * myself as recommended upstream, but I believe some of the recent changes away from star imports were done to appease Python linters. @Repiteo should know more.