cmake: Refactor CMakeExecutor and CMakeTraceParser

This moves most of the execution code from the CMakeInterpreter
into CMakeExecutor. Also, CMakeTraceParser is now responsible
for determining the trace cmd arguments.

5 files changed, 142 insertions, 68 deletions

diff --git a/mesonbuild/cmake/executor.py b/mesonbuild/cmake/executor.py
index bf4fa5d..c3303eb 100644
--- a/mesonbuild/cmake/executor.py
+++ b/mesonbuild/cmake/executor.py
@@ -15,8 +15,9 @@
 # This class contains the basic functionality needed to run any interpreter
 # or an interpreter-based tool.
 
-import subprocess
+import subprocess as S
 from pathlib import Path
+from threading import Thread
 import typing as T
 import re
 import os
@@ -30,19 +31,22 @@ from ..environment import Environment
 if T.TYPE_CHECKING:
     from ..dependencies.base import ExternalProgram
 
+TYPE_result = T.Tuple[int, T.Optional[str], T.Optional[str]]
 
 class CMakeExecutor:
     # The class's copy of the CMake path. Avoids having to search for it
     # multiple times in the same Meson invocation.
     class_cmakebin = PerMachine(None, None)
     class_cmakevers = PerMachine(None, None)
-    class_cmake_cache = {}
+    class_cmake_cache = {}  # type: T.Dict[T.Any, TYPE_result]
 
     def __init__(self, environment: Environment, version: str, for_machine: MachineChoice, silent: bool = False):
         self.min_version = version
         self.environment = environment
         self.for_machine = for_machine
         self.cmakebin, self.cmakevers = self.find_cmake_binary(self.environment, silent=silent)
+        self.always_capture_stderr = True
+        self.print_cmout = False
         if self.cmakebin is False:
             self.cmakebin = None
             return
@@ -130,17 +134,77 @@ class CMakeExecutor:
         cmvers = re.sub(r'\s*cmake version\s*', '', out.split('\n')[0]).strip()
         return cmvers
 
+    def set_exec_mode(self, print_cmout: T.Optional[bool] = None, always_capture_stderr: T.Optional[bool] = None) -> None:
+        if print_cmout is not None:
+            self.print_cmout = print_cmout
+        if always_capture_stderr is not None:
+            self.always_capture_stderr = always_capture_stderr
+
     def _cache_key(self, args: T.List[str], build_dir: str, env):
         fenv = frozenset(env.items()) if env is not None else None
         targs = tuple(args)
         return (self.cmakebin, targs, build_dir, fenv)
 
-    def _call_real(self, args: T.List[str], build_dir: str, env) -> T.Tuple[int, str, str]:
+    def _call_cmout_stderr(self, args: T.List[str], build_dir: str, env) -> TYPE_result:
+        cmd = self.cmakebin.get_command() + args
+        proc = S.Popen(cmd, stdout=S.PIPE, stderr=S.PIPE, cwd=build_dir, env=env)
+
+        # stdout and stderr MUST be read at the same time to avoid pipe
+        # blocking issues. The easiest way to do this is with a separate
+        # thread for one of the pipes.
+        def print_stdout():
+            while True:
+                line = proc.stdout.readline()
+                if not line:
+                    break
+                mlog.log(line.decode(errors='ignore').strip('\n'))
+            proc.stdout.close()
+
+        t = Thread(target=print_stdout)
+        t.start()
+
+        try:
+            # Read stderr line by line and log non trace lines
+            raw_trace = ''
+            tline_start_reg = re.compile(r'^\s*(.*\.(cmake|txt))\(([0-9]+)\):\s*(\w+)\(.*$')
+            inside_multiline_trace = False
+            while True:
+                line = proc.stderr.readline()
+                if not line:
+                    break
+                line = line.decode(errors='ignore')
+                if tline_start_reg.match(line):
+                    raw_trace += line
+                    inside_multiline_trace = not line.endswith(' )\n')
+                elif inside_multiline_trace:
+                    raw_trace += line
+                else:
+                    mlog.warning(line.strip('\n'))
+
+        finally:
+            proc.stderr.close()
+            t.join()
+            proc.wait()
+
+        return proc.returncode, None, raw_trace
+
+    def _call_cmout(self, args: T.List[str], build_dir: str, env) -> TYPE_result:
+        cmd = self.cmakebin.get_command() + args
+        proc = S.Popen(cmd, stdout=S.PIPE, stderr=S.STDOUT, cwd=build_dir, env=env)
+        while True:
+            line = proc.stdout.readline()
+            if not line:
+                break
+            mlog.log(line.decode(errors='ignore').strip('\n'))
+        proc.stdout.close()
+        proc.wait()
+        return proc.returncode, None, None
+
+    def _call_quiet(self, args: T.List[str], build_dir: str, env) -> TYPE_result:
         os.makedirs(build_dir, exist_ok=True)
         cmd = self.cmakebin.get_command() + args
-        ret = subprocess.run(cmd, env=env, cwd=build_dir, close_fds=False,
-                             stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                             universal_newlines=False)
+        ret = S.run(cmd, env=env, cwd=build_dir, close_fds=False,
+                    stdout=S.PIPE, stderr=S.PIPE, universal_newlines=False)
         rc = ret.returncode
         out = ret.stdout.decode(errors='ignore')
         err = ret.stderr.decode(errors='ignore')
@@ -148,21 +212,30 @@ class CMakeExecutor:
         mlog.debug("Called `{}` in {} -> {}".format(call, build_dir, rc))
         return rc, out, err
 
-    def call(self, args: T.List[str], build_dir: str, env=None, disable_cache: bool = False):
+    def _call_impl(self, args: T.List[str], build_dir: str, env) -> TYPE_result:
+        if not self.print_cmout:
+            return self._call_quiet(args, build_dir, env)
+        else:
+            if self.always_capture_stderr:
+                return self._call_cmout_stderr(args, build_dir, env)
+            else:
+                return self._call_cmout(args, build_dir, env)
+
+    def call(self, args: T.List[str], build_dir: str, env=None, disable_cache: bool = False) -> TYPE_result:
         if env is None:
             env = os.environ
 
         if disable_cache:
-            return self._call_real(args, build_dir, env)
+            return self._call_impl(args, build_dir, env)
 
         # First check if cached, if not call the real cmake function
         cache = CMakeExecutor.class_cmake_cache
         key = self._cache_key(args, build_dir, env)
         if key not in cache:
-            cache[key] = self._call_real(args, build_dir, env)
+            cache[key] = self._call_impl(args, build_dir, env)
         return cache[key]
 
-    def call_with_fake_build(self, args: T.List[str], build_dir: str, env=None):
+    def call_with_fake_build(self, args: T.List[str], build_dir: str, env=None) -> TYPE_result:
         # First check the cache
         cache = CMakeExecutor.class_cmake_cache
         key = self._cache_key(args, build_dir, env)
@@ -282,7 +355,7 @@ set(CMAKE_SIZEOF_VOID_P "{}")
     def executable_path(self) -> str:
         return self.cmakebin.get_path()
 
-    def get_command(self):
+    def get_command(self) -> T.List[str]:
         return self.cmakebin.get_command()
 
     def machine_choice(self) -> MachineChoice:
diff --git a/mesonbuild/cmake/interpreter.py b/mesonbuild/cmake/interpreter.py
index 703815e..4f7700f 100644
--- a/mesonbuild/cmake/interpreter.py
+++ b/mesonbuild/cmake/interpreter.py
@@ -24,8 +24,6 @@ from .. import mlog
 from ..environment import Environment
 from ..mesonlib import MachineChoice, version_compare
 from ..compilers.compilers import lang_suffixes, header_suffixes, obj_suffixes, lib_suffixes, is_header
-from subprocess import Popen, PIPE
-from threading import Thread
 from enum import Enum
 from functools import lru_cache
 import typing as T
@@ -741,7 +739,7 @@ class CMakeInterpreter:
         self.languages = []
         self.targets = []
         self.custom_targets = []  # type: T.List[ConverterCustomTarget]
-        self.trace = CMakeTraceParser()
+        self.trace = CMakeTraceParser('', '')  # Will be replaced in analyse
         self.output_target_map = OutputTargetMap(self.build_dir)
 
         # Generated meson data
@@ -754,10 +752,11 @@ class CMakeInterpreter:
         cmake_exe = CMakeExecutor(self.env, '>=3.7', for_machine)
         if not cmake_exe.found():
             raise CMakeException('Unable to find CMake')
+        self.trace = CMakeTraceParser(cmake_exe.version(), self.build_dir, permissive=True)
 
         generator = backend_generator_map[self.backend_name]
-        cmake_args = cmake_exe.get_command()
-        trace_args = ['--trace', '--trace-expand', '--no-warn-unused-cli']
+        cmake_args = []
+        trace_args = self.trace.trace_args()
         cmcmp_args = ['-DCMAKE_POLICY_WARNING_{}=OFF'.format(x) for x in disable_policy_warnings]
 
         if version_compare(cmake_exe.version(), '>=3.14'):
@@ -795,46 +794,15 @@ class CMakeInterpreter:
             os.makedirs(self.build_dir, exist_ok=True)
             os_env = os.environ.copy()
             os_env['LC_ALL'] = 'C'
-            final_command = cmake_args + trace_args + cmcmp_args + [self.src_dir]
-            proc = Popen(final_command, stdout=PIPE, stderr=PIPE, cwd=self.build_dir, env=os_env)
-
-            def print_stdout():
-                while True:
-                    line = proc.stdout.readline()
-                    if not line:
-                        break
-                    mlog.log(line.decode('utf-8').strip('\n'))
-                proc.stdout.close()
-
-            t = Thread(target=print_stdout)
-            t.start()
-
-            # Read stderr line by line and log non trace lines
-            self.raw_trace = ''
-            tline_start_reg = re.compile(r'^\s*(.*\.(cmake|txt))\(([0-9]+)\):\s*(\w+)\(.*$')
-            inside_multiline_trace = False
-            while True:
-                line = proc.stderr.readline()
-                if not line:
-                    break
-                line = line.decode('utf-8')
-                if tline_start_reg.match(line):
-                    self.raw_trace += line
-                    inside_multiline_trace = not line.endswith(' )\n')
-                elif inside_multiline_trace:
-                    self.raw_trace += line
-                else:
-                    mlog.warning(line.strip('\n'))
-
-            proc.stderr.close()
-            proc.wait()
+            final_args = cmake_args + trace_args + cmcmp_args + [self.src_dir]
 
-            t.join()
+            cmake_exe.set_exec_mode(print_cmout=True)
+            rc, _, self.raw_trace = cmake_exe.call(final_args, self.build_dir, env=os_env, disable_cache=True)
 
         mlog.log()
-        h = mlog.green('SUCCEEDED') if proc.returncode == 0 else mlog.red('FAILED')
+        h = mlog.green('SUCCEEDED') if rc == 0 else mlog.red('FAILED')
         mlog.log('CMake configuration:', h)
-        if proc.returncode != 0:
+        if rc != 0:
             raise CMakeException('Failed to configure the CMake subproject')
 
     def initialise(self, extra_cmake_options: T.List[str]) -> None:
@@ -889,7 +857,6 @@ class CMakeInterpreter:
         self.languages = []
         self.targets = []
         self.custom_targets = []
-        self.trace = CMakeTraceParser(permissive=True)
 
         # Parse the trace
         self.trace.parse(self.raw_trace)
diff --git a/mesonbuild/cmake/traceparser.py b/mesonbuild/cmake/traceparser.py
index ceb5b02..cdaeb3d 100644
--- a/mesonbuild/cmake/traceparser.py
+++ b/mesonbuild/cmake/traceparser.py
@@ -18,8 +18,10 @@
 from .common import CMakeException
 from .generator import parse_generator_expressions
 from .. import mlog
+from ..mesonlib import version_compare
 
 import typing as T
+from pathlib import Path
 import re
 import os
 
@@ -60,7 +62,7 @@ class CMakeGeneratorTarget(CMakeTarget):
         self.working_dir = None  # type: T.Optional[str]
 
 class CMakeTraceParser:
-    def __init__(self, permissive: bool = False):
+    def __init__(self, cmake_version: str, build_dir: str, permissive: bool = False):
         # Dict of CMake variables: '<var_name>': ['list', 'of', 'values']
         self.vars = {}
 
@@ -71,10 +73,27 @@ class CMakeTraceParser:
         self.custom_targets = []  # type: T.List[CMakeGeneratorTarget]
 
         self.permissive = permissive  # type: bool
+        self.cmake_version = cmake_version  # type: str
+        self.trace_format = 'human'
 
-    def parse(self, trace: str) -> None:
-        # First parse the trace
-        lexer1 = self._lex_trace(trace)
+    def trace_args(self) -> T.List[str]:
+        arg_map = {
+            'human': ['--trace', '--trace-expand'],
+        }
+
+        base_args = ['--no-warn-unused-cli']
+        return arg_map[self.trace_format] + base_args
+
+    def parse(self, trace: T.Optional[str] = None) -> None:
+        if not trace:
+            raise CMakeException('CMake: The CMake trace was not provided or is empty')
+
+        # Second parse the trace
+        lexer1 = None
+        if self.trace_format == 'human':
+            lexer1 = self._lex_trace_human(trace)
+        else:
+            raise CMakeException('CMake: Internal error: Invalid trace format {}. Expected [human]'.format(self.trace_format))
 
         # All supported functions
         functions = {
@@ -481,7 +500,7 @@ class CMakeTraceParser:
 
             self.targets[target].properties[i[0]] += i[1]
 
-    def _lex_trace(self, trace):
+    def _lex_trace_human(self, trace):
         # The trace format is: '<file>(<line>):  <func>(<args -- can contain \n> )\n'
         reg_tline = re.compile(r'\s*(.*\.(cmake|txt))\(([0-9]+)\):\s*(\w+)\(([\s\S]*?) ?\)\s*\n', re.MULTILINE)
         reg_other = re.compile(r'[^\n]*\n')
@@ -510,7 +529,7 @@ class CMakeTraceParser:
             yield CMakeTraceLine(file, line, func, args)
 
     def _guess_files(self, broken_list: T.List[str]) -> T.List[str]:
-        #Try joining file paths that contain spaces
+        # Try joining file paths that contain spaces
 
         reg_start = re.compile(r'^([A-Za-z]:)?/.*/[^./]+$')
         reg_end = re.compile(r'^.*\.[a-zA-Z]+$')
diff --git a/mesonbuild/dependencies/base.py b/mesonbuild/dependencies/base.py
index 282c314..156be04 100644
--- a/mesonbuild/dependencies/base.py
+++ b/mesonbuild/dependencies/base.py
@@ -1089,7 +1089,6 @@ class CMakeDependency(ExternalDependency):
         # stored in the pickled coredata and recovered.
         self.cmakebin = None
         self.cmakeinfo = None
-        self.traceparser = CMakeTraceParser()
 
         # Where all CMake "build dirs" are located
         self.cmake_root_dir = environment.scratch_dir
@@ -1097,6 +1096,10 @@ class CMakeDependency(ExternalDependency):
         # T.List of successfully found modules
         self.found_modules = []
 
+        # Initialize with None before the first return to avoid
+        # AttributeError exceptions in derived classes
+        self.traceparser = None  # type: CMakeTraceParser
+
         self.cmakebin = CMakeExecutor(environment, CMakeDependency.class_cmake_version, self.for_machine, silent=self.silent)
         if not self.cmakebin.found():
             self.cmakebin = None
@@ -1106,6 +1109,9 @@ class CMakeDependency(ExternalDependency):
             mlog.debug(msg)
             return
 
+        # Setup the trace parser
+        self.traceparser = CMakeTraceParser(self.cmakebin.version(), self._get_build_dir())
+
         if CMakeDependency.class_cmakeinfo[self.for_machine] is None:
             CMakeDependency.class_cmakeinfo[self.for_machine] = self._get_cmake_info()
         self.cmakeinfo = CMakeDependency.class_cmakeinfo[self.for_machine]
@@ -1151,11 +1157,13 @@ class CMakeDependency(ExternalDependency):
             gen_list += [CMakeDependency.class_working_generator]
         gen_list += CMakeDependency.class_cmake_generators
 
+        temp_parser = CMakeTraceParser(self.cmakebin.version(), self._get_build_dir())
+
         for i in gen_list:
             mlog.debug('Try CMake generator: {}'.format(i if len(i) > 0 else 'auto'))
 
             # Prepare options
-            cmake_opts = ['--trace-expand', '.']
+            cmake_opts = temp_parser.trace_args() + ['.']
             if len(i) > 0:
                 cmake_opts = ['-G', i] + cmake_opts
 
@@ -1175,7 +1183,6 @@ class CMakeDependency(ExternalDependency):
             return None
 
         try:
-            temp_parser = CMakeTraceParser()
             temp_parser.parse(err1)
         except MesonException:
             return None
@@ -1328,7 +1335,8 @@ class CMakeDependency(ExternalDependency):
             mlog.debug('Try CMake generator: {}'.format(i if len(i) > 0 else 'auto'))
 
             # Prepare options
-            cmake_opts = ['--trace-expand', '-DNAME={}'.format(name), '-DARCHS={}'.format(';'.join(self.cmakeinfo['archs']))] + args + ['.']
+            cmake_opts = ['-DNAME={}'.format(name), '-DARCHS={}'.format(';'.join(self.cmakeinfo['archs']))] + args + ['.']
+            cmake_opts += self.traceparser.trace_args()
             cmake_opts += self._extra_cmake_opts()
             if len(i) > 0:
                 cmake_opts = ['-G', i] + cmake_opts
@@ -1499,10 +1507,14 @@ class CMakeDependency(ExternalDependency):
         self.compile_args = compileOptions + compileDefinitions + ['-I{}'.format(x) for x in incDirs]
         self.link_args = libraries
 
-    def _setup_cmake_dir(self, cmake_file: str) -> str:
-        # Setup the CMake build environment and return the "build" directory
+    def _get_build_dir(self) -> str:
         build_dir = Path(self.cmake_root_dir) / 'cmake_{}'.format(self.name)
         build_dir.mkdir(parents=True, exist_ok=True)
+        return str(build_dir)
+
+    def _setup_cmake_dir(self, cmake_file: str) -> str:
+        # Setup the CMake build environment and return the "build" directory
+        build_dir = self._get_build_dir()
 
         # Insert language parameters into the CMakeLists.txt and write new CMakeLists.txt
         src_cmake = Path(__file__).parent / 'data' / cmake_file
@@ -1525,11 +1537,11 @@ cmake_minimum_required(VERSION ${{CMAKE_VERSION}})
 project(MesonTemp LANGUAGES {})
 """.format(' '.join(cmake_language)) + cmake_txt
 
-        cm_file = build_dir / 'CMakeLists.txt'
+        cm_file = Path(build_dir) / 'CMakeLists.txt'
         cm_file.write_text(cmake_txt)
         mlog.cmd_ci_include(cm_file.absolute().as_posix())
 
-        return str(build_dir)
+        return build_dir
 
     def _call_cmake(self, args, cmake_file: str, env=None):
         build_dir = self._setup_cmake_dir(cmake_file)
@@ -1552,7 +1564,7 @@ project(MesonTemp LANGUAGES {})
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, default_value: T.Optional[str] = None,
                      pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
-        if cmake:
+        if cmake and self.traceparser is not None:
             try:
                 v = self.traceparser.vars[cmake]
             except KeyError:
diff --git a/mesonbuild/dependencies/dev.py b/mesonbuild/dependencies/dev.py
index def2adf..49f8fea 100644
--- a/mesonbuild/dependencies/dev.py
+++ b/mesonbuild/dependencies/dev.py
@@ -396,6 +396,9 @@ class LLVMDependencyCMake(CMakeDependency):
         self.llvm_opt_modules = stringlistify(extract_as_list(kwargs, 'optional_modules'))
         super().__init__(name='LLVM', environment=env, language='cpp', kwargs=kwargs)
 
+        if self.traceparser is None:
+            return
+
         # Extract extra include directories and definitions
         inc_dirs = self.traceparser.get_cmake_var('PACKAGE_INCLUDE_DIRS')
         defs = self.traceparser.get_cmake_var('PACKAGE_DEFINITIONS')