[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

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

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

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

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/tests/test_suite.py

def _test_suite_dir(self):
         return self.opts.test_suite_root
 
     def _build_threads(self):
         return self.opts.build_threads or self.opts.threads
 
     def _test_threads(self):
         return self.opts.threads
 
-    def _check_call(self, *args, **kwargs):
+    def _check_output(self, *args, **kwargs):
         note('Execute: %s' % ' '.join(args[0]))
         if 'cwd' in kwargs:
             note('          (In %s)' % kwargs['cwd'])
-        return subprocess.check_call(*args, **kwargs)
+        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)
 
-        self._check_call([make_cmd, 'clean'],
+        self._check_output([make_cmd, 'clean'],
                            cwd=subdir)
 
     def _configure(self, path, extra_cmake_defs=[], execute=True):
         cmake_cmd = self.opts.cmake
 
         defs = {
             # FIXME: Support ARCH, SMALL/LARGE etc
             'CMAKE_C_COMPILER': self.opts.cc,
             'CMAKE_CXX_COMPILER': self.opts.cxx,
def _configure(self, path, extra_cmake_defs=[], execute=True):
             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)
+                # 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
+
         for item in extra_cmake_defs:
             k, v = item.split('=', 1)
             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:
             # Shortcut for the common case.
             if not cache.endswith(".cmake") and "/" not in cache:
                 cache = os.path.join(self._test_suite_dir(),
                                      "cmake/caches", cache + ".cmake")
             if not os.path.exists(cache):
                 fatal("Could not find CMake cache file: " + cache)
             cmake_flags += ['-C', cache]
 
         for l in lines:
             note(l)
 
         cmake_cmd = [cmake_cmd] + cmake_flags + [self._test_suite_dir()] + \
                     ['-D%s=%s' % (k, v) for k, v in defs.items()]
         if execute:
-            self._check_call(cmake_cmd, cwd=path)
+            self._check_output(cmake_cmd, cwd=path)
 
         return cmake_cmd
 
     def _collect_pgo(self, path):
         extra_defs = ["TEST_SUITE_PROFILE_GENERATE=On",
                       "TEST_SUITE_RUN_TYPE=train"]
         self._configure(path, extra_cmake_defs=extra_defs)
         self._make(path)
def _make(self, path):
             subdir = os.path.join(*components)
 
         note('Building...')
         if not self.opts.succinct:
             args = ["VERBOSE=1", target]
         else:
             args = [target]
         try:
-            self._check_call([make_cmd,
+            self._check_output([make_cmd,
                               '-k', '-j', str(self._build_threads())] + args,
                              cwd=subdir)
         except subprocess.CalledProcessError:
             # make is expected to exit with code 1 if there was any build
             # failure. Build failures are not unexpected when testing an
             # experimental compiler.
             pass
 
def _lit(self, path, test):
         extra_args = []
         if not test:
             extra_args = ['--no-execute']
         if self.opts.use_perf in ('profile', 'all'):
             extra_args += ['--param', 'profile=perf']
 
         note('Testing...')
         try:
-            self._check_call([lit_cmd,
+            self._check_output([lit_cmd,
                                 '-v',
                                 '-j', str(self._test_threads()),
                                 subdir,
                                 '-o', output_json_path.name] + extra_args)
         except subprocess.CalledProcessError:
             # LIT is expected to exit with code 1 if there were test
             # failures!
             pass
         return json.loads(open(output_json_path.name).read())
 
     def _is_pass_code(self, code):
         return code in ('PASS', 'XPASS', 'XFAIL')
 
     def _get_lnt_code(self, code):
         return {'PASS': lnt.testing.PASS,
                 'FAIL': lnt.testing.FAIL,
                 'XFAIL': lnt.testing.XFAIL,
                 'XPASS': lnt.testing.FAIL,
                 '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._get_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.search(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',

tests/SharedInputs/FakeCompilers/clang++-r154332

+fakecompiler.py
 No newline at end of file

tests/SharedInputs/FakeCompilers/clang-r154332

+fakecompiler.py
 No newline at end of file

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

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

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 \