[LNT] Get cc info from CMake cache instead of from command-line

[kristof.beyls]

Summary

There are multiple ways to specify the compiler to use, such as:

• Using the --cc command line option.
• Using the --cmake-cache command line option.
• Using a --cmake-define=CMAKE_C_COMPILER=xxx command line option.
• ...

Nonetheless, in the report.json file, the compiler pointed to by the
--cc command line option is reported, irrespective of the compiler
actually used by cmake during compilation.

This patch fixes that by getting the compiler used from the
CMakeCache.txt file rather than from the --cc command line option.

[MatzeB]

LGTM, comments below

lnt/tests/test_suite.py

588 ↗

(On Diff #87984)

Wouldn't it make more sense to keep check_call and add check_output as an additional function? The check_output() write() idiom won't output anything until the command is finished and will probably show all of stderr before stdout instead of intermixing them.

645–647 ↗

(On Diff #87984)

It feels a bit odd that the 2nd loop is nearly a duplication of the first one, just that it isn't... How about:

• Adding default=[] for the cmake_defines add_option() call.
• Merging the two loops: for item in self.opts.cmake_defines + extra_cmake_defs: ...

[cmatthews]

LGTM. Thanks Kristof!

lnt/tests/test_suite.py

583 ↗

(On Diff #87984)

I am not a big fan of using check_output for anything but the simplest uses. In this case we can't do anything with the output until the command finishes running, and the stderr is not interleaved with the output. And when it fails we give the user a nice exception as an error message.

Our ci scripts in zorg have a much nicer implementation, they print a shell like version of the command that you can paste in your own terminal, stream all the outputs and give real error messages. I will probably try and put them in here some day. Until then, you have not made this worse :)

800 ↗

(On Diff #87984)

Would match work here? I think these all end up on the start of line.

[kristof.beyls]

Thanks for all the feedback! I've adapted the patch accordingly and committed in r295039.

lnt/trunk/lnt/tests/test_suite.py

def run_test(self, name, args):
         group = OptionGroup(parser, "Inputs")
         group.add_option("", "--test-suite", dest="test_suite_root",
                          type=str, metavar="PATH", default=None,
                          help="Path to the LLVM test-suite sources")
         group.add_option("", "--test-externals", dest="test_suite_externals",
                          type=str, metavar="PATH",
                          help="Path to the LLVM test-suite externals")
         group.add_option("", "--cmake-define", dest="cmake_defines",
-                         action="append",
+                         action="append", default=[],
                          help=("Defines to pass to cmake. These do not require the "
                                "-D prefix and can be given multiple times. e.g.: "
                                "--cmake-define A=B => -DA=B"))
         group.add_option("-C", "--cmake-cache", dest="cmake_cache",
                          action="append", default=[],
                          help=("Use one of the test-suite's cmake configurations."
                                " Ex: Release, Debug"))
         parser.add_option_group(group)
def _test_threads(self):
         return self.opts.threads
 
     def _check_call(self, *args, **kwargs):
         note('Execute: %s' % ' '.join(args[0]))
         if 'cwd' in kwargs:
             note('          (In %s)' % kwargs['cwd'])
         return subprocess.check_call(*args, **kwargs)
 
+    def _check_output(self, *args, **kwargs):
+        note('Execute: %s' % ' '.join(args[0]))
+        if 'cwd' in kwargs:
+            note('          (In %s)' % kwargs['cwd'])
+        output = subprocess.check_output(*args, **kwargs)
+        sys.stdout.write(output)
+        return output
+
     def _clean(self, path):
         make_cmd = self.opts.make
 
         subdir = path
         if self.opts.only_test:
             components = [path] + [self.opts.only_test[0]]
             subdir = os.path.join(*components)
 
def _configure(self, path, extra_cmake_defs=[], execute=True):
         if self.opts.test_suite_externals:
             defs['TEST_SUITE_EXTERNALS_DIR'] = self.opts.test_suite_externals
         if self.opts.pgo and self.trained:
             defs['TEST_SUITE_PROFILE_USE'] = "On"
             defs['TEST_SUITE_PROFILE_GENERATE'] = "Off"
             if 'TEST_SUITE_RUN_TYPE' not in defs:
                 defs['TEST_SUITE_RUN_TYPE'] = 'ref'
 
-        if self.opts.cmake_defines:
-            for item in self.opts.cmake_defines:
-                k, v = item.split('=', 1)
-                defs[k] = v
-        for item in extra_cmake_defs:
+        for item in self.opts.cmake_defines + extra_cmake_defs:
             k, v = item.split('=', 1)
+            # make sure the overriding of the settings above also works
+            # when the cmake-define-defined variable has a datatype
+            # specified.
+            key_no_datatype = k.split(':', 1)[0]
+            if key_no_datatype in defs:
+                del defs[key_no_datatype]
             defs[k] = v
 
+        # We use 'cmake -LAH -N' later to find out the value of the
+        # CMAKE_C_COMPILER and CMAKE_CXX_COMPILER variables.
+        # 'cmake -LAH -N' will only return variables in the cache that have
+        # a cmake type set. Therefore, explicitly set a 'FILEPATH' type on
+        # these variables here, if they were untyped so far.
+        if 'CMAKE_C_COMPILER' in defs:
+            defs['CMAKE_C_COMPILER:FILEPATH'] = defs['CMAKE_C_COMPILER']
+            del defs['CMAKE_C_COMPILER']
+        if 'CMAKE_CXX_COMPILER' in defs:
+            defs['CMAKE_CXX_COMPILER:FILEPATH'] = defs['CMAKE_CXX_COMPILER']
+            del defs['CMAKE_CXX_COMPILER']
+
         lines = ['Configuring with {']
         for k, v in sorted(defs.items()):
             lines.append("  %s: '%s'" % (k, v))
         lines.append('}')
 
         # Prepare cmake cache if requested:
         cmake_flags = []
         for cache in self.opts.cmake_cache:
def _get_lnt_code(self, code):
                 'UNRESOLVED': lnt.testing.FAIL
                 }[code]
 
     def _test_failed_to_compile(self, raw_name, path):
         # FIXME: Do we need to add ".exe" in windows?
         name = raw_name.rsplit('.test', 1)[0]
         return not os.path.exists(os.path.join(path, name))
 
-    def _get_target_flags(self):
-        assert self.configured is True
-        return shlex.split(self.opts.cppflags + self.opts.cflags)
+    def _get_target_flags(self, cmake_vars):
+        build_type = cmake_vars["build_type"]
+        cflags = cmake_vars["c_flags"]
+        if build_type != "":
+            cflags = \
+                " ".join(cflags.split(" ") +
+                         cmake_vars["c_flags_"+build_type.lower()].split(" "))
+        return shlex.split(cflags)
 
     def _get_cc_info(self):
         assert self.configured is True
-        return lnt.testing.util.compilers.get_cc_info(self.opts.cc,
-                                                      self._get_target_flags())
+        cmake_lah_output = self._check_output(
+            [self.opts.cmake] + ['-LAH', '-N'] + [self._base_path])
+        pattern2var = [
+            (re.compile("^%s:[^=]*=(.*)$" % cmakevar), var)
+            for cmakevar, var in (
+                ("CMAKE_C_COMPILER", "cc"),
+                ("CMAKE_BUILD_TYPE", "build_type"),
+                ("CMAKE_CXX_FLAGS", "cxx_flags"),
+                ("CMAKE_CXX_FLAGS_DEBUG", "cxx_flags_debug"),
+                ("CMAKE_CXX_FLAGS_MINSIZEREL", "cxx_flags_minsizerel"),
+                ("CMAKE_CXX_FLAGS_RELEASE", "cxx_flags_release"),
+                ("CMAKE_CXX_FLAGS_RELWITHDEBINFO", "cxx_flags_relwithdebinfo"),
+                ("CMAKE_C_FLAGS", "c_flags"),
+                ("CMAKE_C_FLAGS_DEBUG", "c_flags_debug"),
+                ("CMAKE_C_FLAGS_MINSIZEREL", "c_flags_minsizerel"),
+                ("CMAKE_C_FLAGS_RELEASE", "c_flags_release"),
+                ("CMAKE_C_FLAGS_RELWITHDEBINFO", "c_flags_relwithdebinfo"),)]
+        cmake_vars = {}
+        for line in cmake_lah_output.split("\n"):
+            for pattern, varname in pattern2var:
+                m = re.match(pattern, line)
+                if m:
+                    cmake_vars[varname] = m.group(1)
+        return lnt.testing.util.compilers.get_cc_info(
+            cmake_vars["cc"], self._get_target_flags(cmake_vars))
 
     def _parse_lit_output(self, path, data, only_test=False):
         LIT_METRIC_TO_LNT = {
             'compile_time': 'compile',
             'exec_time': 'exec',
             'score': 'score',
             'hash': 'hash',
             'link_time': 'compile',

lnt/trunk/tests/SharedInputs/FakeCompilers/clang++-r154332

+link fakecompiler.py
 No newline at end of file

lnt/trunk/tests/SharedInputs/FakeCompilers/clang-r154332

+link fakecompiler.py
 No newline at end of file

lnt/trunk/tests/SharedInputs/FakeCompilers/fakecompiler.py

 Target: x86_64-apple-darwin11.3.0
 Thread model: posix
 InstalledDir: /home/foo/bin
 """
         print >>sys.stderr, """\
  "%s" "-cc1" "-E" ... more boring stuff here ...""" % (
             g_program,)
 
+class Clang_r154332(LLVMCompiler):
+    compiler_name = "clang-r154332"
+
+    def print_verbose_info(self):
+        print >>sys.stderr, """\
+clang version 3.1 (trunk 154332) (llvm/trunk 154329)
+Target: x86_64-apple-darwin11.3.0
+Thread model: posix
+InstalledDir: /home/foo/bin
+"""
+        print >>sys.stderr, """\
+ "%s" "-cc1" "-E" ... more boring stuff here ...""" % (
+            g_program,)
+
 # Clang build from a git repository.
 class Clang_git(LLVMCompiler):
     compiler_name = "clang-git"
 
     def print_verbose_info(self):
         print >>sys.stderr, """\
 clang version 3.1\
  (git:/git/clang.git 37ce0feee598d82e7220fa0a4b110619cae6ea72)\

lnt/trunk/tests/runtest/Inputs/test-suite-cmake/fake-cmake

 #!/bin/bash
 
-# If we passed a cache, just drop it and look like cmake.
+CMAKE_SRC_DIR="notfound"
+DUMP_VARS=false
+CMAKE_C_COMPILER=""
+CMAKE_CXX_COMPILER=""
+
+while test $# -gt 0
+do
   if [[ $1 == "-C" ]]; then
+    # If we passed a cache, just drop it and look like cmake.
     echo "Cmake Cache $2"
     shift
-    shift    
-fi    
-if [[ ! -f $1/CMakeLists.txt ]]; then
+    shift
+  elif [[ $1 == -D* ]]; then
+    # handle -D arguments to cmake.
+    if [[ $1 == -DCMAKE_C_COMPILER:* ]]; then CMAKE_C_COMPILER=${1#*=}; fi
+    if [[ $1 == -DCMAKE_CXX_COMPILER:* ]]; then CMAKE_CXX_COMPILER=${1#*=}; fi
+    shift
+  elif [[ $1 == -LAH && $2 == -N ]]; then
+    DUMP_VARS=true
+    shift
+    shift
+  elif [[ ! -f $1/CMakeLists.txt && ! -f $1/CMakeCache.txt ]]; then
+    # arguments not starting with -D or -C are assumed to point to the cmake
+    # src or build dir
     exit 1
   else
-  cp $1/fake-test $1/fake-results.json $1/fake-results-profile.json .
+    CMAKE_SRC_DIR=$1
+    # if CMakeCache.txt exists, read in value for compiler
+    if [[ -f $1/CMakeCache.txt ]]; then
+        CMAKE_C_COMPILER=`grep CMAKE_C_COMPILER:FILEPATH= $1/CMakeCache.txt | cut -f2 -d'='`
+        CMAKE_CXX_COMPILER=`grep CMAKE_CXX_COMPILER:FILEPATH= $1/CMakeCache.txt | cut -f2 -d'='`
+    fi
+    shift
+  fi
+done
+
+if [[ $DUMP_VARS == "true" ]]
+then
+  echo CMAKE_C_COMPILER:FILEPATH=$CMAKE_C_COMPILER
+  echo CMAKE_CXX_COMPILER:FILEPATH=$CMAKE_CXX_COMPILER
+  echo CMAKE_BUILD_TYPE:STRING=RelWithDebInfo
+  echo CMAKE_C_FLAGS:STRING=-O0
+  echo CMAKE_CXX_FLAGS:STRING=
+  echo CMAKE_C_FLAGS_RELWITHDEBINFO:STRING=-O2 -g
+  echo CMAKE_CXX_FLAGS_RELWITHDEBINFO:STRING=-O2 -g
+else
+  cp $CMAKE_SRC_DIR/fake-test $CMAKE_SRC_DIR/fake-results.json $CMAKE_SRC_DIR/fake-results-profile.json .
   echo "Dummy" > CMakeCache.txt
+  echo CMAKE_C_COMPILER:FILEPATH=$CMAKE_C_COMPILER >> CMakeCache.txt
+  echo CMAKE_CXX_COMPILER:FILEPATH=$CMAKE_CXX_COMPILER >> CMakeCache.txt
   mkdir subtest
-  cp $1/fake-test $1/fake-results.json subtest
-  exit 0
+  cp $CMAKE_SRC_DIR/fake-test $CMAKE_SRC_DIR/fake-results.json subtest
 fi
+exit 0

lnt/trunk/tests/runtest/test_suite.py

 # RUN:     --test-suite %S/Inputs/test-suite-cmake \
 # RUN:     --use-cmake %S/Inputs/test-suite-cmake/fake-cmake \
 # RUN:     --use-make %S/Inputs/test-suite-cmake/fake-make \
 # RUN:     --use-lit %S/Inputs/test-suite-cmake/fake-lit \
 # RUN:     > %t.log 2> %t.err || true
 # RUN: FileCheck  --check-prefix CHECK-MISSING-CC < %t.err %s
 # CHECK-MISSING-CC: error: --cc is required
 
+# Check on conflicting -cc and -cmake-define=CMAKE_C_COMPILER
+# options, the right compiler gets stored in the json report
+# RUN: lnt runtest test-suite \
+# RUN:     --sandbox %t.SANDBOX \
+# RUN:     --no-timestamp \
+# RUN:     --test-suite %S/Inputs/test-suite-cmake \
+# RUN:     --cmake-define=CMAKE_C_COMPILER:STRING=%{shared_inputs}/FakeCompilers/clang-r154332 \
+# RUN:     --cc %{shared_inputs}/FakeCompilers/clang-r154331 \
+# RUN:     --use-cmake %S/Inputs/test-suite-cmake/fake-cmake \
+# RUN:     --use-make %S/Inputs/test-suite-cmake/fake-make \
+# RUN:     --use-lit %S/Inputs/test-suite-cmake/fake-lit \
+# RUN:     > %t.log 2> %t.err || true
+# RUN: FileCheck --check-prefix CHECK-CC-CONFL-CMAKEDEFINE < %t.SANDBOX/build/report.json %s
+# CHECK-CC-CONFL-CMAKEDEFINE: "run_order": "154332"
+
+
 # Check running with PGO
 # RUN: lnt runtest test-suite \
 # RUN:     --sandbox %t.SANDBOX \
 # RUN:     --no-timestamp \
 # RUN:     --test-suite %S/Inputs/test-suite-cmake \
 # RUN:     --cc %{shared_inputs}/FakeCompilers/clang-r154331 \
 # RUN:     --use-cmake %S/Inputs/test-suite-cmake/fake-cmake \
 # RUN:     --use-make %S/Inputs/test-suite-cmake/fake-make \