Fix #46: make(parallel=False) regression.

- Added some tests to make sure this stays in place.

4 files changed, 133 insertions, 6 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 2e72f3c787..f9e795ac42 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -73,14 +73,14 @@ class MakeExecutable(Executable):
         self.jobs = jobs
 
     def __call__(self, *args, **kwargs):
-        parallel = kwargs.get('parallel', self.jobs > 1)
-        disable_parallel = env_flag(SPACK_NO_PARALLEL_MAKE)
+        disable = env_flag(SPACK_NO_PARALLEL_MAKE)
+        parallel = not disable and kwargs.get('parallel', self.jobs > 1)
 
-        if self.jobs > 1 and not disable_parallel:
+        if parallel:
             jobs = "-j%d" % self.jobs
             args = (jobs,) + args
 
-        super(MakeExecutable, self).__call__(*args, **kwargs)
+        return super(MakeExecutable, self).__call__(*args, **kwargs)
 
 
 def set_compiler_environment_variables(pkg):
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 7ff512c370..8d4b0c6cde 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -54,7 +54,8 @@ test_names = ['versions',
               'cc',
               'link_tree',
               'spec_yaml',
-              'optional_deps']
+              'optional_deps',
+              'make_executable']
 
 
 def list_tests():
diff --git a/lib/spack/spack/test/make_executable.py b/lib/spack/spack/test/make_executable.py
new file mode 100644
index 0000000000..c4bfeb2a03
--- /dev/null
+++ b/lib/spack/spack/test/make_executable.py
@@ -0,0 +1,125 @@
+##############################################################################
+# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC.
+# Produced at the Lawrence Livermore National Laboratory.
+#
+# This file is part of Spack.
+# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved.
+# LLNL-CODE-647188
+#
+# For details, see https://scalability-llnl.github.io/spack
+# Please also see the LICENSE file for our notice and the LGPL.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License (as published by
+# the Free Software Foundation) version 2.1 dated February 1999.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and
+# conditions of the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+##############################################################################
+"""
+Tests for Spack's built-in parallel make support.
+
+This just tests whether the right args are getting passed to make.
+"""
+import os
+import unittest
+import tempfile
+import shutil
+
+from llnl.util.filesystem import *
+from spack.util.environment import path_put_first
+from spack.build_environment import MakeExecutable
+
+
+class MakeExecutableTest(unittest.TestCase):
+    def setUp(self):
+        self.tmpdir = tempfile.mkdtemp()
+
+        make_exe = join_path(self.tmpdir, 'make')
+        with open(make_exe, 'w') as f:
+            f.write('#!/bin/sh\n')
+            f.write('echo "$@"')
+        os.chmod(make_exe, 0700)
+
+        path_put_first('PATH', [self.tmpdir])
+
+
+    def tearDown(self):
+        shutil.rmtree(self.tmpdir)
+
+
+    def test_make_normal(self):
+        make = MakeExecutable('make', 8)
+        self.assertEqual(make(return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
+
+
+    def test_make_explicit(self):
+        make = MakeExecutable('make', 8)
+        self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
+
+
+    def test_make_one_job(self):
+        make = MakeExecutable('make', 1)
+        self.assertEqual(make(return_output=True).strip(), '')
+        self.assertEqual(make('install', return_output=True).strip(), 'install')
+
+
+    def test_make_parallel_false(self):
+        make = MakeExecutable('make', 8)
+        self.assertEqual(make(parallel=False, return_output=True).strip(), '')
+        self.assertEqual(make('install', parallel=False, return_output=True).strip(), 'install')
+
+
+    def test_make_parallel_disabled(self):
+        make = MakeExecutable('make', 8)
+
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'true'
+        self.assertEqual(make(return_output=True).strip(), '')
+        self.assertEqual(make('install', return_output=True).strip(), 'install')
+
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = '1'
+        self.assertEqual(make(return_output=True).strip(), '')
+        self.assertEqual(make('install', return_output=True).strip(), 'install')
+
+        # These don't disable (false and random string)
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'false'
+        self.assertEqual(make(return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
+
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'foobar'
+        self.assertEqual(make(return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
+
+        del os.environ['SPACK_NO_PARALLEL_MAKE']
+
+
+    def test_make_parallel_precedence(self):
+        make = MakeExecutable('make', 8)
+
+        # These should work
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'true'
+        self.assertEqual(make(parallel=True, return_output=True).strip(), '')
+        self.assertEqual(make('install', parallel=True, return_output=True).strip(), 'install')
+
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = '1'
+        self.assertEqual(make(parallel=True, return_output=True).strip(), '')
+        self.assertEqual(make('install', parallel=True, return_output=True).strip(), 'install')
+
+        # These don't disable (false and random string)
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'false'
+        self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
+
+        os.environ['SPACK_NO_PARALLEL_MAKE'] = 'foobar'
+        self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
+        self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
+
+        del os.environ['SPACK_NO_PARALLEL_MAKE']
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index afdf51c707..7a4ff919ad 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -35,7 +35,8 @@ def get_path(name):
 
 def env_flag(name):
     if name in os.environ:
-        return os.environ[name].lower() == "true"
+        value = os.environ[name].lower()
+        return value == "true" or value == "1"
     return False