Fix #3156: Modify Dictionary implementation to match docs

Documentation for Dictionary(): "Returns a dictionary object containing
copies of all of the construction variables in the environment. If
there are any variable names specified, only the specified construction
variables are returned in the dictionary." However, in the existing
implementation, if called with no arguments it directly returns the
internal dictionary of the construction environment, the one it uses
to implement its dictionary-like behavior (matches docs); if a single
key argument is passed it returns the value for that key; if multiple
keys are passed it returns a list of the matching values. In neither
of the latter two case does it return a dict object.  This change in
Environment.py aligns the behavior with the docs.

Since all the tests are coded to the actual behavior, rather than
the documented behavior, none demonstrated any problem, which is
only seen if a user themselves uses Dictionary() as documented.
All the tests which depend on the old behavior thus needed some
adjustment. The normal pattern follows this example:

  env['FOO'] = 'foo'
  assert env.Dictionary('FOO') == 'foo'

which breaks when Dictionary returns a dict instead of value. That
can change to either of two following forms - the former is more
like the origial intent, but the second pattern has been chosen
because it looks less awkward, except for a few cases that seemed
to be testing specifically the limiting of the returned Dictionary
by giving it a specific number of keys:

  assert env.Dictionary('FOO')['FOO'] == "foo"
  assert env.Dictionary()['FOO'] == "foo"

A couple of internal tests define dummy classes which implement their
own version of Dictionary.  These needed updating as well.  If the
dummy took no args, it was left alone (BuilderTests.py,  FSTests.py,
NodeTests.py). If the dummy ignored args it still does so, but got a
comment line to that effect.  If it takes one key argument it builds a
one-item dict (SubstTests.py, msvsTests.py), if it takes *args like the
main Dictionary it got the change of the original (ProgTests.py), and if
it took args but did something special (FortranTests.py, IDLTests.py)
it was adjusted to be sure it was still returning a dict object. I'm
not clear if the cases where a dummy Dictionary does not have the same
signature as the real Dictionary are correct, but there are enough
changes here already, we could look at that topic separately.

Fixes #3156
Signed-off-by: Mats Wichmann <mats@linux.com>

src/CHANGES.txt

@@ -13,6 +13,15 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Use importlib to dynamically load tool and platform modules instead of imp module
     - sconsign: default to .sconsign.dblite if no filename is specified.
       Be more informative in case of unsupported pickle protocol (py2 only).
+    - convert Dictionary() to behave as documented - always return a dict.
+      Although the "new" behavior is as documented, this does represent
+      a change to behavior: the old way, if Dictionary() is called with
+      one arg it returned the value for that key; if called with multiple
+      args it returned a list of values for those keys, now it returns
+      a dict which is a subset of the full environment.  To update
+      older usage, change references like env.Dictionary('FOO') to be
+      either env.Dictionary()['FOO'] or env.Dictionary('FOO')['FOO'].
+      No change is needed for calls to Dictionary() with no arguments.
 
   From John Doe:
 

src/engine/SCons/Environment.py

@@ -1496,9 +1496,10 @@ def Detect(self, progs):
     def Dictionary(self, *args):
         if not args:
             return self._dict
-        dlist = [self._dict[x] for x in args]
-        if len(dlist) == 1:
-            dlist = dlist[0]
+        try:
+            dlist = {key: self._dict[key] for key in self._dict.viewkeys() & args}
+        except AttributeError:  # python3
+            dlist = {key: self._dict[key] for key in self._dict.keys() & args}
         return dlist
 
     def Dump(self, key = None):
@@ -2344,7 +2345,7 @@ def __contains__(self, key):
             return 1
         return self.__dict__['__subject'].__contains__(key)
     def Dictionary(self):
-        """Emulates the items() method of dictionaries."""
+        """obtain internal dictionary"""
         d = self.__dict__['__subject'].Dictionary().copy()
         d.update(self.__dict__['overrides'])
         return d

src/engine/SCons/EnvironmentTests.py

@@ -1124,7 +1124,7 @@ def test_ENV(self):
         assert 'ENV' in env.Dictionary()
 
         env = self.TestEnvironment(ENV = { 'PATH' : '/foo:/bar' })
-        assert env.Dictionary('ENV')['PATH'] == '/foo:/bar'
+        assert env.Dictionary()['ENV']['PATH'] == '/foo:/bar'
 
     def test_ReservedVariables(self):
         """Test warning generation when reserved variable names are set"""
@@ -1735,9 +1735,9 @@ def test_Clone(self):
 
         env3 = env1.Clone(XXX = 'x3', ZZZ = 'z3')
         assert env3 == env3
-        assert env3.Dictionary('XXX') == 'x3'
-        assert env3.Dictionary('YYY') == 'y'
-        assert env3.Dictionary('ZZZ') == 'z3'
+        assert env3.Dictionary()['XXX'] == 'x3'
+        assert env3.Dictionary()['YYY'] == 'y'
+        assert env3.Dictionary()['ZZZ'] == 'z3'
         assert env1 == env1copy
 
         # Ensure that lists and dictionaries are
@@ -1747,13 +1747,13 @@ class TestA(object):
         env1 = self.TestEnvironment(XXX=TestA(), YYY = [ 1, 2, 3 ],
                            ZZZ = { 1:2, 3:4 })
         env2=env1.Clone()
-        env2.Dictionary('YYY').append(4)
-        env2.Dictionary('ZZZ')[5] = 6
-        assert env1.Dictionary('XXX') is env2.Dictionary('XXX')
-        assert 4 in env2.Dictionary('YYY')
-        assert not 4 in env1.Dictionary('YYY')
-        assert 5 in env2.Dictionary('ZZZ')
-        assert 5 not in env1.Dictionary('ZZZ')
+        env2.Dictionary()['YYY'].append(4)
+        env2.Dictionary()['ZZZ'][5] = 6
+        assert env1.Dictionary()['XXX'] == env2.Dictionary()['XXX']
+        assert 4 in env2.Dictionary()['YYY']
+        assert not 4 in env1.Dictionary()['YYY']
+        assert 5 in env2.Dictionary()['ZZZ']
+        assert 5 not in env1.Dictionary()['ZZZ']
 
         #
         env1 = self.TestEnvironment(BUILDERS = {'b1' : Builder()})
@@ -1942,20 +1942,20 @@ def test_Dictionary(self):
         defaults that get inserted.
         """
         env = self.TestEnvironment(XXX = 'x', YYY = 'y', ZZZ = 'z')
-        assert env.Dictionary('XXX') == 'x'
-        assert env.Dictionary('YYY') == 'y'
-        assert env.Dictionary('XXX', 'ZZZ') == ['x', 'z']
-        xxx, zzz = env.Dictionary('XXX', 'ZZZ')
-        assert xxx == 'x'
-        assert zzz == 'z'
+        assert env.Dictionary()['XXX'] == 'x'
+        assert env.Dictionary()['YYY'] == 'y'
+        assert not {'x', 'z'}.difference(env.Dictionary('XXX', 'ZZZ').values())
+        xxx = env.Dictionary('XXX', 'ZZZ')
+        assert xxx['XXX'] == 'x'
+        assert xxx['ZZZ'] == 'z'
         assert 'BUILDERS' in env.Dictionary()
         assert 'CC' in env.Dictionary()
         assert 'CCFLAGS' in env.Dictionary()
         assert 'ENV' in env.Dictionary()
 
         assert env['XXX'] == 'x'
         env['XXX'] = 'foo'
-        assert env.Dictionary('XXX') == 'foo'
+        assert env.Dictionary()['XXX'] == 'foo'
         del env['XXX']
         assert 'XXX' not in env.Dictionary()
 
@@ -2929,8 +2929,12 @@ def test_NoClean(self):
     def test_Dump(self):
         """Test the Dump() method"""
 
+        # this is to format the expected string
+        import pprint
+        pp = pprint.PrettyPrinter(indent=2)
+        expect = pp.pformat({'FOO': 'foo'})
         env = self.TestEnvironment(FOO = 'foo')
-        assert env.Dump('FOO') == "'foo'", env.Dump('FOO')
+        assert env.Dump('FOO') == expect, env.Dump('FOO')
         assert len(env.Dump()) > 200, env.Dump()    # no args version
 
     def test_Environment(self):

src/engine/SCons/Scanner/CTests.py

@@ -180,6 +180,7 @@ def __init__(self, **kw):
         self.fs = SCons.Node.FS.FS(test.workpath(''))
 
     def Dictionary(self, *args):
+        # note dummy Dictionary ignores args
         return self.data
 
     def subst(self, strSubst, target=None, source=None, conv=None):

src/engine/SCons/Scanner/DTests.py

@@ -41,6 +41,7 @@ def __init__(self, **kw):
         self.fs = SCons.Node.FS.FS(test.workpath(''))
 
     def Dictionary(self, *args):
+        # note dummy Dictionary ignores args
         return self.data
 
     def subst(self, strSubst, target=None, source=None, conv=None):

src/engine/SCons/Scanner/FortranTests.py

@@ -217,7 +217,7 @@ def Dictionary(self, *args):
         if not args:
             return { 'FORTRANPATH': self.path, 'FORTRANMODSUFFIX' : ".mod" }
         elif len(args) == 1 and args[0] == 'FORTRANPATH':
-            return self.path
+            return { 'FORTRANPATH': self.path }
         else:
             raise KeyError("Dummy environment only has FORTRANPATH attribute.")
 

src/engine/SCons/Scanner/IDLTests.py

@@ -198,7 +198,7 @@ def Dictionary(self, *args):
         if not args:
             return { 'CPPPATH': self.path }
         elif len(args) == 1 and args[0] == 'CPPPATH':
-            return self.path
+            return { 'CPPPATH': self.path }
         else:
             raise KeyError("Dummy environment only has CPPPATH attribute.")
 

src/engine/SCons/Scanner/LaTeXTests.py

@@ -84,6 +84,7 @@ def __init__(self, **kw):
         self.fs = SCons.Node.FS.FS(test.workpath(''))
 
     def Dictionary(self, *args):
+        # note dummy Dictionary ignores args
         return self.data
 
     def subst(self, strSubst, target=None, source=None, conv=None):

src/engine/SCons/Scanner/ProgTests.py

@@ -54,10 +54,11 @@ def __init__(self, **kw):
     def Dictionary(self, *args):
         if not args:
             return self._dict
-        elif len(args) == 1:
-            return self._dict[args[0]]
-        else:
-            return [self._dict[x] for x in args]
+        try:
+            dlist = {key: self._dict[key] for key in self._dict.viewkeys() & args}
+        except AttributeError:  # python3
+            dlist = {key: self._dict[key] for key in self._dict.keys() & args}
+        return dlist
 
     def has_key(self, key):
         return key in self.Dictionary()

src/engine/SCons/Scanner/RCTests.py

@@ -80,6 +80,7 @@ def __init__(self,**kw):
         self.fs = SCons.Node.FS.FS(test.workpath(''))
 
     def Dictionary(self, *args):
+        # note dummy Dictionary ignores args
         return self.data
 
     def subst(self, arg, target=None, source=None, conv=None):

src/engine/SCons/SubstTests.py

@@ -56,7 +56,7 @@ def __init__(self, dict={}):
     def Dictionary(self, key = None):
         if not key:
             return self.dict
-        return self.dict[key]
+        return {key: self.dict[key]}
 
     def __getitem__(self, key):
         return self.dict[key]
@@ -93,7 +93,7 @@ def __call__(self, target, source, env, for_signature):
         assert str(target) == 't', target
         assert str(source) == 's', source
         assert for_signature == self.expect_for_signature, for_signature
-        return [ self.mystr, env.Dictionary('BAR') ]
+        return [ self.mystr, env.Dictionary()['BAR'] ]
 
 if os.sep == '/':
     def cvt(str):

src/engine/SCons/Tool/msvsTests.py

@@ -371,7 +371,7 @@ def __init__(self, dict=None):
     def Dictionary(self, key = None):
         if not key:
             return self.dict
-        return self.dict[key]
+        return {key: self.dict[key]}
 
     def __setitem__(self,key,value):
         self.dict[key] = value

test/AR/AR.py

@@ -37,7 +37,7 @@
 
 test.write('SConstruct', """
 foo = Environment(LIBS = ['foo'], LIBPATH = ['.'])
-ar = foo.Dictionary('AR')
+ar = foo.Dictionary()['AR']
 bar = Environment(LIBS = ['bar'], LIBPATH = ['.'], AR = r'%(_python_)s wrapper.py ' + ar)
 foo.Library(target = 'foo', source = 'foo.c')
 bar.Library(target = 'bar', source = 'bar.c')

test/CC/CC.py

@@ -113,7 +113,7 @@
 """)
 
 test.write('SConstruct', """
-cc = Environment().Dictionary('CC')
+cc = Environment().Dictionary()['CC']
 env = Environment(LINK = r'%(_python_)s mylink.py',
                   LINKFLAGS = [],
                   CC = r'%(_python_)s mycc.py',
@@ -129,7 +129,7 @@
 if os.path.normcase('.c') == os.path.normcase('.C'):
 
     test.write('SConstruct', """
-cc = Environment().Dictionary('CC')
+cc = Environment().Dictionary()['CC']
 env = Environment(LINK = r'%(_python_)s mylink.py',
                   CC = r'%(_python_)s mycc.py',
                   CXX = cc)
@@ -143,7 +143,7 @@
 
 test.write('SConstruct', """
 foo = Environment()
-cc = foo.Dictionary('CC')
+cc = foo.Dictionary()['CC']
 bar = Environment(CC = r'%(_python_)s wrapper.py ' + cc)
 foo.Program(target = 'foo', source = 'foo.c')
 bar.Program(target = 'bar', source = 'bar.c')

test/CC/CCVERSION.py

@@ -40,7 +40,7 @@
 test.file_fixture(os.path.join('CC-fixture', 'foo.c'))
 
 test.write('SConstruct', """
-cc = Environment().Dictionary('CC')
+cc = Environment().Dictionary()['CC']
 foo = Environment(CC = r'%(_python_)s versioned.py "${CCVERSION}" ' + cc)
 foo.Program(target = 'foo', source = 'foo.c')
 """ % locals())

test/CC/SHCC.py

@@ -36,7 +36,7 @@
 
 test.write('SConstruct', """
 foo = Environment()
-shcc = foo.Dictionary('SHCC')
+shcc = foo.Dictionary()['SHCC']
 bar = Environment(SHCC = r'%(_python_)s wrapper.py ' + shcc)
 foo.SharedObject(target = 'foo/foo', source = 'foo.c')
 bar.SharedObject(target = 'bar/bar', source = 'bar.c')

test/CXX/CXX.py

@@ -187,7 +187,7 @@
 
 test.write('SConstruct', """
 foo = Environment()
-cxx = foo.Dictionary('CXX')
+cxx = foo.Dictionary()['CXX']
 bar = Environment(CXX = r'%(_python_)s wrapper.py ' + cxx)
 foo.Program(target = 'foo', source = 'foo.cxx')
 bar.Program(target = 'bar', source = 'bar.cxx')

test/CXX/CXXVERSION.py

@@ -54,7 +54,7 @@
 """)
 
 test.write('SConstruct', """
-cxx = Environment().Dictionary('CXX')
+cxx = Environment().Dictionary()['CXX']
 foo = Environment(CXX = r'%(_python_)s versioned.py "${CXXVERSION}" ' + cxx)
 foo.Program(target = 'foo', source = 'foo.cpp')
 """ % locals())

test/CXX/SHCXX.py

@@ -36,7 +36,7 @@
 
 test.write('SConstruct', """
 foo = Environment()
-shcxx = foo.Dictionary('SHCXX')
+shcxx = foo.Dictionary()['SHCXX']
 bar = Environment(SHCXX = r'%(_python_)s wrapper.py ' + shcxx)
 foo.SharedObject(target = 'foo/foo', source = 'foo.cpp')
 bar.SharedObject(target = 'bar/bar', source = 'bar.cpp')

test/DVIPDF/DVIPDF.py

@@ -117,7 +117,7 @@
     test.write('SConstruct', """
 import os
 foo = Environment(ENV = { 'PATH' : os.environ['PATH'] })
-dvipdf = foo.Dictionary('DVIPDF')
+dvipdf = foo.Dictionary()['DVIPDF']
 bar = Environment(ENV = { 'PATH' : os.environ['PATH'] },
                   DVIPDF = r'%(_python_)s wrapper.py ' + dvipdf)
 foo.PDF(target = 'foo.pdf',

test/DVIPDF/DVIPDFFLAGS.py

@@ -122,7 +122,7 @@
 import os
 ENV = {'PATH' : os.environ['PATH']}
 foo = Environment(DVIPDFFLAGS = '-N', ENV = ENV)
-dvipdf = foo.Dictionary('DVIPDF')
+dvipdf = foo.Dictionary()['DVIPDF']
 bar = Environment(DVIPDF = r'%(_python_)s wrapper.py ' + dvipdf, ENV = ENV)
 foo.PDF(target = 'foo.pdf',
         source = foo.DVI(target = 'foo.dvi', source = 'foo.tex'))

test/DVIPDF/makeindex.py

@@ -48,7 +48,7 @@
 test.write('SConstruct', """
 import os
 env = Environment(ENV = { 'PATH' : os.environ['PATH'] })
-dvipdf = env.Dictionary('DVIPDF')
+dvipdf = env.Dictionary()['DVIPDF']
 env.PDF(target = 'foo.pdf',
         source = env.DVI(target = 'foo.dvi', source = 'foo.tex'))
 """)

test/DVIPS/DVIPS.py

@@ -134,7 +134,7 @@
 import os
 ENV = { 'PATH' : os.environ['PATH'] }
 foo = Environment(ENV = ENV)
-dvips = foo.Dictionary('DVIPS')
+dvips = foo.Dictionary()['DVIPS']
 bar = Environment(ENV = ENV, DVIPS = r'%(_python_)s wrapper.py ' + dvips)
 foo.PostScript(target = 'foo.ps', source = 'foo.tex')
 bar.PostScript(target = 'bar1', source = 'bar1.tex')

test/DVIPS/DVIPSFLAGS.py

@@ -137,7 +137,7 @@
 import os
 ENV = {'PATH' : os.environ['PATH']}
 foo = Environment(ENV = ENV, DVIPSFLAGS = '-N')
-dvips = foo.Dictionary('DVIPS')
+dvips = foo.Dictionary()['DVIPS']
 bar = Environment(ENV = ENV,DVIPS = r'%(_python_)s wrapper.py ' + dvips)
 foo.PostScript(target = 'foo.ps', source = 'foo.tex')
 bar.PostScript(target = 'bar1', source = 'bar1.tex')

test/Fortran/F03.py

@@ -85,7 +85,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F03 = '%(fc)s')
-f03 = foo.Dictionary('F03')
+f03 = foo.Dictionary()['F03']
 bar = foo.Clone(F03 = r'%(_python_)s wrapper.py ' + f03)
 foo.Program(target = 'foo', source = 'foo.f03')
 bar.Program(target = 'bar', source = 'bar.f03')

test/Fortran/F03FLAGS.py

@@ -89,7 +89,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F03 = '%(fc)s')
-f03 = foo.Dictionary('F03')
+f03 = foo.Dictionary()['F03']
 bar = foo.Clone(F03 = r'%(_python_)s wrapper.py ' + f03, F03FLAGS = '-Ix')
 foo.Program(target = 'foo', source = 'foo.f03')
 bar.Program(target = 'bar', source = 'bar.f03')

test/Fortran/F08.py

@@ -86,7 +86,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F08 = '%(fc)s')
-f08 = foo.Dictionary('F08')
+f08 = foo.Dictionary()['F08']
 bar = foo.Clone(F08 = r'%(_python_)s wrapper.py ' + f08)
 foo.Program(target = 'foo', source = 'foo.f08')
 bar.Program(target = 'bar', source = 'bar.f08')

test/Fortran/F08FLAGS.py

@@ -89,7 +89,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F08 = '%(fc)s')
-f08 = foo.Dictionary('F08')
+f08 = foo.Dictionary()['F08']
 bar = foo.Clone(F08 = r'%(_python_)s wrapper.py ' + f08, F08FLAGS = '-Ix')
 foo.Program(target = 'foo', source = 'foo.f08')
 bar.Program(target = 'bar', source = 'bar.f08')

test/Fortran/F77.py

@@ -85,7 +85,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F77 = '%(fc)s', tools = ['default', 'f77'], F77FILESUFFIXES = ['.f'])
-f77 = foo.Dictionary('F77')
+f77 = foo.Dictionary()['F77']
 bar = foo.Clone(F77 = r'%(_python_)s wrapper.py ' + f77)
 foo.Program(target = 'foo', source = 'foo.f')
 bar.Program(target = 'bar', source = 'bar.f')

test/Fortran/F77FLAGS.py

@@ -65,7 +65,7 @@
 
     test.write('SConstruct', """
 foo = Environment(F77 = '%(fc)s', tools = ['default', 'f77'], F77FILESUFFIXES = [".f"])
-f77 = foo.Dictionary('F77')
+f77 = foo.Dictionary()['F77']
 bar = foo.Clone(F77 = r'%(_python_)s wrapper.py ' + f77, F77FLAGS = '-I%(directory)s')
 foo.Program(target = 'foo', source = 'foo.f')
 bar.Program(target = 'bar', source = 'bar.f')