normalize the way default options are handled and documented

src/zc/buildout/buildout.py

@@ -112,15 +112,24 @@ def _unannotate(data):
     return data
 
 _buildout_default_options = _annotate_section({
-    'eggs-directory': 'eggs',
-    'develop-eggs-directory': 'develop-eggs',
+    'allow-hosts': '*',
+    'allow-picked-versions': 'true',
     'bin-directory': 'bin',
-    'parts-directory': 'parts',
-    'installed': '.installed.cfg',
-    'python': 'buildout',
+    'develop-eggs-directory': 'develop-eggs',
+    'eggs-directory': 'eggs',
     'executable': sys.executable,
-    'log-level': 'INFO',
+    'find-links': '',
+    'install-from-cache': 'false',
+    'installed': '.installed.cfg',
     'log-format': '',
+    'log-level': 'INFO',
+    'newest': 'true',
+    'offline': 'false',
+    'parts-directory': 'parts',
+    'prefer-final': 'false',
+    'python': 'buildout',
+    'unzip': 'false',
+    'use-dependency-links': 'true',
     }, 'DEFAULT_VALUE')
 
 
@@ -191,7 +200,7 @@ class Buildout(UserDict.DictMixin):
         # provide some defaults before options are parsed
         # because while parsing options those attributes might be
         # used already (Gottfried Ganssauge)
-        buildout_section = data.get('buildout')
+        buildout_section = data['buildout']
 
         # Try to make sure we have absolute paths for standard
         # directories. We do this before doing substitutions, in case
@@ -204,22 +213,28 @@ class Buildout(UserDict.DictMixin):
                 d = self._buildout_path(buildout_section[name+'-directory'])
                 buildout_section[name+'-directory'] = d
 
-        links = buildout_section and buildout_section.get('find-links', '')
+        # Attributes on this buildout object shouldn't be used by
+        # recipes in their __init__.  It can cause bugs, because the
+        # recipes will be instantiated below (``options = self['buildout']``)
+        # before this has completed initializing.  These attributes are
+        # left behind for legacy support but recipe authors should
+        # beware of using them.  A better practice is for a recipe to
+        # use the buildout['buildout'] options.
+        links = buildout_section['find-links']
         self._links = links and links.split() or ()
-
-        allow_hosts = buildout_section and buildout_section.get(
-             'allow-hosts', '*').split('\n')
+        allow_hosts = buildout_section['allow-hosts'].split('\n')
         self._allow_hosts = tuple([host.strip() for host in allow_hosts
                                    if host.strip() != ''])
-
         self._logger = logging.getLogger('zc.buildout')
-        self.offline = False
-        self.newest = True
+        self.offline = (buildout_section['offline'] == 'true')
+        self.newest = (buildout_section['newest'] == 'true')
 
         ##################################################################
         ## WARNING!!!
         ## ALL ATTRIBUTES MUST HAVE REASONABLE DEFAULTS AT THIS POINT
-        ## OTHERWISE ATTRIBUTEERRORS MIGHT HAPPEN ANY TIME
+        ## OTHERWISE ATTRIBUTEERRORS MIGHT HAPPEN ANY TIME FROM RECIPES.
+        ## RECIPES SHOULD GENERALLY USE buildout['buildout'] OPTIONS, NOT
+        ## BUILDOUT ATTRIBUTES.
         ##################################################################
         # initialize some attrs and buildout directories.
         options = self['buildout']
@@ -228,7 +243,7 @@ class Buildout(UserDict.DictMixin):
         links = options.get('find-links', '')
         self._links = links and links.split() or ()
 
-        allow_hosts = options.get('allow-hosts', '*').split('\n')
+        allow_hosts = options['allow-hosts'].split('\n')
         self._allow_hosts = tuple([host.strip() for host in allow_hosts
                                    if host.strip() != ''])
 
@@ -246,44 +261,42 @@ class Buildout(UserDict.DictMixin):
 
         self._setup_logging()
 
-        offline = options.get('offline', 'false')
+        offline = options['offline']
         if offline not in ('true', 'false'):
             self._error('Invalid value for offline option: %s', offline)
-        options['offline'] = offline
-        self.offline = offline == 'true'
+        self.offline = (offline == 'true')
 
         if self.offline:
             newest = options['newest'] = 'false'
         else:
-            newest = options.get('newest', 'true')
+            newest = options['newest']
             if newest not in ('true', 'false'):
                 self._error('Invalid value for newest option: %s', newest)
-            options['newest'] = newest
-        self.newest = newest == 'true'
+        self.newest = (newest == 'true')
 
         versions = options.get('versions')
         if versions:
             zc.buildout.easy_install.default_versions(dict(self[versions]))
 
-        prefer_final = options.get('prefer-final', 'false')
+        prefer_final = options['prefer-final']
         if prefer_final not in ('true', 'false'):
             self._error('Invalid value for prefer-final option: %s',
                         prefer_final)
         zc.buildout.easy_install.prefer_final(prefer_final=='true')
 
-        use_dependency_links = options.get('use-dependency-links', 'true')
+        use_dependency_links = options['use-dependency-links']
         if use_dependency_links not in ('true', 'false'):
             self._error('Invalid value for use-dependency-links option: %s',
                         use_dependency_links)
         zc.buildout.easy_install.use_dependency_links(
             use_dependency_links == 'true')
 
-        allow_picked_versions = options.get('allow-picked-versions', 'true')
+        allow_picked_versions = options['allow-picked-versions']
         if allow_picked_versions not in ('true', 'false'):
             self._error('Invalid value for allow-picked-versions option: %s',
                         allow_picked_versions)
         zc.buildout.easy_install.allow_picked_versions(
-            allow_picked_versions=='true')
+            allow_picked_versions == 'true')
 
         download_cache = options.get('download-cache')
         if download_cache:
@@ -300,22 +313,18 @@ class Buildout(UserDict.DictMixin):
 
             zc.buildout.easy_install.download_cache(download_cache)
 
-        install_from_cache = options.get('install-from-cache')
-        if install_from_cache:
-            if install_from_cache not in ('true', 'false'):
-                self._error('Invalid value for install-from-cache option: %s',
-                            install_from_cache)
-            if install_from_cache == 'true':
-                zc.buildout.easy_install.install_from_cache(True)
-
-
-        always_unzip = options.get('unzip')
-        if always_unzip:
-            if always_unzip not in ('true', 'false'):
-                self._error('Invalid value for unzip option: %s',
-                            always_unzip)
-            if always_unzip == 'true':
-                zc.buildout.easy_install.always_unzip(True)
+        install_from_cache = options['install-from-cache']
+        if install_from_cache not in ('true', 'false'):
+            self._error('Invalid value for install-from-cache option: %s',
+                        install_from_cache)
+        zc.buildout.easy_install.install_from_cache(
+            install_from_cache=='true')
+
+        always_unzip = options['unzip']
+        if always_unzip not in ('true', 'false'):
+            self._error('Invalid value for unzip option: %s',
+                        always_unzip)
+        zc.buildout.easy_install.always_unzip(always_unzip=='true')
 
         # "Use" each of the defaults so they aren't reported as unused options.
         for name in _buildout_default_options:

src/zc/buildout/tests.py

@@ -2980,6 +2980,9 @@ def test_suite():
                    '-q develop -mxN -d /sample-buildout/develop-eggs'
                 ),
                (re.compile(r'^[*]...'), '...'),
+               # for bug_92891_bootstrap_crashes_with_egg_recipe_in_buildout_section
+               (re.compile(r"Unused options for buildout: 'eggs' 'scripts'\."),
+                "Unused options for buildout: 'scripts' 'eggs'."),
                ]),
             ),
         zc.buildout.testselectingpython.test_suite(),

zc.recipe.egg_/src/zc/recipe/egg/egg.py

@@ -25,8 +25,8 @@ class Eggs(object):
         self.buildout = buildout
         self.name = name
         self.options = options
-        links = options.get('find-links',
-                            buildout['buildout'].get('find-links'))
+        b_options = buildout['buildout']
+        links = options.get('find-links', b_options['find-links'])
         if links:
             links = links.split()
             options['find-links'] = '\n'.join(links)
@@ -34,25 +34,25 @@ class Eggs(object):
             links = ()
         self.links = links
 
-        index = options.get('index', buildout['buildout'].get('index'))
+        index = options.get('index', b_options.get('index'))
         if index is not None:
             options['index'] = index
         self.index = index
 
-        allow_hosts = buildout['buildout'].get('allow-hosts', '*')
+        allow_hosts = b_options['allow-hosts']
         allow_hosts = tuple([host.strip() for host in allow_hosts.split('\n')
                                if host.strip()!=''])
         self.allow_hosts = allow_hosts
 
-        options['eggs-directory'] = buildout['buildout']['eggs-directory']
+        options['eggs-directory'] = b_options['eggs-directory']
         options['_e'] = options['eggs-directory'] # backward compat.
-        options['develop-eggs-directory'
-                ] = buildout['buildout']['develop-eggs-directory']
+        options['develop-eggs-directory'] = b_options['develop-eggs-directory']
         options['_d'] = options['develop-eggs-directory'] # backward compat.
 
-        assert options.get('unzip') in ('true', 'false', None)
+        # verify that this is None, 'true' or 'false'
+        get_bool(options, 'unzip')
 
-        python = options.get('python', buildout['buildout']['python'])
+        python = options.get('python', b_options['python'])
         options['executable'] = buildout[python]['executable']
 
     def working_set(self, extra=()):
@@ -61,6 +61,7 @@ class Eggs(object):
         This is intended for reuse by similar recipes.
         """
         options = self.options
+        b_options = self.buildout['buildout']
 
         distributions = [
             r.strip()
@@ -76,9 +77,8 @@ class Eggs(object):
                 )
         else:
             kw = {}
-            if options.get('unzip'):
+            if 'unzip' in options:
                 kw['always_unzip'] = get_bool(options, 'unzip')
-
             ws = zc.buildout.easy_install.install(
                 distributions, options['eggs-directory'],
                 links=self.links,