OverrideEnvironment problems with env._dict

[mwichmann]

The OverrideEnvironment class is a proxy, but it's passed around like a normal env by the rest of SCons - it's supposed to behave like one, but there's one particular sequence where the right thing isn't happening.

• "Real" environments (derived from SubstitutionEnvironment) "behave like a dict" with respect to construction variables. They have an attribute _dict, which is indeed a dict, and the inherited __getitem__ method resolves things like env['CPPDEFINES'] by fetching the value from self._dict.
• References to environment methods which do not have corresponding methods in OverrideEnvironment will be passed to OE's __getattr__ method for resolution, and are then fetched from the subject (the overridden environment) - this is the way it's supposed to work.
• Lots of environment methods (things from the public API) that manipulate environment variables access the _dict attribute directly to do so. This is probably a little dodgy, since the __getitem__ and __setitem__ methods are able to handle that, but it's not broken - for "real" environments.
• When one of these methods is called as if it were a method of an OE, the (proxied) method accesses self._dict, not knowing it's self is actually an OE instance. An OE has no _dict, so it is sent to __getattr__ for resolution, and served up from the subject. This bypasses the normal behavior of an OE of consulting the overrides dict first, followed by the subject, so on fetches the override isn't seen, and on updates, the variable in the subject environment is what is modified, not the one in the override dict.

There are really two approaches to cleaning this up:

1. Teach OverrideEnvironment to pretend it has a _dict, which behaves as expected for an OE.
2. Stop all these environment methods from accessing _dict directly, and leave the work to __getitem__ and __setitem__ (and __delitem__, for that matter) - those methods are provided by an OE so not fetched from the subject and thus they won't end up touching the _dict it doesn't have.

The first is probably a bit easier since it requires a change in only one place - the OE itself; the second would require touching a lot of places. Neither is completely trivial - the things that ccurrently access believe they're talking to a Python native dict, or complete emulation thereof, we might find out Base and/or OverrideEnvironment don't emulate that well enough (though have tried to make those complete).

There isn't a lot of known impact from this. Running the test suite with SCons modified to fail on an OE accessing it's own nonexistent _dict broke a ton of tests, but the vast majority were from the method _gsm; changing that one to not access _dict directly lets the rpm tool and the LaTeX scanner doing some things which modify an OE; the LaTeX scanner does an odd "restore original" dance which suggests it may have run into a problem with the underlying env changing, but I can't prove that without prospecting in the history.

[mwichmann]

It turns out that this small change leaves the test suite working, including new tests crafted to show the leakage problem, which previously did not:

@@ -2554,6 +2553,13 @@ class OverrideEnvironment(Base):
 
     # Methods that make this class act like a proxy.
     def __getattr__(self, name):
+        # Proxied environment methods don't know they could be called with
+        # us as 'self' and may access the _data consvar store directly. We
+        # need to pretend to have one, not serve up the one from the subject,
+        # or it will miss the overridden values (and possibly modify the base).
+        # Use ourselves and hope the dict-like methods below are sufficient.
+        if name == '_dict':
+            return self
         attr = getattr(self.__dict__['__subject'], name)
         # Here we check if attr is one of the Wrapper classes. For
         # example when a pseudo-builder is being called from an