Fixed some bugs in variable substitutions.

The characters "-", "." and " ", weren't allowed in section or
option names.

Substitutions with invalid names were ignored, which caused
missleading failures downstream.

README.txt

@@ -192,6 +192,14 @@ Change History
 1.0.0b3
 -------
 
+- Fixed some bugs in variable substitutions.  
+
+  The characters "-", "." and " ", weren't allowed in section or
+  option names.
+
+  Substitutions with invalid names were ignored, which caused
+  missleading failures downstream.
+
 - Improved error handling.  No longer show tracebacks for user errors.
 
 - Now require a recipe option (and therefore a section) for every part.

src/zc/buildout/buildout.py

@@ -158,12 +158,32 @@ class Buildout(dict):
         seen.pop()
         return value
 
-    _template_split = re.compile('([$]{\w+:\w+})').split
+    _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 _dosubs_esc(self, value, data, converted, seen):
         value = self._template_split(value)
         subs = []
-        for s in value[1::2]:
-            s = tuple(s[2:-1].split(':'))
+        for ref in value[1::2]:
+            s = tuple(ref[2:-1].split(':'))
+            if not self._valid(ref):
+                if len(s) < 2:
+                    raise UserError("The substitution, %s,\n"
+                                    "doesn't contain a colon."
+                                    % ref)
+                if len(s) > 2:
+                    raise UserError("The substitution, %s,\n"
+                                    "has too many colons."
+                                    % ref)
+                if not self._simple(s[0]):
+                    raise UserError("The section name in substitution, %s,\n"
+                                    "has invalid characters."
+                                    % ref)
+                if not self._simple(s[1]):
+                    raise UserError("The option name in substitution, %s,\n"
+                                    "has invalid characters."
+                                    % ref)
+                
             v = converted.get(s)
             if v is None:
                 options = data.get(s[0])
@@ -414,6 +434,7 @@ class Buildout(dict):
         old = self._installed_path()
         if os.path.isfile(old):
             parser = ConfigParser.SafeConfigParser(_spacey_defaults)
+            parser.optionxform = lambda s: s
             parser.read(old)
             return dict([
                 (section,
@@ -556,6 +577,7 @@ def _open(base, filename, seen):
     result = {}
 
     parser = ConfigParser.SafeConfigParser()
+    parser.optionxform = lambda s: s
     parser.readfp(open(filename))
     extends = extended_by = None
     for section in parser.sections():

src/zc/buildout/buildout.txt

@@ -212,10 +212,10 @@ Now let's update our buildout.cfg:
     ... """
     ... [buildout]
     ... develop = recipes
-    ... parts = data_dir
+    ... parts = data-dir
     ... log-level = INFO
     ...
-    ... [data_dir]
+    ... [data-dir]
     ... recipe = recipes:mkdir
     ... path = mystuff
     ... """)
@@ -235,7 +235,7 @@ provide locally-defined recipes needed by the parts.
 
 ::
 
-    parts = data_dir
+    parts = data-dir
 
 Here we've named a part to be "built".  We can use any name we want
 except that different part names must be unique and recipes will often
@@ -251,7 +251,7 @@ buildout and recipes are doing.
 
 ::
 
-    [data_dir]
+    [data-dir]
     recipe = recipes:mkdir
     path = mystuff    
 
@@ -269,8 +269,8 @@ buildout:
     >>> buildout = os.path.join(sample_buildout, 'bin', 'buildout')
     >>> print system(buildout),
     buildout: Running /tmp/sample-buildout/recipes/setup.py -q develop ...
-    buildout: Installing data_dir
-    data_dir: Creating directory mystuff
+    buildout: Installing data-dir
+    data-dir: Creating directory mystuff
 
 We see that the recipe created the directory, as expected:
 
@@ -289,9 +289,9 @@ installed:
 
     >>> cat(sample_buildout, '.installed.cfg')
     [buildout]
-    parts = data_dir
+    parts = data-dir
     <BLANKLINE>
-    [data_dir]
+    [data-dir]
     __buildout_installed__ = /tmp/sample-buildout/mystuff
     __buildout_signature__ = recipes-c7vHV6ekIDUPy/7fjAaYjg==
     path = /tmp/sample-buildout/mystuff
@@ -308,19 +308,19 @@ we'll see that the directory gets removed and recreated:
     ... """
     ... [buildout]
     ... develop = recipes
-    ... parts = data_dir
+    ... parts = data-dir
     ... log-level = INFO
     ...
-    ... [data_dir]
+    ... [data-dir]
     ... recipe = recipes:mkdir
     ... path = mydata
     ... """)
 
     >>> print system(buildout),
     buildout: Running /tmp/sample-buildout/recipes/setup.py -q develop ...
-    buildout: Uninstalling data_dir
-    buildout: Installing data_dir
-    data_dir: Creating directory mydata
+    buildout: Uninstalling data-dir
+    buildout: Installing data-dir
+    data-dir: Creating directory mydata
 
     >>> ls(sample_buildout)
     -  .installed.cfg
@@ -332,6 +332,30 @@ we'll see that the directory gets removed and recreated:
     d  parts
     d  recipes
 
+Configuration file syntax
+-------------------------
+
+As mentioned earlier, buildout configuration files use the format
+defined by the Python ConfigParser module with extensions.  The
+extensions are:
+
+- option names are case sensitive
+
+- option values can ue a substitution syntax, described below, to
+  refer to option values in specific sections.
+
+The ConfigParser syntax is very flexible.  Section names can contain
+any characters other than newlines and right square braces ("]").
+Option names can contain any characters other than newlines, colons,
+and equal signs, can not start with a space, and don't include
+trailing spaces.
+
+It is likely that, in the future, some characters will be given
+special buildout-defined meanings.  This is already true of the
+characters ":", "$", "%", "(", and ")".  For now, it is a good idea to
+keep section and option names simple, sticking to alphanumeric
+characters, hyphens, and periods.
+
 Variable substitutions
 ----------------------
 
@@ -386,16 +410,17 @@ examples:
     ... """
     ... [buildout]
     ... develop = recipes
-    ... parts = data_dir debug
+    ... parts = data-dir debug
     ... log-level = INFO
     ...
     ... [debug]
     ... recipe = recipes:debug
-    ... file1 = ${data_dir:path}/file
-    ... file2 = %(file1)s.out
-    ... file3 = %(base)s/file3
+    ... File 1 = ${data-dir:path}/file
+    ... File 2 = %(File 1)s.out
+    ... File 3 = %(base)s/file3
+    ... File 4 = ${debug:File 3}/log
     ...
-    ... [data_dir]
+    ... [data-dir]
     ... recipe = recipes:mkdir
     ... path = mydata
     ...
@@ -418,14 +443,15 @@ substituted.
 
     >>> print system(buildout),
     buildout: Running /tmp/sample-buildout/recipes/setup.py -q develop ...
-    buildout: Uninstalling data_dir
-    buildout: Installing data_dir
-    data_dir: Creating directory mydata
+    buildout: Uninstalling data-dir
+    buildout: Installing data-dir
+    data-dir: Creating directory mydata
     buildout: Installing debug
+    File 1 mydata/file
+    File 2 mydata/file.out
+    File 3 var/file3
+    File 4 var/file3/log
     base var
-    file1 mydata/file
-    file2 mydata/file.out
-    file3 var/file3
     recipe recipes:debug
 
 It might seem surprising that mydata was created again.  This is
@@ -436,12 +462,13 @@ the buildout:
 
     >>> print system(buildout),
     buildout: Running /tmp/sample-buildout/recipes/setup.py -q develop ...
-    buildout: Installing data_dir
+    buildout: Installing data-dir
     buildout: Installing debug
+    File 1 mydata/file
+    File 2 mydata/file.out
+    File 3 var/file3
+    File 4 var/file3/log
     base var
-    file1 mydata/file
-    file2 mydata/file.out
-    file3 var/file3
     recipe recipes:debug
 
 We can see that mydata was not recreated.
@@ -449,6 +476,10 @@ We can see that mydata was not recreated.
 Note that, in this case, we didn't specify a log level, so
 we didn't get output about what the buildout was doing.
 
+Section and option names in variable substitutions are only allowed to
+contain alphanumeric characters, hyphens, periods and spaces. This
+restriction might be relaxed in future releases.
+
 Multiple configuration files
 ----------------------------
 

src/zc/buildout/tests.py

@@ -59,6 +59,58 @@ It is an error to create a variable-reference cycle:
     We're evaluating buildout:y, buildout:z, buildout:x
     and are referencing: buildout:y.
 
+It is an error to use funny characters in variable refereces:
+
+    >>> write(sample_buildout, 'buildout.cfg',
+    ... '''
+    ... [buildout]
+    ... develop = recipes
+    ... parts = data_dir debug
+    ... x = ${bui$ldout:y}
+    ... ''')
+
+    >>> print system(os.path.join(sample_buildout, 'bin', 'buildout')),
+    Error: The section name in substitution, ${bui$ldout:y},
+    has invalid characters.
+
+    >>> write(sample_buildout, 'buildout.cfg',
+    ... '''
+    ... [buildout]
+    ... develop = recipes
+    ... parts = data_dir debug
+    ... x = ${buildout:y{z}
+    ... ''')
+
+    >>> print system(os.path.join(sample_buildout, 'bin', 'buildout')),
+    Error: The option name in substitution, ${buildout:y{z},
+    has invalid characters.
+
+and too have too many or too few colons:
+
+    >>> write(sample_buildout, 'buildout.cfg',
+    ... '''
+    ... [buildout]
+    ... develop = recipes
+    ... parts = data_dir debug
+    ... x = ${parts}
+    ... ''')
+
+    >>> print system(os.path.join(sample_buildout, 'bin', 'buildout')),
+    Error: The substitution, ${parts},
+    doesn't contain a colon.
+
+    >>> write(sample_buildout, 'buildout.cfg',
+    ... '''
+    ... [buildout]
+    ... develop = recipes
+    ... parts = data_dir debug
+    ... x = ${buildout:y:z}
+    ... ''')
+
+    >>> print system(os.path.join(sample_buildout, 'bin', 'buildout')),
+    Error: The substitution, ${buildout:y:z},
+    has too many colons.
+
 Al parts have to have a section:
 
     >>> write(sample_buildout, 'buildout.cfg',
@@ -160,7 +212,6 @@ uninstalling anything because the configuration hasn't changed.
     buildout: Installing debug
 """
 
-
 def linkerSetUp(test):
     zc.buildout.testing.buildoutSetUp(test, clear_home=False)
     zc.buildout.testing.multi_python(test)