Clone() does not honor 'variables' argument

[pauschar]

Describe the bug
As per discussion with bdbaddog on Discord around 2020-03-12, the following SConstruct does not work as expected:

vars = Variables()
vars.Add(BoolVariable('MYTEST', 'bla',default=False))

env = Environment(variables=vars)
print(env.get('MYTEST'))
env.Replace(MYTEST=True)
print(env.get('MYTEST'))

# passing variables to Clone() does not seem to have an effect
# and there is no indication of failure to update.
env1 = env.Clone(variables=vars)

# after Clone(), the expected value of MYTEST is 'False' because 'vars' has
# been passed into the clone. But we have to enforce that env1 is using the
# values from vars, not from env.
# commenting out the Update call proves this ...
vars.Update(env1)
print(env1.get('MYTEST'))``

Required information

• Link to SCons Users thread discussing your issue: None, discussed on Discord
• Version of SCons: 3.1.2
• Version of Python: 3.6.8
• Which python distribution if applicable (python.org, cygwin, anaconda, macports, brew,etc): WSL Ubuntu 18.04
• How you installed SCons: pip
• What Platform are you on? (Linux/Windows and which version): WSL Ubuntu 18.04
• How to reproduce your issue? Run the above SConstruct, the expected 3rd line printout is 'True' with vars.Update(env1) disabled - env1 should hold 'MYTEST' as False because that's the default from line 2 of the script.
• How you invoke scons (The command line you're using "scons --flags some_arguments")
  scons

[bdbaddog]

Thanks for filing a complete Issue!
Can't make any promises when we'll get to this, but we have sufficient info to properly understand the issue now.

[mwichmann]

Quick look - there's no code for it. Environment recognizes variables separate from the generic kwargs, and folds them in; Clone does not.

[pauschar]

Re-reading my initial report and Mats' quick look makes me think that this could be handled in 2 steps:

• make Clone() report some error/warning that it was unable to parse/evaluate provided named arguments. This behaviour was most disturbing to me when trying to pass variables into a clone - it took me quite a while to figure that variables is ignored silently.
• make Clone() behave much as Environment() - to me this would be SCons'ic.

[bdbaddog]

@pauschar - Unfortunately the first option you list is really not viable. As you can specify any variable to be added or overwritten(from the env being cloned) to the cloned env.

[mwichmann]

Four years later... been gardening around Variables anyway, and tried a quick fix, which works fine:

$ git diff SCons/Environment.py
diff --git a/SCons/Environment.py b/SCons/Environment.py
index 5bf763d91..1ff09fa9e 100644
--- a/SCons/Environment.py
+++ b/SCons/Environment.py
@@ -1568,7 +1568,7 @@ class Base(SubstitutionEnvironment):
                     self._dict[key] = dk + val
         self.scanner_map_delete(kw)
 
-    def Clone(self, tools=[], toolpath=None, parse_flags = None, **kw):
+    def Clone(self, tools=[], toolpath=None, variables=None, parse_flags = None, **kw):
         """Return a copy of a construction Environment.
 
         The copy is like a Python "deep copy"--that is, independent
@@ -1577,7 +1577,6 @@ class Base(SubstitutionEnvironment):
         (like a function).  There are no references to any mutable
         objects in the original Environment.
         """
-
         builders = self._dict.get('BUILDERS', {})
 
         clone = copy.copy(self)
@@ -1603,6 +1602,8 @@ class Base(SubstitutionEnvironment):
         for key, value in kw.items():
             new[key] = SCons.Subst.scons_subst_once(value, self, key)
         clone.Replace(**new)
+        if variables:
+            variables.Update(clone)
 
         apply_tools(clone, tools, toolpath)
 

I added a couple of printouts to the original example - namely to print the value of env['variables'] (not set), and, after the Clone, env1['variables']. The latter looked like <SCons.Variables.Variables object at 0x7f785fd31d90> before the patch, and becomes unset after, as we've consumed it and not passed it on as described above. And added a print before the vars.Update just to show it really got picked up.

@bdbaddog is this worth pushing?

[mwichmann]

The docs fail to mention the extra args to Clone anyway, so we could argue that using variables here is undocumented and thus unsupported behavior... which would be a different resolution to the problem. If patching is preferred, a bit of a doc update seems to be in order.

Clone() manpage signature: `env.Clone([key=val, ...])``

SCons.Environment.Base.Clone function signature: def Clone(self, tools=[], toolpath=None, parse_flags = None, **kw):

You can argue that the key=val,... stanza covers the "called out" arguments in the actual interface, but since they're handled one way and unknown kwargs another, it seems better to mention them. IMO it doesn't need to be more than a copy of the line from the Environment() entry:

The keyword arguments parse_flags, platform, toolpath, tools and variables are also specially recognized. See the manpage section "Construction Environments" for more details.

leaving out platform (not handled now, and with the assumption a clone should have the same platform setup as its sibling), and, if the change for this issue is rejected, also leaving out variables.