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

[Daniel Dunbar]

On Mon, Sep 9, 2013 at 10:24 AM, Chris Matthews <chris.matthews at apple.com>wrote:

> 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)
> +
>

I would just call this isexecfile() and put it into lnt.util.


> +
>  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
>

This function is generic enough I think it should also just go into
lnt.util and avoid creating another "util-like" module.

 - Daniel

+
> +
> 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
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130909/030a4aab/attachment.html>