From 16c25886358ca8e89d2cad252be26677c9137e27 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 31 May 2015 13:03:30 -0700 Subject: Fix #46: make(parallel=False) regression. - Added some tests to make sure this stays in place. --- lib/spack/spack/build_environment.py | 8 +- lib/spack/spack/test/__init__.py | 3 +- lib/spack/spack/test/make_executable.py | 125 ++++++++++++++++++++++++++++++++ lib/spack/spack/util/environment.py | 3 +- 4 files changed, 133 insertions(+), 6 deletions(-) create mode 100644 lib/spack/spack/test/make_executable.py 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 -- cgit v1.2.3-70-g09d2