Commit 8376678b authored by Godefroid Chapelle's avatar Godefroid Chapelle

Problem: _update should not mutate in place

because it is the cause for subtle bugs

Solution: return copies of object rather than mutating
parent fe6b33fc
...@@ -368,11 +368,15 @@ class Buildout(DictMixin): ...@@ -368,11 +368,15 @@ class Buildout(DictMixin):
# load configuration files # load configuration files
if config_file: if config_file:
data_buildout_copy = copy.deepcopy(data['buildout']) data_buildout_copy = copy.deepcopy(data['buildout'])
_update(data, _open(os.path.dirname(config_file), config_file, [], cfg_data = _open(
data_buildout_copy, override, set())) os.path.dirname(config_file),
config_file, [], data_buildout_copy,
override, set()
)
data = _update(data, cfg_data)
# apply command-line options # apply command-line options
_update(data, cloptions) data = _update(data, cloptions)
# Set up versions section, if necessary # Set up versions section, if necessary
if 'versions' not in data['buildout']: if 'versions' not in data['buildout']:
...@@ -1483,7 +1487,7 @@ class Options(DictMixin): ...@@ -1483,7 +1487,7 @@ class Options(DictMixin):
result = _annotate_section(result, "") result = _annotate_section(result, "")
data = _annotate_section(copy.deepcopy(data), "") data = _annotate_section(copy.deepcopy(data), "")
_update_section(result, data) result = _update_section(result, data)
result = _unannotate_section(result) result = _unannotate_section(result)
result.pop('<', None) result.pop('<', None)
return result return result
...@@ -1775,7 +1779,7 @@ def _open(base, filename, seen, dl_options, override, downloaded): ...@@ -1775,7 +1779,7 @@ def _open(base, filename, seen, dl_options, override, downloaded):
Recursively open other files based on buildout options found. Recursively open other files based on buildout options found.
""" """
_update_section(dl_options, override) dl_options = _update_section(dl_options, override)
dl_options_copy = copy.deepcopy(dl_options) dl_options_copy = copy.deepcopy(dl_options)
_dl_options = _unannotate_section(dl_options_copy) _dl_options = _unannotate_section(dl_options_copy)
newest = bool_option(_dl_options, 'newest', 'false') newest = bool_option(_dl_options, 'newest', 'false')
...@@ -1841,8 +1845,9 @@ def _open(base, filename, seen, dl_options, override, downloaded): ...@@ -1841,8 +1845,9 @@ def _open(base, filename, seen, dl_options, override, downloaded):
eresult = _open(base, extends.pop(0), seen, dl_options, override, eresult = _open(base, extends.pop(0), seen, dl_options, override,
downloaded) downloaded)
for fname in extends: for fname in extends:
_update(eresult, _open(base, fname, seen, dl_options, override, next_extend = _open(base, fname, seen, dl_options, override,
downloaded)) downloaded)
eresult = _update(eresult, next_extend)
result = _update(eresult, result) result = _update(eresult, result)
seen.pop() seen.pop()
...@@ -1901,7 +1906,8 @@ def _dists_sig(dists): ...@@ -1901,7 +1906,8 @@ def _dists_sig(dists):
result.append(os.path.basename(location)) result.append(os.path.basename(location))
return result return result
def _update_section(s1, s2): def _update_section(in1, s2):
s1 = copy.deepcopy(in1)
# Base section 2 on section 1; section 1 is copied, with key-value pairs # Base section 2 on section 1; section 1 is copied, with key-value pairs
# in section 2 overriding those in section 1. If there are += or -= # in section 2 overriding those in section 1. If there are += or -=
# operators in section 2, process these to add or substract items (delimited # operators in section 2, process these to add or substract items (delimited
...@@ -1914,6 +1920,7 @@ def _update_section(s1, s2): ...@@ -1914,6 +1920,7 @@ def _update_section(s1, s2):
implicit_value = SectionKey("", "IMPLICIT_VALUE") implicit_value = SectionKey("", "IMPLICIT_VALUE")
# Find v1 in s2 first; it may have been defined locally too. # Find v1 in s2 first; it may have been defined locally too.
section_key = s2.get(key, s1.get(key, implicit_value)) section_key = s2.get(key, s1.get(key, implicit_value))
section_key = copy.deepcopy(section_key)
section_key.addToValue(v.value, v.source) section_key.addToValue(v.value, v.source)
s2[key] = section_key s2[key] = section_key
del s2[k] del s2[k]
...@@ -1922,6 +1929,7 @@ def _update_section(s1, s2): ...@@ -1922,6 +1929,7 @@ def _update_section(s1, s2):
implicit_value = SectionKey("", "IMPLICIT_VALUE") implicit_value = SectionKey("", "IMPLICIT_VALUE")
# Find v1 in s2 first; it may have been set by a += operation first # Find v1 in s2 first; it may have been set by a += operation first
section_key = s2.get(key, s1.get(key, implicit_value)) section_key = s2.get(key, s1.get(key, implicit_value))
section_key = copy.deepcopy(section_key)
section_key.removeFromValue(v.value, v.source) section_key.removeFromValue(v.value, v.source)
s2[key] = section_key s2[key] = section_key
del s2[k] del s2[k]
...@@ -1935,14 +1943,15 @@ def _update_verbose(s1, s2): ...@@ -1935,14 +1943,15 @@ def _update_verbose(s1, s2):
v1 = s1[key] v1 = s1[key]
v1.overrideValue(v2) v1.overrideValue(v2)
else: else:
s1[key] = v2 s1[key] = copy.deepcopy(v2)
def _update(d1, d2): def _update(in1, d2):
d1 = copy.deepcopy(in1)
for section in d2: for section in d2:
if section in d1: if section in d1:
d1[section] = _update_section(d1[section], d2[section]) d1[section] = _update_section(d1[section], d2[section])
else: else:
d1[section] = d2[section] d1[section] = copy.deepcopy(d2[section])
return d1 return d1
def _recipe(options): def _recipe(options):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment