Refactored the environment variable handling logic.

Python versions prior to 2.6 have an issue clearing the environment variables
using ``os.environ.clear()`` (See http://bugs.python.org/issue3227).

Instead of modifying ``os.environ`` directly we use the ``subprocess``
module to run the commands in child processes which are given an explicit
environment which is a copy of the current ``os.environ`` augmented with
the per-part overrides.

See https://github.com/hexagonit/hexagonit.recipe.cmmi/issues/issue/1/#issue/1/comment/605362
for details.

Due to this change the hook scripts no longer have the augmented environment.
They can still access the buildout configuration to read the overrides but
need to do this manually.

CHANGES.txt

 Change History
 **************
 
+1.4.1 (XXXX-XX-XX)
+==================
+
+  - Refactored the environment variable handling logic. Python versions prior
+    to 2.6 have an issue clearing the environment variables using
+    ``os.environ.clear()`` (See http://bugs.python.org/issue3227). [dokai]
+
+    Instead of modifying ``os.environ`` directly we use the ``subprocess``
+    module to run the commands in child processes which are given an explicit
+    environment which is a copy of the current ``os.environ`` augmented with
+    the per-part overrides.
+    
+    See https://github.com/hexagonit/hexagonit.recipe.cmmi/issues/issue/1/#issue/1/comment/605362
+    for details.
+    
+    .. warning:: Due to this change the hook scripts no longer have the
+                 augmented environment. They can still access the buildout
+                 configuration to read the overrides but need to do this
+                 manually.
+
 1.4.0 (2010-08-27)
 ==================
 

hexagonit/recipe/cmmi/__init__.py

@@ -4,6 +4,7 @@ import imp
 import logging
 import os
 import shutil
+import subprocess
 import zc.buildout
 
 class Recipe(object):
@@ -45,16 +46,21 @@ class Recipe(object):
                     self.environ[key.strip()] = value
                 except ValueError:
                     raise zc.buildout.UserError('Invalid environment variable definition: %s', variable)
+        # Extrapolate the environment variables using values from the current
+        # environment.
         for key in self.environ:
             self.environ[key] = self.environ[key] % os.environ
 
-    def restore_environment(self):
-        """Restores the original os.environ environment in case the recipe
-        made changes to it.
+    def augmented_environment(self):
+        """Returns a dictionary containing the current environment variables
+        augmented with the part specific overrides.
+
+        The dictionary is an independent copy of ``os.environ`` and
+        modifications will not be reflected in back in ``os.environ``.
         """
-        if self.environ:
-            os.environ.clear()
-            os.environ.update(self.original_environment)
+        env = os.environ.copy()
+        env.update(self.environ)
+        return env
 
     def update(self):
         pass
@@ -71,11 +77,22 @@ class Recipe(object):
         getattr(module, callable.strip())(self.options, self.buildout)
 
     def run(self, cmd):
+        """Run the given ``cmd`` in a child process."""
         log = logging.getLogger(self.name)
-        if os.system(cmd):
-            log.error('Error executing command: %s' % cmd)
+        try:
+            retcode = subprocess.call(cmd, shell=True, env=self.augmented_environment())
+
+            if retcode < 0:
+                log.error('Command received signal %s: %s' % (-retcode, cmd))
+                raise zc.buildout.UserError('System error')
+            elif retcode > 0:
+                log.error('Command failed with exit code %s: %s' % (retcode, cmd))
+                raise zc.buildout.UserError('System error')
+        except OSError, e:
+            log.error('Command failed: %s: %s' % (e, cmd))
             raise zc.buildout.UserError('System error')
 
+
     def install(self):
         log = logging.getLogger(self.name)
         parts = []
@@ -101,7 +118,6 @@ class Recipe(object):
         if self.environ:
             for key in sorted(self.environ.keys()):
                 log.info('[ENV] %s = %s', key, self.environ[key])
-            os.environ.update(self.environ)
 
         # Download the source using hexagonit.recipe.download
         if self.options['url']:
@@ -170,7 +186,6 @@ class Recipe(object):
                           'you can inspect what went wrong' % os.getcwd())
                 raise
         finally:
-            self.restore_environment()
             os.chdir(current_dir)
 
         if self.options['url']:

hexagonit/recipe/cmmi/tests.py

@@ -68,7 +68,7 @@ class NonInformativeTests(unittest.TestCase):
         compile_directory = os.path.join(self.dir, 'compile_directory')
         os.makedirs(compile_directory)
         self.write_file(os.path.join(compile_directory, 'configure'), 'Dummy configure')
-        
+
         recipe = self.make_recipe({}, 'test', {'path' : compile_directory})
         os.chdir(self.dir)
         self.assertEquals(self.dir, os.getcwd())
@@ -146,6 +146,10 @@ class NonInformativeTests(unittest.TestCase):
         # Make sure the sentinel value is still in the environment
         self.assertEquals(os.environ.get('HRC_SENTINEL'), 'sentinel')
 
+    def test_run__unknown_command(self):
+        recipe = self.make_recipe({}, 'test', {
+            'url' : 'file://%s/testdata/package-0.0.0.tar.gz' % os.path.dirname(__file__)})
+        self.assertRaises(zc.buildout.UserError, lambda:recipe.run('this-command-does-not-exist'))
 
 def test_suite():
     suite = unittest.TestSuite((