Update Package: Metis (#1783)

* Fixed a bug that was causing post-install METIS tests to fail.

* Improved the patching procedure used in the 'metis' install script.

* Enabled patch skipping for the 'metis' and 'parmetis' packages.

* Fixed some minor style issues in the 'parmetis' package.

* Improved the 'metis' test fix and added 'run_tests' support.

2 files changed, 91 insertions, 84 deletions

diff --git a/var/spack/repos/builtin/packages/metis/package.py b/var/spack/repos/builtin/packages/metis/package.py
index 9f8ed5c9e8..ca48a47817 100644
--- a/var/spack/repos/builtin/packages/metis/package.py
+++ b/var/spack/repos/builtin/packages/metis/package.py
@@ -43,16 +43,12 @@ class Metis(Package):
     version('5.0.2', 'acb521a4e8c2e6dd559a7f9abd0468c5')
     version('4.0.3', 'd3848b454532ef18dc83e4fb160d1e10')
 
-    variant('shared', default=True,
-            description='Enables the build of shared libraries')
-    variant('debug', default=False,
-            description='Builds the library in debug mode')
-    variant('gdb', default=False, description='Enables gdb support')
+    variant('shared', default=True, description='Enables the build of shared libraries.')
+    variant('debug', default=False, description='Builds the library in debug mode.')
+    variant('gdb', default=False, description='Enables gdb support.')
 
-    variant('idx64', default=False,
-            description='Use int64_t as default index type')
-    variant('real64', default=False,
-            description='Use double precision floating point types')
+    variant('idx64', default=False, description='Sets the bit width of METIS\'s index type to 64.')
+    variant('real64', default=False, description='Sets the bit width of METIS\'s real type to 64.')
 
     depends_on('cmake@2.8:', when='@5:', type='build')
 
@@ -63,12 +59,37 @@ class Metis(Package):
         return '%s/%smetis-%s.tar.gz' % (Metis.base_url, verdir, version)
 
     @when('@:4')
+    def patch(self):
+        pass
+
+    @when('@5:')
+    def patch(self):
+        source_path = self.stage.source_path
+        metis_header = FileFilter(join_path(source_path, 'include', 'metis.h'))
+
+        metis_header.filter(
+            r'(\b)(IDXTYPEWIDTH )(\d+)(\b)',
+            r'\1\2{0}\4'.format('64' if '+idx64' in self.spec else '32'),
+        )
+        metis_header.filter(
+            r'(\b)(REALTYPEWIDTH )(\d+)(\b)',
+            r'\1\2{0}\4'.format('64' if '+real64' in self.spec else '32'),
+        )
+
+        # Make clang 7.3 happy.
+        # Prevents "ld: section __DATA/__thread_bss extends beyond end of file"
+        # See upstream LLVM issue https://llvm.org/bugs/show_bug.cgi?id=27059
+        # and https://github.com/Homebrew/homebrew-science/blob/master/metis.rb
+        if self.spec.satisfies('%clang@7.3.0'):
+            filter_file('#define MAX_JBUFS 128', '#define MAX_JBUFS 24',
+                        join_path(source_path, 'GKlib', 'error.c'))
+
+    @when('@:4')
     def install(self, spec, prefix):
         # Process library spec and options
-        unsupp_vars = [v for v in ('+gdb', '+idx64', '+real64') if v in spec]
-        if unsupp_vars:
-            msg = 'Given variants %s are unsupported by METIS 4!' % unsupp_vars
-            raise InstallError(msg)
+        if any('+{0}'.format(v) in spec for v in ['gdb', 'idx64', 'real64']):
+            raise InstallError('METIS@:4 does not support the following '
+                               'variants: gdb, idx64, real64.')
 
         options = ['COPTIONS=-fPIC']
         if '+debug' in spec:
@@ -118,49 +139,48 @@ class Metis(Package):
                  join_path('Programs', 'io.o'), join_path('Test', 'mtest.c'),
                  '-o', '%s/mtest' % prefix.bin, '-lmetis', '-lm')
 
-        test_bin = lambda testname: join_path(prefix.bin, testname)
-        test_graph = lambda graphname: join_path(prefix.share, graphname)
-
-        graph = test_graph('4elt.graph')
-        os.system('%s %s' % (test_bin('mtest'), graph))
-        os.system('%s %s 40' % (test_bin('kmetis'), graph))
-        os.system('%s %s' % (test_bin('onmetis'), graph))
-        graph = test_graph('test.mgraph')
-        os.system('%s %s 2' % (test_bin('pmetis'), graph))
-        os.system('%s %s 2' % (test_bin('kmetis'), graph))
-        os.system('%s %s 5' % (test_bin('kmetis'), graph))
-        graph = test_graph('metis.mesh')
-        os.system('%s %s 10' % (test_bin('partnmesh'), graph))
-        os.system('%s %s 10' % (test_bin('partdmesh'), graph))
-        os.system('%s %s' % (test_bin('mesh2dual'), graph))
-
-        # FIXME: The following code should replace the testing code in the
-        # block above since it causes installs to fail when one or more of the
-        # Metis tests fail, but it currently doesn't work because the 'mtest',
-        # 'onmetis', and 'partnmesh' tests return error codes that trigger
-        # false positives for failure.
-        """
-        Executable(test_bin('mtest'))(test_graph('4elt.graph'))
-        Executable(test_bin('kmetis'))(test_graph('4elt.graph'), '40')
-        Executable(test_bin('onmetis'))(test_graph('4elt.graph'))
-
-        Executable(test_bin('pmetis'))(test_graph('test.mgraph'), '2')
-        Executable(test_bin('kmetis'))(test_graph('test.mgraph'), '2')
-        Executable(test_bin('kmetis'))(test_graph('test.mgraph'), '5')
-
-        Executable(test_bin('partnmesh'))(test_graph('metis.mesh'), '10')
-        Executable(test_bin('partdmesh'))(test_graph('metis.mesh'), '10')
-        Executable(test_bin('mesh2dual'))(test_graph('metis.mesh'))
-        """
+        if self.run_tests:
+            test_bin = lambda testname: join_path(prefix.bin, testname)
+            test_graph = lambda graphname: join_path(prefix.share, graphname)
+
+            graph = test_graph('4elt.graph')
+            os.system('%s %s' % (test_bin('mtest'), graph))
+            os.system('%s %s 40' % (test_bin('kmetis'), graph))
+            os.system('%s %s' % (test_bin('onmetis'), graph))
+            graph = test_graph('test.mgraph')
+            os.system('%s %s 2' % (test_bin('pmetis'), graph))
+            os.system('%s %s 2' % (test_bin('kmetis'), graph))
+            os.system('%s %s 5' % (test_bin('kmetis'), graph))
+            graph = test_graph('metis.mesh')
+            os.system('%s %s 10' % (test_bin('partnmesh'), graph))
+            os.system('%s %s 10' % (test_bin('partdmesh'), graph))
+            os.system('%s %s' % (test_bin('mesh2dual'), graph))
+
+            # FIXME: The following code should replace the testing code in the
+            # block above since it causes installs to fail when one or more of
+            # the Metis tests fail, but it currently doesn't work because the
+            # 'mtest', 'onmetis', and 'partnmesh' tests return error codes that
+            # trigger false positives for failure.
+            """
+            Executable(test_bin('mtest'))(test_graph('4elt.graph'))
+            Executable(test_bin('kmetis'))(test_graph('4elt.graph'), '40')
+            Executable(test_bin('onmetis'))(test_graph('4elt.graph'))
+
+            Executable(test_bin('pmetis'))(test_graph('test.mgraph'), '2')
+            Executable(test_bin('kmetis'))(test_graph('test.mgraph'), '2')
+            Executable(test_bin('kmetis'))(test_graph('test.mgraph'), '5')
+
+            Executable(test_bin('partnmesh'))(test_graph('metis.mesh'), '10')
+            Executable(test_bin('partdmesh'))(test_graph('metis.mesh'), '10')
+            Executable(test_bin('mesh2dual'))(test_graph('metis.mesh'))
+            """
 
     @when('@5:')
     def install(self, spec, prefix):
-        options = []
-        options.extend(std_cmake_args)
-
-        build_directory = join_path(self.stage.path, 'spack-build')
         source_directory = self.stage.source_path
+        build_directory = join_path(source_directory, 'build')
 
+        options = std_cmake_args[:]
         options.append('-DGKLIB_PATH:PATH=%s/GKlib' % source_directory)
         options.append('-DCMAKE_INSTALL_NAME_DIR:PATH=%s/lib' % prefix)
 
@@ -172,26 +192,24 @@ class Metis(Package):
         if '+gdb' in spec:
             options.append('-DGDB:BOOL=ON')
 
-        metis_header = join_path(source_directory, 'include', 'metis.h')
-        if '+idx64' in spec:
-            filter_file('IDXTYPEWIDTH 32', 'IDXTYPEWIDTH 64', metis_header)
-        if '+real64' in spec:
-            filter_file('REALTYPEWIDTH 32', 'REALTYPEWIDTH 64', metis_header)
-
-        # Make clang 7.3 happy.
-        # Prevents "ld: section __DATA/__thread_bss extends beyond end of file"
-        # See upstream LLVM issue https://llvm.org/bugs/show_bug.cgi?id=27059
-        # and https://github.com/Homebrew/homebrew-science/blob/master/metis.rb
-        if spec.satisfies('%clang@7.3.0'):
-            filter_file('#define MAX_JBUFS 128', '#define MAX_JBUFS 24',
-                        join_path(source_directory, 'GKlib', 'error.c'))
-
         with working_dir(build_directory, create=True):
             cmake(source_directory, *options)
             make()
             make('install')
 
-            # now run some tests:
+            # install GKlib headers, which will be needed for ParMETIS
+            GKlib_dist = join_path(prefix.include, 'GKlib')
+            mkdirp(GKlib_dist)
+            hfiles = glob.glob(join_path(source_directory, 'GKlib', '*.h'))
+            for hfile in hfiles:
+                install(hfile, GKlib_dist)
+
+        if self.run_tests:
+            # FIXME: On some systems, the installed binaries for METIS cannot
+            # be executed without first being read.
+            ls = which('ls')
+            ls('-a', '-l', prefix.bin)
+
             for f in ['4elt', 'copter2', 'mdual']:
                 graph = join_path(source_directory, 'graphs', '%s.graph' % f)
                 Executable(join_path(prefix.bin, 'graphchk'))(graph)
@@ -202,10 +220,3 @@ class Metis(Package):
             Executable(join_path(prefix.bin, 'gpmetis'))(graph, '2')
             graph = join_path(source_directory, 'graphs', 'metis.mesh')
             Executable(join_path(prefix.bin, 'mpmetis'))(graph, '2')
-
-            # install GKlib headers, which will be needed for ParMETIS
-            GKlib_dist = join_path(prefix.include, 'GKlib')
-            mkdirp(GKlib_dist)
-            hfiles = glob.glob(join_path(source_directory, 'GKlib', '*.h'))
-            for hfile in hfiles:
-                install(hfile, GKlib_dist)
diff --git a/var/spack/repos/builtin/packages/parmetis/package.py b/var/spack/repos/builtin/packages/parmetis/package.py
index 8afae91af1..74e00cc9e0 100644
--- a/var/spack/repos/builtin/packages/parmetis/package.py
+++ b/var/spack/repos/builtin/packages/parmetis/package.py
@@ -38,11 +38,9 @@ class Parmetis(Package):
     version('4.0.3', 'f69c479586bf6bb7aff6a9bc0c739628')
     version('4.0.2', '0912a953da5bb9b5e5e10542298ffdce')
 
-    variant('shared', default=True,
-            description='Enables the build of shared libraries')
-    variant('debug', default=False,
-            description='Builds the library in debug mode')
-    variant('gdb', default=False, description='Enables gdb support')
+    variant('shared', default=True, description='Enables the build of shared libraries.')
+    variant('debug', default=False, description='Builds the library in debug mode.')
+    variant('gdb', default=False, description='Enables gdb support.')
 
     depends_on('cmake@2.8:', type='build')
     depends_on('mpi')
@@ -50,9 +48,9 @@ class Parmetis(Package):
 
     patch('enable_external_metis.patch')
     # bug fixes from PETSc developers
-    # https://bitbucket.org/petsc/pkg-parmetis/commits/1c1a9fd0f408dc4d42c57f5c3ee6ace411eb222b/raw/
+    # https://bitbucket.org/petsc/pkg-parmetis/commits/1c1a9fd0f408dc4d42c57f5c3ee6ace411eb222b/raw/  # NOQA: E501
     patch('pkg-parmetis-1c1a9fd0f408dc4d42c57f5c3ee6ace411eb222b.patch')
-    # https://bitbucket.org/petsc/pkg-parmetis/commits/82409d68aa1d6cbc70740d0f35024aae17f7d5cb/raw/
+    # https://bitbucket.org/petsc/pkg-parmetis/commits/82409d68aa1d6cbc70740d0f35024aae17f7d5cb/raw/  # NOQA: E501
     patch('pkg-parmetis-82409d68aa1d6cbc70740d0f35024aae17f7d5cb.patch')
 
     def url_for_version(self, version):
@@ -60,12 +58,10 @@ class Parmetis(Package):
         return '%s/%sparmetis-%s.tar.gz' % (Parmetis.base_url, verdir, version)
 
     def install(self, spec, prefix):
-        options = []
-        options.extend(std_cmake_args)
-
-        build_directory = join_path(self.stage.path, 'spack-build')
         source_directory = self.stage.source_path
+        build_directory = join_path(source_directory, 'build')
 
+        options = std_cmake_args[:]
         options.extend([
             '-DGKLIB_PATH:PATH=%s/GKlib' % spec['metis'].prefix.include,
             '-DMETIS_PATH:PATH=%s' % spec['metis'].prefix,