AutotoolsPackage: minor improvements (#2859)

* AutotoolsPackage: added configure_directory to permit build out of source. The configure script executable is now invoked with an absolute path. Modified a few packages accordingly.

* build_systems: functions returning directories are now properties

* build_systems: fixed issues with tcl and tk

* AutotoolsPackage: reworked recipe for autoreconf

4 files changed, 95 insertions, 25 deletions

diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py
index d08ea02428..af6f5507b2 100644
--- a/lib/spack/spack/build_systems/autotools.py
+++ b/lib/spack/spack/build_systems/autotools.py
@@ -30,8 +30,10 @@ import shutil
 from subprocess import PIPE
 from subprocess import check_call
 
-from llnl.util.filesystem import working_dir
-from spack.package import PackageBase, run_after
+import llnl.util.tty as tty
+from llnl.util.filesystem import working_dir, join_path, force_remove
+from spack.package import PackageBase, run_after, run_before
+from spack.util.executable import Executable
 
 
 class AutotoolsPackage(PackageBase):
@@ -79,8 +81,14 @@ class AutotoolsPackage(PackageBase):
     #: phase
     install_targets = ['install']
 
+    #: Callback names for build-time test
     build_time_test_callbacks = ['check']
 
+    #: Set to true to force the autoreconf step even if configure is present
+    force_autoreconf = False
+    #: Options to be passed to autoreconf when using the default implementation
+    autoreconf_extra_args = []
+
     def _do_patch_config_guess(self):
         """Some packages ship with an older config.guess and need to have
         this updated when installed on a newer architecture."""
@@ -147,9 +155,26 @@ class AutotoolsPackage(PackageBase):
 
         return False
 
+    @property
+    def configure_directory(self):
+        """Returns the directory where 'configure' resides.
+
+        :return: directory where to find configure
+        """
+        return self.stage.source_path
+
+    @property
+    def configure_abs_path(self):
+        # Absolute path to configure
+        configure_abs_path = join_path(
+            os.path.abspath(self.configure_directory), 'configure'
+        )
+        return configure_abs_path
+
+    @property
     def build_directory(self):
         """Override to provide another place to build the package"""
-        return self.stage.source_path
+        return self.configure_directory
 
     def patch(self):
         """Patches config.guess if
@@ -165,21 +190,62 @@ class AutotoolsPackage(PackageBase):
             if not self._do_patch_config_guess():
                 raise RuntimeError('Failed to find suitable config.guess')
 
+    @run_before('autoreconf')
+    def delete_configure_to_force_update(self):
+        if self.force_autoreconf:
+            force_remove(self.configure_abs_path)
+
     def autoreconf(self, spec, prefix):
         """Not needed usually, configure should be already there"""
-        pass
+        # If configure exists nothing needs to be done
+        if os.path.exists(self.configure_abs_path):
+            return
+        # Else try to regenerate it
+        autotools = ['m4', 'autoconf', 'automake', 'libtool']
+        missing = [x for x in autotools if x not in spec]
+        if missing:
+            msg = 'Cannot generate configure: missing dependencies {0}'
+            raise RuntimeError(msg.format(missing))
+        tty.msg('Configure script not found: trying to generate it')
+        tty.warn('*********************************************************')
+        tty.warn('* If the default procedure fails, consider implementing *')
+        tty.warn('*        a custom AUTORECONF phase in the package       *')
+        tty.warn('*********************************************************')
+        with working_dir(self.configure_directory):
+            m = inspect.getmodule(self)
+            # This part should be redundant in principle, but
+            # won't hurt
+            m.libtoolize()
+            m.aclocal()
+            # This line is what is needed most of the time
+            # --install, --verbose, --force
+            autoreconf_args = ['-ivf']
+            if 'pkg-config' in spec:
+                autoreconf_args += [
+                    '-I',
+                    join_path(spec['pkg-config'].prefix, 'share', 'aclocal'),
+                ]
+            autoreconf_args += self.autoreconf_extra_args
+            m.autoreconf(*autoreconf_args)
 
     @run_after('autoreconf')
-    def is_configure_or_die(self):
-        """Checks the presence of a `configure` file after the
-        :py:meth:`.autoreconf` phase.
+    def set_configure_or_die(self):
+        """Checks the presence of a ``configure`` file after the
+        autoreconf phase. If it is found sets a module attribute
+        appropriately, otherwise raises an error.
 
-        :raise RuntimeError: if the ``configure`` script does not exist.
+        :raises RuntimeError: if a configure script is not found in
+            :py:meth:`~.configure_directory`
         """
-        with working_dir(self.build_directory()):
-            if not os.path.exists('configure'):
-                raise RuntimeError(
-                    'configure script not found in {0}'.format(os.getcwd()))
+        # Check if a configure script is there. If not raise a RuntimeError.
+        if not os.path.exists(self.configure_abs_path):
+            msg = 'configure script not found in {0}'
+            raise RuntimeError(msg.format(self.configure_directory))
+
+        # Monkey-patch the configure script in the corresponding module
+        inspect.getmodule(self).configure = Executable(
+            self.configure_abs_path
+        )
 
     def configure_args(self):
         """Produces a list containing all the arguments that must be passed to
@@ -195,21 +261,21 @@ class AutotoolsPackage(PackageBase):
         """
         options = ['--prefix={0}'.format(prefix)] + self.configure_args()
 
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory, create=True):
             inspect.getmodule(self).configure(*options)
 
     def build(self, spec, prefix):
         """Makes the build targets specified by
         :py:attr:``~.AutotoolsPackage.build_targets``
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.build_targets)
 
     def install(self, spec, prefix):
         """Makes the install targets specified by
         :py:attr:``~.AutotoolsPackage.install_targets``
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.install_targets)
 
     run_after('build')(PackageBase._run_default_build_time_test_callbacks)
@@ -218,7 +284,7 @@ class AutotoolsPackage(PackageBase):
         """Searches the Makefile for targets ``test`` and ``check``
         and runs them if found.
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             self._if_make_target_execute('test')
             self._if_make_target_execute('check')
 
diff --git a/lib/spack/spack/build_systems/cmake.py b/lib/spack/spack/build_systems/cmake.py
index 823ef502c4..43d177d3cb 100644
--- a/lib/spack/spack/build_systems/cmake.py
+++ b/lib/spack/spack/build_systems/cmake.py
@@ -82,6 +82,7 @@ class CMakePackage(PackageBase):
         """
         return 'RelWithDebInfo'
 
+    @property
     def root_cmakelists_dir(self):
         """Returns the location of the root CMakeLists.txt
 
@@ -119,6 +120,7 @@ class CMakePackage(PackageBase):
         args.append('-DCMAKE_INSTALL_RPATH:STRING={0}'.format(rpaths))
         return args
 
+    @property
     def build_directory(self):
         """Returns the directory to use when building the package
 
@@ -141,19 +143,19 @@ class CMakePackage(PackageBase):
 
     def cmake(self, spec, prefix):
         """Runs ``cmake`` in the build directory"""
-        options = [self.root_cmakelists_dir()] + self.std_cmake_args + \
+        options = [self.root_cmakelists_dir] + self.std_cmake_args + \
             self.cmake_args()
-        with working_dir(self.build_directory(), create=True):
+        with working_dir(self.build_directory, create=True):
             inspect.getmodule(self).cmake(*options)
 
     def build(self, spec, prefix):
         """Make the build targets"""
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.build_targets)
 
     def install(self, spec, prefix):
         """Make the install targets"""
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.install_targets)
 
     run_after('build')(PackageBase._run_default_build_time_test_callbacks)
@@ -162,7 +164,7 @@ class CMakePackage(PackageBase):
         """Searches the CMake-generated Makefile for the target ``test``
         and runs it if found.
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             self._if_make_target_execute('test')
 
     # Check that self.prefix is there after installation
diff --git a/lib/spack/spack/build_systems/makefile.py b/lib/spack/spack/build_systems/makefile.py
index f07bcd62ab..7274384478 100644
--- a/lib/spack/spack/build_systems/makefile.py
+++ b/lib/spack/spack/build_systems/makefile.py
@@ -72,6 +72,7 @@ class MakefilePackage(PackageBase):
     #: phase
     install_targets = ['install']
 
+    @property
     def build_directory(self):
         """Returns the directory containing the main Makefile
 
@@ -89,14 +90,14 @@ class MakefilePackage(PackageBase):
         """Calls make, passing :py:attr:`~.MakefilePackage.build_targets`
         as targets.
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.build_targets)
 
     def install(self, spec, prefix):
         """Calls make, passing :py:attr:`~.MakefilePackage.install_targets`
         as targets.
         """
-        with working_dir(self.build_directory()):
+        with working_dir(self.build_directory):
             inspect.getmodule(self).make(*self.install_targets)
 
     # Check that self.prefix is there after installation
diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index 5e7c1c356d..60a850d356 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -97,10 +97,11 @@ class PythonPackage(PackageBase):
 
     extends('python')
 
-    def setup_file(self, spec, prefix):
+    def setup_file(self):
         """Returns the name of the setup file to use."""
         return 'setup.py'
 
+    @property
     def build_directory(self):
         """The directory containing the ``setup.py`` file."""
         return self.stage.source_path
@@ -109,7 +110,7 @@ class PythonPackage(PackageBase):
         inspect.getmodule(self).python(*args)
 
     def setup_py(self, *args):
-        setup = self.setup_file(self.spec, self.prefix)
+        setup = self.setup_file()
 
         with working_dir(self.build_directory()):
             self.python(setup, '--no-user-cfg', *args)