EnsureSConsVersion does too much

[abitrolly]

Why does EnsureSConsVersion try to initialize DefaultEnvironment with all the tools (including default gcc)? It doesn't need all these slow-to-initialize things to run a version check and exit.

  File "/home/anatoli/g/godot/src/SConstruct", line 3:
    EnsureSConsVersion(3, 0, 0)
  File "/home/anatoli/g/godot/src/.scons/SCons/Script/SConscript.py", line 658:
    env = self.factory()
  File "/home/anatoli/g/godot/src/.scons/SCons/Script/SConscript.py", line 638:
    default_env = SCons.Defaults.DefaultEnvironment()
  File "/home/anatoli/g/godot/src/.scons/SCons/Defaults.py", line 83:
    _default_env = SCons.Environment.Environment(*args, **kw)
  File "/home/anatoli/g/godot/src/.scons/SCons/Environment.py", line 1010:
    apply_tools(self, tools, toolpath)
  File "/home/anatoli/g/godot/src/.scons/SCons/Environment.py", line 116:
    _ = env.Tool(tool)
  File "/home/anatoli/g/godot/src/.scons/SCons/Environment.py", line 1879:
    tool(self)
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/__init__.py", line 274:
    self.generate(env, *args, **kw)
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/default.py", line 40:
    for t in SCons.Tool.tool_list(env['PLATFORM'], env):
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/__init__.py", line 776:
    c_compiler = FindTool(c_compilers, env) or c_compilers[0]
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/__init__.py", line 678:
    t = Tool(tool)
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/__init__.py", line 120:
    module = self._tool_module()
  File "/home/anatoli/g/godot/src/.scons/SCons/Tool/__init__.py", line 190:

[abitrolly]

EnsureSconsVersion is injected into script global namespace here.

scons/SCons/Script/__init__.py

Lines 294 to 298 in e3e91c1

	GlobalDefaultEnvironmentFunctions = [
	# Methods from the SConsEnvironment class, above.
	'Default',
	'EnsurePythonVersion',
	'EnsureSConsVersion',

scons/SCons/Script/__init__.py

Lines 379 to 381 in e3e91c1

	for name in GlobalDefaultEnvironmentFunctions + GlobalDefaultBuilders:
	exec ("%s = _SConscript.DefaultEnvironmentCall(%s)" % (name, repr(name)))
	del name

[abitrolly]

EnsureSConsVersion is a static function that doesn't really refers anything. Why does it need a complete initialized environment (DefaultEnvironment)?

scons/SCons/Script/SConscript.py

Lines 491 to 506 in e3e91c1

	def EnsureSConsVersion(self, major, minor, revision=0):
	"""Exit abnormally if the SCons version is not late enough."""
	# split string to avoid replacement during build process
	if SCons.__version__ == '__' + 'VERSION__':
	SCons.Warnings.warn(SCons.Warnings.DevelopmentVersionWarning,
	"EnsureSConsVersion is ignored for development version")
	return
	scons_ver = self._get_major_minor_revision(SCons.__version__)
	if scons_ver < (major, minor, revision):
	if revision:
	scons_ver_string = '%d.%d.%d' % (major, minor, revision)
	else:
	scons_ver_string = '%d.%d' % (major, minor)
	print("SCons %s or greater required, but you have SCons %s" % \
	(scons_ver_string, SCons.__version__))
	sys.exit(2)

[abitrolly]

What happens if DefaultEnvironment(tools=[]) is called after EnsureSConsVersion? Is it reset somehow?

[mwichmann]

What happens if DefaultEnvironment(tools=[]) is called after EnsureSConsVersion? Is it reset somehow?

The default environment is basically a singleton, you always get the same thing. Explicitly calling it just forces the initialization to happen on your terms, rather than when SCons thinks it needs to. Personally not sure why it's instantiated at all if you don't make use of it, but that's a topic for a separate discussion.

[abitrolly]

That means that tools that are already initialized won't be removed, right?

[mwichmann]

That means that tools that are already initialized won't be removed, right?

If the default environment has already been initialized, you just get that already-initialized environment returned and the call does no work - any tools argument is just ignored - if that's what the question is.

[abitrolly]

That answers the question, thanks.

[mwichmann]

Let's wait till @bdbaddog is back and see if he has an opinion on whether such a change would be acceptable before you go to the trouble.

[abitrolly]

I couldn't resist. #4224

[mwichmann]

Just to record it somewhere - this seems as good a place as any: Exit, GetLaunchDir and SConscriptChdir also don't seem to have any reason to be presented as environment methods, and could be fine just as global functions. Would it be worth un-documenting these five as being environment methods? That is, in the xml, change them so they say <arguments signature="global"> - not actually change their existence in env.

Help is kind of dodgy as well as are Export and Import, since they all by their nature do Global Things and not environment-specific things, but those three do use something from the environment - in Help it's a self.subst call and in the other two it's a self.Split, which differs from SCons.Util.Split by, of course, doing a subst.

[abitrolly]

I'd like to place them into a separate file, but then I need to write be code that injects them back into env, which will not make things more clear. But if the end goal is deprecating them as env functions and leave them only in global in the next major release, then that would worth the case.

Of course the proper change should estimate the impact by looking which scripts in the wild (on GitHub probably) use these functions as Environment functions. But going that route without some company back up is not sustainable for me.

[bdbaddog]

If any arguements to these could use env vars, then there should be env. versions of them.

[mwichmann]

the ones mentioned don't do any subst'ing, which is why I separated them out in my question from the ones that do. But that does confirm the latter set should be left alone!

[mwichmann]

Cross-reference update: the five functions were changed in the docs to show only as Global functions (#4230)

[abitrolly]

Does it make sense to deprecate them and move out of SConsEnvironment class in the next major release?

[abitrolly]

Made all these functions static in #4234.