Pass the augmented environment to the hook scripts.

The call signature for the hook scripts was changed by adding a third
parameter which is a dictionary containing the environment variables copied
from ``os.environ`` and augmented with the environment overrides from the
part configuration.

Existing hook scripts that accept only two arguments continue to work but
reading ``os.environ`` directly will not contain the overridden values.

CHANGES.txt

 Change History
 **************
 
-1.4.1 (XXXX-XX-XX)
+1.5.0 (XXXX-XX-XX)
 ==================
 
   - Refactored the environment variable handling logic. Python versions prior
@@ -11,15 +11,19 @@ Change History
     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.
-    
+    the per-part overrides. As a result, ``os.environ`` is no longer modified
+    by this recipe.
+
+    The `Python hook scripts`_ are passed the augmented environment dictionary
+    as a third parameter.
+
+    .. warning:: Existing hook scripts accepting only two parameters
+                 continue to work but they do not have access to the modified
+                 environment variables. To fix this they should be refactored
+                 to accept the third parameter.
+
     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)
 ==================
@@ -34,7 +38,7 @@ Change History
       - the ``configure-command`` is not used to specify a custom configure command and
 
       - ``--prefix`` is not given explicitly in the ``configure-options`` option.
-      
+
     [dokai]
 
   - Removed the ``is_build_dir()`` heuristic.
@@ -45,7 +49,7 @@ Change History
     an error message if they were missing. However, the recipe is useful for
     building many different kinds of software packages and checking for
     particular files limited its use severely.
-    
+
     Now the recipe omits any checks for particular files in the downloaded
     package. It is recommended that you use the ``md5sum`` option in your part
     configuration to assert that you are downloading the package you expect

hexagonit/recipe/cmmi/README.txt

@@ -69,17 +69,32 @@ Supported options
     List of patch files to the applied to the extracted source. Each
     file should be given on a separate line.
 
+.. _Python hook scripts:
+
 ``pre-configure-hook``
     Custom python script that will be executed before running the
     ``configure`` script. The format of the options is::
 
         /path/to/the/module.py:name_of_callable
 
-    where the first part is a filesystem path to the python module and
-    the second part is the name of the callable in the module that
-    will be called. The callable will be passed two parameters: the
-    ``options`` dictionary from the recipe and the global ``buildout``
-    dictionary. The callable is not expected to return anything.
+    where the first part is a filesystem path to the python module and the
+    second part is the name of the callable in the module that will be called.
+    The callable will be passed three parameters in the following order:
+
+        1. The ``options`` dictionary from the recipe.
+
+        2. The global ``buildout`` dictionary.
+
+        3. A dictionary containing the current ``os.environ`` augmented with
+           the part specific overrides.
+
+    The callable is not expected to return anything.
+
+    .. note:: The ``os.environ`` is not modified so if the hook script is
+              interested in the environment variable overrides defined for the
+              part it needs to read them from the dictionary that is passed in
+              as the third parameter instead of accessing ``os.environ``
+              directly.
 
 ``pre-make-hook``
     Custom python script that will be executed before running
@@ -102,18 +117,25 @@ Supported options
 ``environment-section``
 
     Name of a section that provides environment variables that will be used to
-    update ``os.environ`` before executing the recipe.
-    
+    augment the variables read from ``os.environ`` before executing the
+    recipe.
+
+    This recipe does not modify ``os.environ`` directly. External commands
+    run as part of the recipe (e.g. make, configure, etc.) get an augmented
+    environment when they are forked. Python hook scripts are passed the
+    augmented as a parameter.
+
     The values of the environment variables may contain references to other
     existing environment variables (including themselves) in the form of
     Python string interpolation variables using the dictionary notation. These
-    references will be expanded before ``os.environ`` is updated. This can be
+    references will be expanded using values from ``os.environ``. This can be
     used, for example, to append to the ``PATH`` variable, e.g.::
-        
+
         [component]
         recipe = hexagonit.recipe.cmmi
-        environment-section = environment
-        
+        environment-section =
+            environment
+
         [environment]
         PATH = %(PATH)s:${buildout:directory}/bin
 
@@ -122,7 +144,7 @@ Supported options
   A sequence of ``KEY=VALUE`` pairs separated by newlines that define
   additional environment variables used to update ``os.environ`` before
   executing the recipe.
-  
+
   The semantics of this option are the same as ``environment-section``. If
   both ``environment-section`` and ``environment`` are provided the values from
   the former will be overridden by the latter allowing per-part customization.
@@ -203,7 +225,7 @@ a custom location within the buildout::
     ... configure-command = perl -I${buildout:perl_lib}/lib/perl5 Makefile.PL INSTALL_BASE=${buildout:perl_lib}
     ... url = file://%s/Foo-Bar-0.0.0.tar.gz
     ... """ % src)
-    
+
     >>> print system(buildout)
     Uninstalling package.
     Installing foobar.
@@ -254,15 +276,15 @@ Makefile and using explicit ``make`` options to control the build process.
 Installing checkouts
 ====================
 
-Sometimes instead of downloading and building an existing tarball we
-need to work with code that is already available on the filesystem,
-for example an SVN checkout.
+Sometimes instead of downloading and building an existing tarball we need to
+work with code that is already available on the filesystem, for example an SVN
+checkout.
 
-Instead of providing the ``url`` option we will provide a ``path``
-option to the directory containing the source code.
+Instead of providing the ``url`` option we will provide a ``path`` option to
+the directory containing the source code.
 
-Let's demonstrate this by first unpacking our test package to the
-filesystem and building that.
+Let's demonstrate this by first unpacking our test package to the filesystem
+and building that.
 
     >>> checkout_dir = tmpdir('checkout')
     >>> import setuptools.archive_util
@@ -290,21 +312,21 @@ filesystem and building that.
     building package
     installing package
 
-Since using the ``path`` implies that the source code has been
-acquired outside of the control of the recipe also the responsibility
-of managing it is outside of the recipe.
+Since using the ``path`` implies that the source code has been acquired
+outside of the control of the recipe also the responsibility of managing it is
+outside of the recipe.
 
-Depending on the software you may need to manually run ``make clean``
-etc. between buildout runs if you make changes to the code. Also, the
+Depending on the software you may need to manually run ``make clean`` etc.
+between buildout runs if you make changes to the code. Also, the
 ``keep-compile-dir`` has no effect when ``path`` is used.
 
 
 Advanced configuration
 ======================
 
-The above options are enough to build most packages. However, in some
-cases it is not enough and we need to control the build process
-more. Let's try again with a new buildout and provide more options.
+The above options are enough to build most packages. However, in some cases it
+is not enough and we need to control the build process more. Let's try again
+with a new buildout and provide more options.
 
     >>> write('buildout.cfg',
     ... """
@@ -335,10 +357,9 @@ more. Let's try again with a new buildout and provide more options.
     ...     patches/Makefile.dist.patch
     ... """ % dict(src=src))
 
-This configuration uses custom configure options, an environment
-section, per-part customization to the environment, custom prefix,
-multiple make targets and also patches the source code before the
-scripts are run.
+This configuration uses custom configure options, an environment section,
+per-part customization to the environment, custom prefix, multiple make
+targets and also patches the source code before the scripts are run.
 
     >>> print system(buildout)
     Uninstalling package.
@@ -358,14 +379,14 @@ scripts are run.
 Customizing the build process
 =============================
 
-Sometimes even the above is not enough and you need to be able to
-control the process in even more detail. One such use case would be to
-perform dynamic substitutions on the source code (possible based on
-information from the buildout) which cannot be done with static
-patches or to simply run arbitrary commands.
+Sometimes even the above is not enough and you need to be able to control the
+process in even more detail. One such use case would be to perform dynamic
+substitutions on the source code (possible based on information from the
+buildout) which cannot be done with static patches or to simply run arbitrary
+commands.
 
-The recipe allows you to write custom python scripts that hook into
-the build process. You can define a script to be run:
+The recipe allows you to write custom python scripts that hook into the build
+process. You can define a script to be run:
 
  - before the configure script is executed (pre-configure-hook)
  - before the make process is executed (pre-make-hook)
@@ -375,13 +396,22 @@ Each option needs to contain the following information
 
   /full/path/to/the/python/module.py:name_of_callable
 
-where the callable object (here name_of_callable) is expected to take
-two parameters, the ``options`` dictionary from the recipe and the
-global ``buildout`` dictionary.
+where the callable object (here name_of_callable) is expected to take three
+parameters:
+
+    1. The ``options`` dictionary from the recipe.
+
+    2. The global ``buildout`` dictionary.
+
+    3. A dictionary containing the current ``os.environ`` augmented with
+       the part specific overrides.
 
-Let's create a simple python script to demonstrate the
-functionality. You can naturally have separate scripts for each hook
-or simply use just one or two hooks. Here we use just a single module.
+These parameters should provide the callable all the necessary information to
+perform any part specific customization to the build process.
+
+Let's create a simple python script to demonstrate the functionality. You can
+naturally have separate modules for each hook or simply use just one or two
+hooks. Here we use just a single module.
 
     >>> hooks = tmpdir('hooks')
     >>> write(hooks, 'customhandlers.py',
@@ -389,15 +419,15 @@ or simply use just one or two hooks. Here we use just a single module.
     ... import logging
     ... log = logging.getLogger('hook')
     ...
-    ... def preconfigure(options, buildout):
+    ... def preconfigure(options, buildout, environment):
     ...     log.info('This is pre-configure-hook!')
-    ...     
-    ... def premake(options, buildout):
+    ...
+    ... def premake(options, buildout, environment):
     ...     log.info('This is pre-make-hook!')
-    ...     
-    ... def postmake(options, buildout):
+    ...
+    ... def postmake(options, buildout, environment):
     ...     log.info('This is post-make-hook!')
-    ...     
+    ...
     ... """)
 
 and a new buildout to try it out
@@ -431,8 +461,8 @@ and a new buildout to try it out
     hook: This is post-make-hook!
 
 For even more specific needs you can write your own recipe that uses
-``hexagonit.recipe.cmmi`` and set the ``keep-compile-dir`` option to
-``true``. You can then continue from where this recipe finished by
-reading the location of the compile directory from
-``options['compile-directory']`` from your own recipe.
+``hexagonit.recipe.cmmi`` and set the ``keep-compile-dir`` option to ``true``.
+You can then continue from where this recipe finished by reading the location
+of the compile directory from ``options['compile-directory']`` from your own
+recipe.
 

hexagonit/recipe/cmmi/__init__.py

@@ -73,8 +73,15 @@ class Recipe(object):
         filename, callable = script.split(':')
         filename = os.path.abspath(filename)
         module = imp.load_source('script', filename)
-        # Run the script with all options
-        getattr(module, callable.strip())(self.options, self.buildout)
+        script = getattr(module, callable.strip())
+
+        try:
+            script(self.options, self.buildout, self.augmented_environment())
+        except TypeError:
+            # BBB: Support hook scripts that do not take the environment as
+            # the third parameter
+            script(self.options, self.buildout)
+
 
     def run(self, cmd):
         """Run the given ``cmd`` in a child process."""

hexagonit/recipe/cmmi/tests.py

@@ -29,6 +29,9 @@ class NonInformativeTests(unittest.TestCase):
 
     def tearDown(self):
         shutil.rmtree(self.dir)
+        for var in os.environ.keys():
+            if var.startswith('HRC_'):
+                del os.environ[var]
 
     def write_file(self, filename, contents, mode=stat.S_IREAD|stat.S_IWUSR):
         path = os.path.join(self.dir, filename)
@@ -151,6 +154,48 @@ class NonInformativeTests(unittest.TestCase):
             '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_call_script__bbb_for_callable_with_two_parameters(self):
+        recipe = self.make_recipe({}, 'test', {
+            'url' : 'file://%s/testdata/package-0.0.0.tar.gz' % os.path.dirname(__file__),
+            })
+
+        # The hook script does not return anything so we (ab)use exceptions
+        # as a mechanism for asserting the function behaviour.
+        filename = os.path.join(self.dir, 'hooks.py')
+        script = open(filename, 'w')
+        script.write('def my_hook(options, buildout): raise ValueError("I got called")\n')
+        script.close()
+
+        try:
+            recipe.call_script('%s:my_hook' % filename)
+            self.fail("The hook script was not called.")
+        except ValueError, e:
+            self.assertEquals(str(e), 'I got called')
+
+    def test_call_script__augmented_environment_as_third_parameter(self):
+        os.environ['HRC_SENTINEL'] = 'sentinel'
+        os.environ['HRC_TESTVAR'] = 'foo'
+
+        recipe = self.make_recipe({}, 'test', {
+            'url' : 'file://%s/testdata/package-0.0.0.tar.gz' % os.path.dirname(__file__),
+            'environment' : 'HRC_TESTVAR=bar'
+            })
+
+        # The hook script does not return anything so we (ab)use exceptions
+        # as a mechanism for asserting the function behaviour.
+        filename = os.path.join(self.dir, 'hooks.py')
+        script = open(filename, 'w')
+        script.write('def my_hook(options, buildout, env): raise ValueError("%(HRC_SENTINEL)s %(HRC_TESTVAR)s" % env)\n')
+        script.close()
+
+        try:
+            recipe.call_script('%s:my_hook' % filename)
+            self.fail("The hook script was not called.")
+        except ValueError, e:
+            self.assertEquals(str(e), 'sentinel bar')
+
+
 def test_suite():
     suite = unittest.TestSuite((
             doctest.DocFileSuite(