[fix] 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

@@ -29,6 +29,11 @@ try:
 except ImportError:
     from UserDict import DictMixin
 
+try:
+    from cStringIO import StringIO
+except ImportError:
+    from io import StringIO
+
 try:
     from urllib.parse import urljoin
 except ImportError: # BBB Py2
@@ -38,6 +43,7 @@ import zc.buildout.configparser
 import copy
 import datetime
 import distutils.errors
+import errno
 import glob
 import importlib
 import inspect
@@ -264,6 +270,12 @@ def _print_annotate(data, verbose, chosen_sections, basedir):
                 sectionkey = data[section][key]
                 sectionkey.printAll(key, basedir, verbose)
 
+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:
@@ -330,6 +342,8 @@ class Buildout(DictMixin):
 
     COMMANDS = set()
 
+    installed_part_options = None
+
     def __init__(self, config_file, cloptions,
                  use_user_defaults=True,
                  command=None, args=()):
@@ -744,6 +758,19 @@ class Buildout(DictMixin):
 
     @command
     def install(self, install_args):
+        try:
+            self._install_parts(install_args)
+        finally:
+            if self.installed_part_options is not None:
+                try:
+                    self._save_installed_options()
+                finally:
+                    del self.installed_part_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()
@@ -757,8 +784,8 @@ class Buildout(DictMixin):
         self._maybe_upgrade()
 
         # load installed data
-        (installed_part_options, installed_exists
-         )= self._read_installed_part_options()
+        installed_part_options = self._read_installed_part_options()
+        installed_parts = installed_part_options['buildout']['parts'].split()
 
         # Remove old develop eggs
         self._uninstall(
@@ -771,21 +798,15 @@ class Buildout(DictMixin):
         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 = installed_parts and installed_parts.split() or []
+        # From now, the caller will update the .installed.cfg at return.
+        self.installed_part_options = installed_part_options
 
+        install_parts = self['buildout']['parts']
         if install_args:
             install_parts = install_args
             uninstall_missing = False
         else:
-            install_parts = conf_parts
+            install_parts = install_parts.split()
             uninstall_missing = True
 
         # load and initialize recipes
@@ -846,9 +867,8 @@ class Buildout(DictMixin):
 
             self._uninstall_part(part, installed_part_options)
             installed_parts = [p for p in installed_parts if p != part]
-
-            if installed_exists:
-                self._update_installed(parts=' '.join(installed_parts))
+            installed_part_options['buildout']['parts'] = (
+                ' '.join(installed_parts))
 
         # Check for unused buildout options:
         _check_for_unused_options_in_section(self, 'buildout')
@@ -859,11 +879,10 @@ 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_installed_files = old_options['__buildout_installed__']
+                installed_files = old_options['__buildout_installed__']
 
                 try:
                     update = recipe.update
@@ -875,34 +894,21 @@ class Buildout(DictMixin):
                         part)
 
                 try:
-                    installed_files = self[part]._call(update)
-                except:
+                    updated_files = self[part]._call(update)
+                except Exception:
                     installed_parts.remove(part)
-                    self._uninstall(old_installed_files)
-                    if installed_exists:
-                        self._update_installed(
-                            parts=' '.join(installed_parts))
+                    self._uninstall(installed_files)
+                    installed_part_options['buildout']['parts'] = (
+                        ' '.join(installed_parts))
                     raise
 
-                old_installed_files = old_installed_files.split('\n')
-                if installed_files is None:
-                    installed_files = old_installed_files
-                else:
-                    if isinstance(installed_files, str):
-                        installed_files = [installed_files]
-                    else:
-                        installed_files = list(installed_files)
-
-                    need_to_save_installed = [
-                        p for p in installed_files
-                        if p not in old_installed_files]
-
-                    if need_to_save_installed:
-                        installed_files = (old_installed_files
-                                           + need_to_save_installed)
+                if updated_files:
+                    installed_files = set(installed_files.split('\n'))
+                    (installed_files.add if isinstance(updated_files, str) else
+                     installed_files.update)(updated_files)
+                    installed_files = '\n'.join(sorted(installed_files))
 
             else: # install
-                need_to_save_installed = True
                 __doing__ = 'Installing %s.', part
                 self._logger.info(*__doing__)
                 installed_files = self[part]._call(recipe.install)
@@ -911,47 +917,19 @@ class Buildout(DictMixin):
                         "The %s install returned None.  A path or "
                         "iterable os paths should be returned.",
                         part)
-                    installed_files = ()
-                elif isinstance(installed_files, str):
-                    installed_files = [installed_files]
-                else:
-                    installed_files = list(installed_files)
+                    installed_files = ""
+                elif not isinstance(installed_files, str):
+                    installed_files = '\n'.join(installed_files)
 
-            installed_part_options[part] = saved_options
-            saved_options['__buildout_installed__'
-                          ] = '\n'.join(installed_files)
+            saved_options['__buildout_installed__'] = installed_files
             saved_options['__buildout_signature__'] = signature
+            installed_part_options[part] = saved_options
 
-            installed_parts = [p for p in installed_parts if p != part]
-            installed_parts.append(part)
-            _check_for_unused_options_in_section(self, part)
-
-            if need_to_save_installed:
+            if part not in installed_parts:
+                installed_parts.append(part)
                 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))
-
-        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()
+                _check_for_unused_options_in_section(self, part)
 
     def _uninstall_part(self, part, installed_part_options):
         # uninstall part
@@ -1068,11 +1046,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'):
@@ -1113,16 +1089,39 @@ class Buildout(DictMixin):
         return ' '.join(installed)
 
 
-    def _save_installed_options(self, installed_options):
-        installed = self['buildout']['installed']
-        if not installed:
+    def _save_installed_options(self):
+        installed_path = self['buildout']['installed']
+        if not installed_path:
             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()
+        installed_part_options = self.installed_part_options
+        buildout = installed_part_options['buildout']
+        installed_parts = buildout['parts']
+        if installed_parts or buildout['installed_develop_eggs']:
+            new = StringIO()
+            _save_options('buildout', buildout, new)
+            for part in installed_parts.split():
+                new.write('\n')
+                _save_options(part, installed_part_options[part], new)
+            new = new.getvalue()
+            try:
+                with open(installed_path) 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_path + ".tmp"
+                try:
+                    with open(installed_tmp, "w") as f:
+                        f.write(new)
+                        f.flush()
+                        os.fsync(f.fileno())
+                    os.rename(installed_tmp, installed_path)
+                finally:
+                    _remove_ignore_missing(installed_tmp)
+        else:
+            _remove_ignore_missing(installed_path)
 
     def _error(self, message, *args):
         raise zc.buildout.UserError(message % args)
@@ -1434,10 +1433,6 @@ class Buildout(DictMixin):
         self[name] # Add to parts
 
     def parse(self, data):
-        try:
-            from cStringIO import StringIO
-        except ImportError:
-            from io import StringIO
         import textwrap
 
         sections = zc.buildout.configparser.parse(

src/zc/buildout/tests/buildout.txt

@@ -2804,10 +2804,10 @@ provide alternate locations, and even names for these directories::
     Creating directory '/sample-alt/work'.
     Creating directory '/sample-alt/developbasket'.
     Develop: '/sample-buildout/recipes'
-    Uninstalling d4.
-    Uninstalling d3.
     Uninstalling d2.
     Uninstalling debug.
+    Uninstalling d4.
+    Uninstalling d3.
 
     >>> ls(alt)
     d  basket

src/zc/buildout/tests/test_all.py

@@ -1679,7 +1679,7 @@ some evil recipes that exit uncleanly:
     >>> mkdir('recipes')
     >>> write('recipes', 'recipes.py',
     ... '''
-    ... import os
+    ... import sys
     ...
     ... class Clean:
     ...     def __init__(*_): pass
@@ -1687,10 +1687,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',
@@ -1784,10 +1784,10 @@ Now let's look at 3 cases:
 
     >>> print_(system(buildout+' buildout:parts='), end='')
     Develop: '/sample-buildout/recipes'
-    Uninstalling p2.
-    Uninstalling p1.
     Uninstalling p4.
     Uninstalling p3.
+    Uninstalling p2.
+    Uninstalling p1.
 
 3. We exit while installing or updating after uninstalling:
 

zc.recipe.egg_/src/zc/recipe/egg/custom.rst

@@ -434,8 +434,8 @@ Create a clean buildout.cfg w/o the checkenv recipe, and delete the recipe:
     ... """ % dict(server=link_server))
     >>> print_(system(buildout), end='') # doctest: +ELLIPSIS
     Develop: '/sample-buildout/recipes'
-    Uninstalling checkenv.
     Uninstalling extdemo.
+    Uninstalling checkenv.
     Installing extdemo...
 
     >>> rmdir(sample_buildout, 'recipes')