[LNT][PATCH] Better errors for invalid --cc parameters.

[Chris Matthews]

When you specified something odd for --cc you'd get a really cryptic "Permission Denied" error when we go to check the compiler version. Do better checking on the --cc executable first, and error if it is not 1) executable and 2) a file. Also, make the compile test function use the same compiler resolution procedure, including this error.

diff --git a/lnt/testing/util/compilers.py b/lnt/testing/util/compilers.py
index 6b8f0348c7b21865c576374c38d2d4b13c1b51ec..ffdd061edbf5add2fd7c119c78bb1fb76b6b7df2 100644
--- a/lnt/testing/util/compilers.py
+++ b/lnt/testing/util/compilers.py
@@ -13,6 +13,12 @@ def ishexhash(string):
              for c in string
              if c.isdigit() or c in 'abcdef']) == 40
 
+
+def is_valid(path):
+    """Does this path point to a valid executable?"""
+    return os.path.isfile(path) and os.access(path, os.X_OK)
+
+
 def get_cc_info(path, cc_flags=[]):
     """get_cc_info(path) -> { ... }
 
diff --git a/lnt/tests/common.py b/lnt/tests/common.py
new file mode 100644
index 0000000000000000000000000000000000000000..23a6b11bfb40b645e1ccd2a834c796bdd409483c
--- /dev/null
+++ b/lnt/tests/common.py
@@ -0,0 +1,24 @@
+"""Some functions which are common to test suites."""
+import os
+from lnt.testing.util.commands import capture, mkdir_p
+
+
+def resolve_command_path(name):
+    """Try to make the name/path given into an absolute path to an
+    executable.
+
+    """
+    # If the given name exists (or is a path), make it absolute.
+    if os.path.exists(name):
+        return os.path.abspath(name)
+
+    # Otherwise we most likely have a command name, try to look it up.
+    path = which(name)
+    if path is not None:
+        note("resolved command %r to path %r" % (name, path))
+        return path
+
+    # If that failed just return the original name.
+    return name
+
+
diff --git a/lnt/tests/compile.py b/lnt/tests/compile.py
index 77a235da7fcfb55b558ef246ef71acf5d1531d94..eaa4f9bea34fc986d3875a70fe6eaac34f27ba32 100644
--- a/lnt/tests/compile.py
+++ b/lnt/tests/compile.py
@@ -19,6 +19,7 @@ from lnt.testing.util.commands import note, warning, error, fatal
 from lnt.testing.util.misc import TeeStream, timestamp
 from lnt.testing.util import commands, machineinfo
 from lnt.util import stats
+from lnt.tests.common import resolve_command_path
 
 def args_to_quoted_string(args):
     def quote_arg(arg):
@@ -750,6 +751,13 @@ class CompileTest(builtintest.BuiltinTest):
         if len(args) != 0:
             parser.error("invalid number of arguments")
 
+        # Resolve the cc_under_test path.
+        opts.cc = resolve_command_path(opts.cc)
+
+        if not lnt.testing.util.compilers.is_valid(opts.cc):
+            parser.error('--cc does not point to a valid executable.')
+
+
         # Attempt to infer the cxx compiler if not given.
         if opts.cc and opts.cxx is None:
             opts.cxx = lnt.testing.util.compilers.infer_cxx_compiler(opts.cc)
diff --git a/lnt/tests/nt.py b/lnt/tests/nt.py
index cefb38c42c7c05cc24df76bc263e93cb50b7b09b..fd274e19bb22b27d2e535b2e7653e6ac0069823b 100644
--- a/lnt/tests/nt.py
+++ b/lnt/tests/nt.py
@@ -14,8 +14,9 @@ from datetime import datetime
 import lnt.testing
 import lnt.testing.util.compilers
 
+from lnt.tests.common import resolve_command_path
 from lnt.testing.util.commands import note, warning, error, fatal
-from lnt.testing.util.commands import capture, mkdir_p, which
+from lnt.testing.util.commands import capture, mkdir_p
 from lnt.testing.util.rcs import get_source_version
 from lnt.testing.util.misc import timestamp
 
@@ -387,20 +388,6 @@ class TestConfiguration(object):
         
 ###
 
-def resolve_command_path(name):
-    # If the given name exists (or is a path), make it absolute.
-    if os.path.exists(name):
-        return os.path.abspath(name)
-
-    # Otherwise we most likely have a command name, try to look it up.
-    path = which(name)
-    if path is not None:
-        note("resolved command %r to path %r" % (name, path))
-        return path
-
-    # If that failed just return the original name.
-    return name
-
 def scan_for_test_modules(config):
     base_modules_path = os.path.join(config.test_suite_root, 'LNTBased')
     if config.only_test is None:
@@ -1339,6 +1326,9 @@ class NTTest(builtintest.BuiltinTest):
         # Resolve the cc_under_test path.
         opts.cc_under_test = resolve_command_path(opts.cc_under_test)
 
+        if not lnt.testing.util.compilers.is_valid(opts.cc_under_test):
+            parser.error('--cc does not point to a valid executable.')
+
         # If there was no --cxx given, attempt to infer it from the --cc.
         if opts.cxx_under_test is None:
             opts.cxx_under_test = lnt.testing.util.compilers.infer_cxx_compiler(



Chris Matthews
chris.matthews at apple.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130909/ad29aa2c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch_commit_53d59cb00624.patch
Type: application/octet-stream
Size: 4404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130909/ad29aa2c/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130909/ad29aa2c/attachment-0001.html>