Merge pull request #527 from LLNL/bugfix/github-525

Bugfix/GitHub 525

lib/spack/spack/hooks/sbang.py

@@ -35,7 +35,7 @@
 shebang_limit = 127
 
 def shebang_too_long(path):
-    """Detects whether an file has a shebang line that is too long."""
+    """Detects whether a file has a shebang line that is too long."""
     with open(path, 'r') as script:
         bytes = script.read(2)
         if bytes != '#!':
@@ -47,14 +47,21 @@ def shebang_too_long(path):
 
 def filter_shebang(path):
     """Adds a second shebang line, using sbang, at the beginning of a file."""
+    with open(path, 'r') as original_file:
+        original = original_file.read()
+
+    # This line will be prepended to file
+    new_sbang_line = '#!/bin/bash %s/bin/sbang\n' % spack.spack_root
+
+    # Skip files that are already using sbang.
+    if original.startswith(new_sbang_line):
+        return
+
     backup = path + ".shebang.bak"
     os.rename(path, backup)
 
-    with open(backup, 'r') as bak_file:
-        original = bak_file.read()
-
     with open(path, 'w') as new_file:
-        new_file.write('#!/bin/bash %s/bin/sbang\n' % spack.spack_root)
+        new_file.write(new_sbang_line)
         new_file.write(original)
 
     copy_mode(backup, path)
@@ -63,15 +70,29 @@ def filter_shebang(path):
     tty.warn("Patched overly long shebang in %s" % path)
 
 
+def filter_shebangs_in_directory(directory):
+    for file in os.listdir(directory):
+        path = os.path.join(directory, file)
+
+        # only handle files
+        if not os.path.isfile(path):
+            continue
+
+        # only handle links that resolve within THIS package's prefix.
+        if os.path.islink(path):
+            real_path = os.path.realpath(path)
+            if not real_path.startswith(directory + os.sep):
+                continue
+
+        # test the file for a long shebang, and filter
+        if shebang_too_long(path):
+            filter_shebang(path)
+
+
 def post_install(pkg):
     """This hook edits scripts so that they call /bin/bash
        $spack_prefix/bin/sbang instead of something longer than the
        shebang limit."""
     if not os.path.isdir(pkg.prefix.bin):
         return
-
-    for file in os.listdir(pkg.prefix.bin):
-        path = os.path.join(pkg.prefix.bin, file)
-        if shebang_too_long(path):
-            filter_shebang(path)
-
+    filter_shebangs_in_directory(pkg.prefix.bin)

lib/spack/spack/test/__init__.py

@@ -65,7 +65,8 @@
               'lock',
               'database',
               'namespace_trie',
-              'yaml']
+              'yaml',
+              'sbang']
 
 
 def list_tests():
@@ -87,20 +88,20 @@ def run(names, outputDir, verbose=False):
                           "Valid names are:")
                 colify(sorted(test_names), indent=4)
                 sys.exit(1)
-    
+
     tally = Tally()
     for test in names:
         module = 'spack.test.' + test
         print module
-        
+
         tty.msg("Running test: %s" % test)
-        
+
         runOpts = ["--with-%s" % spack.test.tally_plugin.Tally.name]
-        
+
         if outputDir:
             xmlOutputFname = "unittests-{0}.xml".format(test)
             xmlOutputPath = join_path(outputDir, xmlOutputFname)
-            runOpts += ["--with-xunit", 
+            runOpts += ["--with-xunit",
                 "--xunit-file={0}".format(xmlOutputPath)]
         argv = [""] + runOpts + [module]
         result = nose.run(argv=argv, addplugins=[tally])

lib/spack/spack/test/sbang.py

@@ -0,0 +1,93 @@
+##############################################################################
+# 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://github.com/llnl/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
+##############################################################################
+"""\
+Test that Spack's shebang filtering works correctly.
+"""
+import os
+import unittest
+import tempfile
+import shutil
+
+from llnl.util.filesystem import *
+from spack.hooks.sbang import filter_shebangs_in_directory
+import spack
+
+short_line = "#!/this/is/short/bin/bash\n"
+long_line  = "#!/this/" + ('x' * 200) + "/is/long\n"
+sbang_line = '#!/bin/bash %s/bin/sbang\n' % spack.spack_root
+last_line = "last!\n"
+
+class SbangTest(unittest.TestCase):
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+
+        # make sure we can ignore non-files
+        directory = os.path.join(self.tempdir, 'dir')
+        mkdirp(directory)
+
+        # Script with short shebang
+        self.short_shebang = os.path.join(self.tempdir, 'short')
+        with open(self.short_shebang, 'w') as f:
+            f.write(short_line)
+            f.write(last_line)
+
+        # Script with long shebang
+        self.long_shebang = os.path.join(self.tempdir, 'long')
+        with open(self.long_shebang, 'w') as f:
+            f.write(long_line)
+            f.write(last_line)
+
+        # Script already using sbang.
+        self.has_shebang = os.path.join(self.tempdir, 'shebang')
+        with open(self.has_shebang, 'w') as f:
+            f.write(sbang_line)
+            f.write(long_line)
+            f.write(last_line)
+
+
+    def tearDown(self):
+         shutil.rmtree(self.tempdir, ignore_errors=True)
+
+
+
+    def test_shebang_handling(self):
+        filter_shebangs_in_directory(self.tempdir)
+
+        # Make sure this is untouched
+        with open(self.short_shebang, 'r') as f:
+            self.assertEqual(f.readline(), short_line)
+            self.assertEqual(f.readline(), last_line)
+
+        # Make sure this got patched.
+        with open(self.long_shebang, 'r') as f:
+            self.assertEqual(f.readline(), sbang_line)
+            self.assertEqual(f.readline(), long_line)
+            self.assertEqual(f.readline(), last_line)
+
+        # Make sure this is untouched
+        with open(self.has_shebang, 'r') as f:
+            self.assertEqual(f.readline(), sbang_line)
+            self.assertEqual(f.readline(), long_line)
+            self.assertEqual(f.readline(), last_line)