Closed mwichmann closed 2 weeks ago
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
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.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 likeenv['CPPDEFINES']
by fetching the value fromself._dict
.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._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.self._dict
, not knowing it'sself
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:
OverrideEnvironment
to pretend it has a_dict
, which behaves as expected for an OE._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/orOverrideEnvironment
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.