Write .installed.cfg only once, in safe way and only if there's any change.

Also, updating a part does not put it anymore at the end of the list of
installed parts, that was making .installed.cfg too big.

src/zc/buildout/buildout.py

@@ -36,6 +36,7 @@ import zc.buildout.configparser
 import copy
 import datetime
 import distutils.errors
+import errno
 import glob
 import itertools
 import logging
@@ -177,6 +178,12 @@ def _print_annotate(data):
                     line = '   '
     print_()
 
+def _remove_ignore_missing(path):
+    try:
+        os.remove(path)
+    except OSError as e:
+        if e.errno != errno.ENOENT:
+            raise
 
 def _unannotate_section(section):
     for key in section:
@@ -619,6 +626,15 @@ class Buildout(DictMixin):
             self.install(())
 
     def install(self, install_args):
+        try:
+            self._install_parts(install_args)
+        finally:
+            self._save_installed_options()
+        if self.show_picked_versions or self.update_versions_file:
+            self._print_picked_versions()
+        self._unload_extensions()
+
+    def _install_parts(self, install_args):
         __doing__ = 'Installing.'
 
         self._load_extensions()
@@ -632,28 +648,23 @@ class Buildout(DictMixin):
         self._maybe_upgrade()
 
         # load installed data
-        (installed_part_options, installed_exists
-         )= self._read_installed_part_options()
+        self.installed_part_options = self._read_installed_part_options()
 
         # Remove old develop eggs
         self._uninstall(
-            installed_part_options['buildout'].get(
+            self.installed_part_options['buildout'].get(
                 'installed_develop_eggs', '')
             )
 
         # Build develop eggs
         installed_develop_eggs = self._develop()
-        installed_part_options['buildout']['installed_develop_eggs'
+        self.installed_part_options['buildout']['installed_develop_eggs'
                                            ] = installed_develop_eggs
 
-        if installed_exists:
-            self._update_installed(
-                installed_develop_eggs=installed_develop_eggs)
-
         # get configured and installed part lists
         conf_parts = self['buildout']['parts']
         conf_parts = conf_parts and conf_parts.split() or []
-        installed_parts = installed_part_options['buildout']['parts']
+        installed_parts = self.installed_part_options['buildout']['parts']
         installed_parts = installed_parts and installed_parts.split() or []
 
         if install_args:
@@ -685,7 +696,7 @@ class Buildout(DictMixin):
         # have changed
         for part in reversed(installed_parts):
             if part in install_parts:
-                old_options = installed_part_options[part].copy()
+                old_options = self.installed_part_options[part].copy()
                 installed_files = old_options.pop('__buildout_installed__')
                 new_options = self.get(part)
                 if old_options == new_options:
@@ -718,12 +729,9 @@ class Buildout(DictMixin):
             elif not uninstall_missing:
                 continue
 
-            self._uninstall_part(part, installed_part_options)
+            self._uninstall_part(part, self.installed_part_options)
             installed_parts = [p for p in installed_parts if p != part]
 
-            if installed_exists:
-                self._update_installed(parts=' '.join(installed_parts))
-
         # Check for unused buildout options:
         _check_for_unused_options_in_section(self, 'buildout')
 
@@ -733,10 +741,9 @@ class Buildout(DictMixin):
             saved_options = self[part].copy()
             recipe = self[part].recipe
             if part in installed_parts: # update
-                need_to_save_installed = False
                 __doing__ = 'Updating %s.', part
                 self._logger.info(*__doing__)
-                old_options = installed_part_options[part]
+                old_options = self.installed_part_options[part]
                 old_installed_files = old_options['__buildout_installed__']
 
                 try:
@@ -753,9 +760,8 @@ class Buildout(DictMixin):
                 except:
                     installed_parts.remove(part)
                     self._uninstall(old_installed_files)
-                    if installed_exists:
-                        self._update_installed(
-                            parts=' '.join(installed_parts))
+                    self.installed_part_options['buildout']['parts'] = (
+                        ' '.join(installed_parts))
                     raise
 
                 old_installed_files = old_installed_files.split('\n')
@@ -776,10 +782,14 @@ class Buildout(DictMixin):
                                            + need_to_save_installed)
 
             else: # install
-                need_to_save_installed = True
                 __doing__ = 'Installing %s.', part
                 self._logger.info(*__doing__)
-                installed_files = self[part]._call(recipe.install)
+                try:
+                    installed_files = self[part]._call(recipe.install)
+                except:
+                    self.installed_part_options['buildout']['parts'] = (
+                        ' '.join(installed_parts))
+                    raise
                 if installed_files is None:
                     self._logger.warning(
                         "The %s install returned None.  A path or "
@@ -791,7 +801,7 @@ class Buildout(DictMixin):
                 else:
                     installed_files = list(installed_files)
 
-            installed_part_options[part] = saved_options
+            self.installed_part_options[part] = saved_options
             saved_options['__buildout_installed__'
                           ] = '\n'.join(installed_files)
             saved_options['__buildout_signature__'] = signature
@@ -800,32 +810,11 @@ class Buildout(DictMixin):
             installed_parts.append(part)
             _check_for_unused_options_in_section(self, part)
 
-            if need_to_save_installed:
-                installed_part_options['buildout']['parts'] = (
-                    ' '.join(installed_parts))
-                self._save_installed_options(installed_part_options)
-                installed_exists = True
-            else:
-                assert installed_exists
-                self._update_installed(parts=' '.join(installed_parts))
+            self.installed_part_options['buildout']['parts'] = (
+                ' '.join(installed_parts))
 
-        if installed_develop_eggs:
-            if not installed_exists:
-                self._save_installed_options(installed_part_options)
-        elif (not installed_parts) and installed_exists:
-            os.remove(self['buildout']['installed'])
-
-        if self.show_picked_versions or self.update_versions_file:
-            self._print_picked_versions()
-        self._unload_extensions()
-
-    def _update_installed(self, **buildout_options):
-        installed = self['buildout']['installed']
-        f = open(installed, 'a')
-        f.write('\n[buildout]\n')
-        for option, value in list(buildout_options.items()):
-            _save_option(option, value, f)
-        f.close()
+        self.installed_part_options['buildout']['parts'] = (
+            ' '.join(installed_parts))
 
     def _uninstall_part(self, part, installed_part_options):
         # uninstall part
@@ -947,11 +936,9 @@ class Buildout(DictMixin):
                         options[option] = value
                 result[section] = self.Options(self, section, options)
 
-            return result, True
+            return result
         else:
-            return ({'buildout': self.Options(self, 'buildout', {'parts': ''})},
-                    False,
-                    )
+            return {'buildout': self.Options(self, 'buildout', {'parts': ''})}
 
     def _uninstall(self, installed):
         for f in installed.split('\n'):
@@ -992,16 +979,40 @@ class Buildout(DictMixin):
         return ' '.join(installed)
 
 
-    def _save_installed_options(self, installed_options):
+    def _save_installed_options(self):
+        installed_options = getattr(self, 'installed_part_options', None)
         installed = self['buildout']['installed']
-        if not installed:
+        if not installed_options or not installed:
             return
-        f = open(installed, 'w')
-        _save_options('buildout', installed_options['buildout'], f)
-        for part in installed_options['buildout']['parts'].split():
-            print_(file=f)
-            _save_options(part, installed_options[part], f)
-        f.close()
+        buildout = installed_options['buildout']
+        installed_parts = buildout['parts'].split()
+        if installed_parts or buildout['installed_develop_eggs']:
+            new = StringIO()
+            buildout['parts'] = ' '.join(installed_parts)
+            _save_options('buildout', buildout, new)
+            for part in installed_parts:
+                new.write('\n')
+                _save_options(part, installed_options[part], new)
+            new = new.getvalue()
+            try:
+                with open(installed) as f:
+                    save = f.read(1+len(new)) != new
+            except IOError as e:
+                if e.errno != errno.ENOENT:
+                    raise
+                save = True
+            if save:
+                installed_tmp = installed + ".tmp"
+                try:
+                    with open(installed_tmp, "w") as f:
+                        f.write(new)
+                        f.flush()
+                        os.fsync(f.fileno())
+                    os.rename(installed_tmp, installed)
+                finally:
+                    _remove_ignore_missing(installed_tmp)
+        else:
+            _remove_ignore_missing(installed)
 
     def _error(self, message, *args):
         raise zc.buildout.UserError(message % args)
@@ -1397,12 +1408,16 @@ class Options(DictMixin):
     def _dosub(self, option, v):
         __doing__ = 'Getting option %s:%s.', self.name, option
         seen = [(self.name, option)]
-        v = '$$'.join([self._sub(s, seen) for s in v.split('$$')])
+        v = '$$'.join([self._sub(s, seen, last=False)
+                       for s in v.split('$$')])
         self._cooked[option] = v
 
-    def get(self, option, default=None, seen=None):
+    def get(self, option, default=None, seen=None, last=True):
         try:
-            return self._data[option]
+            if last:
+                return self._data[option].replace('$${', '${')
+            else:
+                return self._data[option]
         except KeyError:
             pass
 
@@ -1424,16 +1439,20 @@ class Options(DictMixin):
                     )
             else:
                 seen.append(key)
-            v = '$$'.join([self._sub(s, seen) for s in v.split('$$')])
+            v = '$$'.join([self._sub(s, seen, last=False)
+                           for s in v.split('$$')])
             seen.pop()
 
         self._data[option] = v
-        return v
+        if last:
+            return v.replace('$${', '${')
+        else:
+            return v
 
     _template_split = re.compile('([$]{[^}]*})').split
     _simple = re.compile('[-a-zA-Z0-9 ._]+$').match
     _valid = re.compile('\${[-a-zA-Z0-9 ._]*:[-a-zA-Z0-9 ._]+}$').match
-    def _sub(self, template, seen):
+    def _sub(self, template, seen, last=True):
         value = self._template_split(template)
         subs = []
         for ref in value[1::2]:
@@ -1463,7 +1482,7 @@ class Options(DictMixin):
                 section = self.name
             elif section != 'buildout':
                 self._dependency.add(section)
-            v = self.buildout[section].get(option, None, seen)
+            v = self.buildout[section].get(option, None, seen, last=last)
             if v is None:
                 if option == '_buildout_section_name_':
                     v = self.name
@@ -1476,14 +1495,6 @@ class Options(DictMixin):
         return ''.join([''.join(v) for v in zip(value[::2], subs)])
 
     def __getitem__(self, key):
-        try:
-            v = self._data[key]
-            if v.startswith(SERIALISED_VALUE_MAGIC):
-                v = loads(v)
-            return v
-        except KeyError:
-            pass
-
         v = self.get(key)
         if v is None:
             raise MissingOption("Missing option: %s:%s" % (self.name, key))

src/zc/buildout/debugging.txt

@@ -85,6 +85,8 @@ supply some input:
       File "/zc/buildout/buildout.py", line 1352, in main
         getattr(buildout, command)(args)
       File "/zc/buildout/buildout.py", line 383, in install
+        self._install_parts(install_args)
+      File buildout.py", line 791, in _install_parts
         installed_files = self[part]._call(recipe.install)
       File "/zc/buildout/buildout.py", line 961, in _call
         return f()

src/zc/buildout/tests.py

@@ -1498,7 +1498,7 @@ some evil recipes that exit uncleanly:
     >>> mkdir('recipes')
     >>> write('recipes', 'recipes.py',
     ... '''
-    ... import os
+    ... import sys
     ...
     ... class Clean:
     ...     def __init__(*_): pass
@@ -1506,10 +1506,10 @@ some evil recipes that exit uncleanly:
     ...     def update(_): pass
     ...
     ... class EvilInstall(Clean):
-    ...     def install(_): os._exit(1)
+    ...     def install(_): sys.exit(1)
     ...
     ... class EvilUpdate(Clean):
-    ...     def update(_): os._exit(1)
+    ...     def update(_): sys.exit(1)
     ... ''')
 
     >>> write('recipes', 'setup.py',
@@ -1606,7 +1606,6 @@ Now let's look at 3 cases:
     Uninstalling p2.
     Uninstalling p1.
     Uninstalling p4.
-    Uninstalling p3.
 
 3. We exit while installing or updating after uninstalling:
 
@@ -1682,7 +1681,6 @@ Now let's look at 3 cases:
 
     >>> print_(system(buildout), end='')
     Develop: '/sample-buildout/recipes'
-    Uninstalling p1.
     Installing p1.
     Updating p2.
     Updating p3.
@@ -2742,6 +2740,73 @@ def increment_on_command_line():
       recipe='zc.buildout:debug'
     """
 
+def bug_664539_simple_buildout():
+  r"""
+  >>> write('buildout.cfg', '''
+  ... [buildout]
+  ... parts = escape
+  ...
+  ... [escape]
+  ... recipe = zc.buildout:debug
+  ... foo = $${nonexistent:option}
+  ... ''')
+
+  >>> print_(system(buildout), end='')
+  Installing escape.
+    foo='${nonexistent:option}'
+    recipe='zc.buildout:debug'
+  """
+
+def bug_664539_reference():
+  r"""
+  >>> write('buildout.cfg', '''
+  ... [buildout]
+  ... parts = escape
+  ...
+  ... [escape]
+  ... recipe = zc.buildout:debug
+  ... foo = ${:bar}
+  ... bar = $${nonexistent:option}
+  ... ''')
+
+  >>> print_(system(buildout), end='')
+  Installing escape.
+    bar='${nonexistent:option}'
+    foo='${nonexistent:option}'
+    recipe='zc.buildout:debug'
+  """
+
+def bug_664539_complex_buildout():
+  r"""
+  >>> write('buildout.cfg', '''
+  ... [buildout]
+  ... parts = escape
+  ...
+  ... [escape]
+  ... recipe = zc.buildout:debug
+  ... foo = ${level1:foo}
+  ...
+  ... [level1]
+  ... recipe = zc.buildout:debug
+  ... foo = ${level2:foo}
+  ...
+  ... [level2]
+  ... recipe = zc.buildout:debug
+  ... foo = $${nonexistent:option}
+  ... ''')
+
+  >>> print_(system(buildout), end='')
+  Installing level2.
+    foo='${nonexistent:option}'
+    recipe='zc.buildout:debug'
+  Installing level1.
+    foo='${nonexistent:option}'
+    recipe='zc.buildout:debug'
+  Installing escape.
+    foo='${nonexistent:option}'
+    recipe='zc.buildout:debug'
+  """
+
 def test_constrained_requirement():
     """
     zc.buildout.easy_install._constrained_requirement(constraint, requirement)