From 3607f50d7f5bfa33806a4ed48d4fb773f35ee537 Mon Sep 17 00:00:00 2001 From: Daniel Mensinger Date: Wed, 8 Jan 2020 17:06:58 +0100 Subject: [PATCH 1/3] 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. --- mesonbuild/cmake/executor.py | 95 +++++++++++++++++++++++++++++---- mesonbuild/cmake/interpreter.py | 51 ++++-------------- mesonbuild/cmake/traceparser.py | 31 ++++++++--- mesonbuild/dependencies/base.py | 30 +++++++---- mesonbuild/dependencies/dev.py | 3 ++ 5 files changed, 142 insertions(+), 68 deletions(-) diff --git a/mesonbuild/cmake/executor.py b/mesonbuild/cmake/executor.py index bf4fa5d08..c3303ebd6 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 703815e24..4f7700f72 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 ceb5b024f..cdaeb3db7 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: '': ['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: '(): ( )\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 282c314b6..156be0438 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 def2adf40..49f8feadd 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') From acc6dbfab7ed58e8c20cd61252d4d111bd9ca642 Mon Sep 17 00:00:00 2001 From: Daniel Mensinger Date: Wed, 8 Jan 2020 17:10:24 +0100 Subject: [PATCH 2/3] cmake: Add support for --trace-redirect --- mesonbuild/cmake/interpreter.py | 2 +- mesonbuild/cmake/traceparser.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/mesonbuild/cmake/interpreter.py b/mesonbuild/cmake/interpreter.py index 4f7700f72..2aa0c0191 100644 --- a/mesonbuild/cmake/interpreter.py +++ b/mesonbuild/cmake/interpreter.py @@ -796,7 +796,7 @@ class CMakeInterpreter: os_env['LC_ALL'] = 'C' final_args = cmake_args + trace_args + cmcmp_args + [self.src_dir] - cmake_exe.set_exec_mode(print_cmout=True) + cmake_exe.set_exec_mode(print_cmout=True, always_capture_stderr=self.trace.requires_stderr()) rc, _, self.raw_trace = cmake_exe.call(final_args, self.build_dir, env=os_env, disable_cache=True) mlog.log() diff --git a/mesonbuild/cmake/traceparser.py b/mesonbuild/cmake/traceparser.py index cdaeb3db7..5bf9547b2 100644 --- a/mesonbuild/cmake/traceparser.py +++ b/mesonbuild/cmake/traceparser.py @@ -74,6 +74,8 @@ class CMakeTraceParser: self.permissive = permissive # type: bool self.cmake_version = cmake_version # type: str + self.trace_file = 'cmake_trace.txt' + self.trace_file_path = Path(build_dir) / self.trace_file self.trace_format = 'human' def trace_args(self) -> T.List[str]: @@ -82,9 +84,20 @@ class CMakeTraceParser: } base_args = ['--no-warn-unused-cli'] + if not self.requires_stderr(): + base_args += ['--trace-redirect={}'.format(self.trace_file)] + return arg_map[self.trace_format] + base_args + def requires_stderr(self) -> bool: + return version_compare(self.cmake_version, '<3.16') + def parse(self, trace: T.Optional[str] = None) -> None: + # First load the trace (if required) + if not self.requires_stderr(): + if not self.trace_file_path.exists and not self.trace_file_path.is_file(): + raise CMakeException('CMake: Trace file "{}" not found'.format(str(self.trace_file_path))) + trace = self.trace_file_path.read_text() if not trace: raise CMakeException('CMake: The CMake trace was not provided or is empty') From 375a51712b0ac01c2aa055f0198abba4b9ca4a34 Mon Sep 17 00:00:00 2001 From: Daniel Mensinger Date: Wed, 8 Jan 2020 17:13:55 +0100 Subject: [PATCH 3/3] cmake: Some test improvements --- run_project_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/run_project_tests.py b/run_project_tests.py index e480160b1..acad22533 100755 --- a/run_project_tests.py +++ b/run_project_tests.py @@ -856,6 +856,8 @@ def check_format(): continue if 'meson-logs' in root or 'meson-private' in root: continue + if '__CMake_build' in root: + continue if '.eggs' in root or '_cache' in root: # e.g. .mypy_cache continue for fname in filenames: @@ -919,7 +921,7 @@ def print_tool_versions(): { 'tool': 'cmake', 'args': ['--version'], - 'regex': re.compile(r'^cmake version ([0-9]+(\.[0-9]+)*)$'), + 'regex': re.compile(r'^cmake version ([0-9]+(\.[0-9]+)*(-[a-z0-9]+)?)$'), 'match_group': 1, }, ]