Merge pull request #6635 from mensinda/cmOTMFix

cmake: Fix dependency loops in custom targets (fixes #6632)

mesonbuild/cmake/data/preload.cmake

@@ -0,0 +1,35 @@
+if(MESON_PS_LOADED)
+  return()
+endif()
+
+set(MESON_PS_LOADED ON)
+
+# Dummy macros that have a special meaning in the meson code
+macro(meson_ps_execute_delayed_calls)
+endmacro()
+
+macro(meson_ps_reload_vars)
+endmacro()
+
+# Helper macro to inspect the current CMake state
+macro(meson_ps_inspect_vars)
+  set(MESON_PS_CMAKE_CURRENT_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  set(MESON_PS_CMAKE_CURRENT_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
+  meson_ps_execute_delayed_calls()
+endmacro()
+
+
+# Override some system functions with custom code and forward the args
+# to the original function
+macro(add_custom_command)
+  meson_ps_inspect_vars()
+  _add_custom_command(${ARGV})
+endmacro()
+
+macro(add_custom_target)
+  meson_ps_inspect_vars()
+  _add_custom_target(${ARGV})
+endmacro()
+
+set(MESON_PS_DELAYED_CALLS add_custom_command;add_custom_target)
+meson_ps_reload_vars()

mesonbuild/cmake/interpreter.py

@@ -26,6 +26,7 @@ from ..mesonlib import MachineChoice, version_compare
 from ..compilers.compilers import lang_suffixes, header_suffixes, obj_suffixes, lib_suffixes, is_header
 from enum import Enum
 from functools import lru_cache
+from pathlib import Path
 import typing as T
 import os, re
 
@@ -592,6 +593,8 @@ class ConverterCustomTarget:
     out_counter = 0  # type: int
 
     def __init__(self, target: CMakeGeneratorTarget):
+        assert(target.current_bin_dir is not None)
+        assert(target.current_src_dir is not None)
         self.name = target.name
         if not self.name:
             self.name = 'custom_tgt_{}'.format(ConverterCustomTarget.tgt_counter)
@@ -605,6 +608,8 @@ class ConverterCustomTarget:
         self.depends_raw = target.depends
         self.inputs = []
         self.depends = []
+        self.current_bin_dir = Path(target.current_bin_dir)
+        self.current_src_dir = Path(target.current_src_dir)
 
         # Convert the target name to a valid meson target name
         self.name = _sanitize_cmake_name(self.name)
@@ -612,29 +617,24 @@ class ConverterCustomTarget:
     def __repr__(self) -> str:
         return '<{}: {} {}>'.format(self.__class__.__name__, self.name, self.outputs)
 
-    def postprocess(self, output_target_map: OutputTargetMap, root_src_dir: str, subdir: str, build_dir: str, all_outputs: T.List[str]) -> None:
-        # Default the working directory to the CMake build dir. This
-        # is not 100% correct, since it should be the value of
-        # ${CMAKE_CURRENT_BINARY_DIR} when add_custom_command is
-        # called. However, keeping track of this variable is not
-        # trivial and the current solution should work in most cases.
+    def postprocess(self, output_target_map: OutputTargetMap, root_src_dir: str, subdir: str, all_outputs: T.List[str]) -> None:
+        # Default the working directory to ${CMAKE_CURRENT_BINARY_DIR}
         if not self.working_dir:
-            self.working_dir = build_dir
+            self.working_dir = self.current_bin_dir.as_posix()
 
         # relative paths in the working directory are always relative
-        # to ${CMAKE_CURRENT_BINARY_DIR} (see note above)
+        # to ${CMAKE_CURRENT_BINARY_DIR}
         if not os.path.isabs(self.working_dir):
-            self.working_dir = os.path.normpath(os.path.join(build_dir, self.working_dir))
+            self.working_dir = (self.current_bin_dir / self.working_dir).as_posix()
 
         # Modify the original outputs if they are relative. Again,
         # relative paths are relative to ${CMAKE_CURRENT_BINARY_DIR}
-        # and the first disclaimer is still in effect
-        def ensure_absolute(x: str):
-            if os.path.isabs(x):
+        def ensure_absolute(x: Path) -> Path:
+            if x.is_absolute():
                 return x
             else:
-                return os.path.normpath(os.path.join(build_dir, x))
-        self.original_outputs = [ensure_absolute(x) for x in self.original_outputs]
+                return self.current_bin_dir / x
+        self.original_outputs = [ensure_absolute(Path(x)).as_posix() for x in self.original_outputs]
 
         # Ensure that there is no duplicate output in the project so
         # that meson can handle cases where the same filename is
@@ -671,23 +671,35 @@ class ConverterCustomTarget:
             self.outputs = [self.name + '.h']
 
         # Check dependencies and input files
+        root = Path(root_src_dir)
         for i in self.depends_raw:
             if not i:
                 continue
+            raw = Path(i)
             art = output_target_map.artifact(i)
             tgt = output_target_map.target(i)
             gen = output_target_map.generated(i)
 
-            if art:
+            rel_to_root = None
+            try:
+                rel_to_root = raw.relative_to(root)
+            except ValueError:
+                rel_to_root = None
+
+            # First check for existing files. Only then check for existing
+            # targets, etc. This reduces the chance of misdetecting input files
+            # as outputs from other targets.
+            # See https://github.com/mesonbuild/meson/issues/6632
+            if not raw.is_absolute() and (self.current_src_dir / raw).exists():
+                self.inputs += [(self.current_src_dir / raw).relative_to(root).as_posix()]
+            elif raw.is_absolute() and raw.exists() and rel_to_root is not None:
+                self.inputs += [rel_to_root.as_posix()]
+            elif art:
                 self.depends += [art]
             elif tgt:
                 self.depends += [tgt]
             elif gen:
                 self.inputs += [gen.get_ref(i)]
-            elif not os.path.isabs(i) and os.path.exists(os.path.join(root_src_dir, i)):
-                self.inputs += [i]
-            elif os.path.isabs(i) and os.path.exists(i) and os.path.commonpath([i, root_src_dir]) == root_src_dir:
-                self.inputs += [os.path.relpath(i, root_src_dir)]
 
     def process_inter_target_dependencies(self):
         # Move the dependencies from all transfer_dependencies_from to the target
@@ -767,10 +779,19 @@ class CMakeInterpreter:
             raise CMakeException('Unable to find CMake')
         self.trace = CMakeTraceParser(cmake_exe.version(), self.build_dir, permissive=True)
 
+        preload_file = Path(__file__).resolve().parent / 'data' / 'preload.cmake'
+
+        # Prefere CMAKE_PROJECT_INCLUDE over CMAKE_TOOLCHAIN_FILE if possible,
+        # since CMAKE_PROJECT_INCLUDE was actually designed for code injection.
+        preload_var = 'CMAKE_PROJECT_INCLUDE'
+        if version_compare(cmake_exe.version(), '<3.15'):
+            preload_var = 'CMAKE_TOOLCHAIN_FILE'
+
         generator = backend_generator_map[self.backend_name]
         cmake_args = []
         trace_args = self.trace.trace_args()
         cmcmp_args = ['-DCMAKE_POLICY_WARNING_{}=OFF'.format(x) for x in disable_policy_warnings]
+        pload_args = ['-D{}={}'.format(preload_var, str(preload_file))]
 
         if version_compare(cmake_exe.version(), '>=3.14'):
             self.cmake_api = CMakeAPI.FILE
@@ -802,12 +823,13 @@ class CMakeInterpreter:
             mlog.log(mlog.bold('  - build directory:         '), self.build_dir)
             mlog.log(mlog.bold('  - source directory:        '), self.src_dir)
             mlog.log(mlog.bold('  - trace args:              '), ' '.join(trace_args))
+            mlog.log(mlog.bold('  - preload file:            '), str(preload_file))
             mlog.log(mlog.bold('  - disabled policy warnings:'), '[{}]'.format(', '.join(disable_policy_warnings)))
             mlog.log()
             os.makedirs(self.build_dir, exist_ok=True)
             os_env = os.environ.copy()
             os_env['LC_ALL'] = 'C'
-            final_args = cmake_args + trace_args + cmcmp_args + [self.src_dir]
+            final_args = cmake_args + trace_args + cmcmp_args + pload_args + [self.src_dir]
 
             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)
@@ -913,7 +935,7 @@ class CMakeInterpreter:
         object_libs = []
         custom_target_outputs = []  # type: T.List[str]
         for i in self.custom_targets:
-            i.postprocess(self.output_target_map, self.src_dir, self.subdir, self.build_dir, custom_target_outputs)
+            i.postprocess(self.output_target_map, self.src_dir, self.subdir, custom_target_outputs)
         for i in self.targets:
             i.postprocess(self.output_target_map, self.src_dir, self.subdir, self.install_prefix, self.trace)
             if i.type == 'OBJECT_LIBRARY':

mesonbuild/cmake/traceparser.py

@@ -47,6 +47,8 @@ class CMakeTarget:
         self.imported = imported
         self.tline = tline
         self.depends = []
+        self.current_bin_dir = None
+        self.current_src_dir = None
 
     def __repr__(self):
         s = 'CMake TARGET:\n  -- name:      {}\n  -- type:      {}\n  -- imported:  {}\n  -- properties: {{\n{}     }}\n  -- tline: {}'
@@ -83,6 +85,36 @@ class CMakeTraceParser:
         self.trace_file_path = Path(build_dir) / self.trace_file
         self.trace_format = 'json-v1' if version_compare(cmake_version, '>=3.17') else 'human'
 
+        # State for delayed command execution. Delayed command execution is realised
+        # with a custom CMake file that overrides some functions and adds some
+        # introspection information to the trace.
+        self.delayed_commands = []  # type: T.List[str]
+        self.stored_commands = []   # type: T.List[CMakeTraceLine]
+
+        # All supported functions
+        self.functions = {
+            'set': self._cmake_set,
+            'unset': self._cmake_unset,
+            'add_executable': self._cmake_add_executable,
+            'add_library': self._cmake_add_library,
+            'add_custom_command': self._cmake_add_custom_command,
+            'add_custom_target': self._cmake_add_custom_target,
+            'set_property': self._cmake_set_property,
+            'set_target_properties': self._cmake_set_target_properties,
+            'target_compile_definitions': self._cmake_target_compile_definitions,
+            'target_compile_options': self._cmake_target_compile_options,
+            'target_include_directories': self._cmake_target_include_directories,
+            'target_link_libraries': self._cmake_target_link_libraries,
+            'target_link_options': self._cmake_target_link_options,
+            'add_dependencies': self._cmake_add_dependencies,
+
+            # Special functions defined in the preload script.
+            # These functions do nothing in the CMake code, but have special
+            # meaning here in the trace parser.
+            'meson_ps_execute_delayed_calls': self._meson_ps_execute_delayed_calls,
+            'meson_ps_reload_vars': self._meson_ps_reload_vars,
+        }
+
     def trace_args(self) -> T.List[str]:
         arg_map = {
             'human': ['--trace', '--trace-expand'],
@@ -116,28 +148,15 @@ class CMakeTraceParser:
         else:
             raise CMakeException('CMake: Internal error: Invalid trace format {}. Expected [human, json-v1]'.format(self.trace_format))
 
-        # All supported functions
-        functions = {
-            'set': self._cmake_set,
-            'unset': self._cmake_unset,
-            'add_executable': self._cmake_add_executable,
-            'add_library': self._cmake_add_library,
-            'add_custom_command': self._cmake_add_custom_command,
-            'add_custom_target': self._cmake_add_custom_target,
-            'set_property': self._cmake_set_property,
-            'set_target_properties': self._cmake_set_target_properties,
-            'target_compile_definitions': self._cmake_target_compile_definitions,
-            'target_compile_options': self._cmake_target_compile_options,
-            'target_include_directories': self._cmake_target_include_directories,
-            'target_link_libraries': self._cmake_target_link_libraries,
-            'target_link_options': self._cmake_target_link_options,
-            'add_dependencies': self._cmake_add_dependencies,
-        }
-
         # Primary pass -- parse everything
         for l in lexer1:
+            # store the function if its execution should be delayed
+            if l.func in self.delayed_commands:
+                self.stored_commands += [l]
+                continue
+
             # "Execute" the CMake function if supported
-            fn = functions.get(l.func, None)
+            fn = self.functions.get(l.func, None)
             if(fn):
                 fn(l)
 
@@ -160,6 +179,12 @@ class CMakeTraceParser:
 
         return []
 
+    def var_to_str(self, var: str) -> T.Optional[str]:
+        if var in self.vars and self.vars[var]:
+            return self.vars[var][0]
+
+        return None
+
     def var_to_bool(self, var):
         if var not in self.vars:
             return False
@@ -300,7 +325,7 @@ class CMakeTraceParser:
         target = CMakeGeneratorTarget(name)
 
         def handle_output(key: str, target: CMakeGeneratorTarget) -> None:
-            target.outputs += [key]
+            target.outputs += key.split(';')
 
         def handle_command(key: str, target: CMakeGeneratorTarget) -> None:
             if key == 'ARGS':
@@ -308,7 +333,7 @@ class CMakeTraceParser:
             target.command[-1] += key.split(';')
 
         def handle_depends(key: str, target: CMakeGeneratorTarget) -> None:
-            target.depends += [key]
+            target.depends += key.split(';')
 
         def handle_working_dir(key: str, target: CMakeGeneratorTarget) -> None:
             if target.working_dir is None:
@@ -337,6 +362,8 @@ class CMakeTraceParser:
             if fn is not None:
                 fn(i, target)
 
+        target.current_bin_dir = self.var_to_str('MESON_PS_CMAKE_CURRENT_BINARY_DIR')
+        target.current_src_dir = self.var_to_str('MESON_PS_CMAKE_CURRENT_SOURCE_DIR')
         target.outputs = self._guess_files(target.outputs)
         target.depends = self._guess_files(target.depends)
         target.command = [self._guess_files(x) for x in target.command]
@@ -465,7 +492,8 @@ class CMakeTraceParser:
         if not target:
             return self._gen_exception('add_dependencies', 'target not found', tline)
 
-        target.depends += args[1:]
+        for i in args[1:]:
+            target.depends += i.split(';')
 
     def _cmake_target_compile_definitions(self, tline: CMakeTraceLine) -> None:
         # DOC: https://cmake.org/cmake/help/latest/command/target_compile_definitions.html
@@ -531,6 +559,18 @@ class CMakeTraceParser:
 
             self.targets[target].properties[i[0]] += i[1]
 
+    def _meson_ps_execute_delayed_calls(self, tline: CMakeTraceLine) -> None:
+        for l in self.stored_commands:
+            fn = self.functions.get(l.func, None)
+            if(fn):
+                fn(l)
+
+        # clear the stored commands
+        self.stored_commands = []
+
+    def _meson_ps_reload_vars(self, tline: CMakeTraceLine) -> None:
+        self.delayed_commands = self.get_cmake_var('MESON_PS_DELAYED_CALLS')
+
     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)

setup.py

@@ -39,7 +39,7 @@ packages = ['mesonbuild',
             'mesonbuild.wrap']
 package_data = {
     'mesonbuild.dependencies': ['data/CMakeLists.txt', 'data/CMakeListsLLVM.txt', 'data/CMakePathInfo.txt'],
-    'mesonbuild.cmake': ['data/run_ctgt.py'],
+    'mesonbuild.cmake': ['data/run_ctgt.py', 'data/preload.cmake'],
 }
 data_files = []
 if sys.platform != 'win32':

test cases/cmake/8 custom command/subprojects/cmMod/CMakeLists.txt

@@ -22,7 +22,7 @@ add_custom_command(
   COMMAND mycpy cpyBase.cpp.in                               cpyBase.cpp.something
   COMMAND mycpy cpyBase.cpp.something                        cpyBase.cpp.IAmRunningOutOfIdeas
   COMMAND mycpy cpyBase.cpp.IAmRunningOutOfIdeas             cpyBase.cpp
-  DEPENDS cpyBase.cpp.am gen
+  DEPENDS cpyBase.cpp.am;gen
 )
 
 add_custom_command(
@@ -89,7 +89,29 @@ add_custom_command(
   WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/s2_a_cpp"
 )
 
-add_library(cmModLib SHARED cmMod.cpp genTest.cpp cpyBase.cpp cpyBase.hpp cpyNext.cpp cpyNext.hpp)
+# cpyTest (copy file without renaming)
+add_custom_command(
+  OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/cpyTest.hpp"
+  COMMAND mycpy "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest/cpyTest.hpp" "${CMAKE_CURRENT_BINARY_DIR}/cpyTest.hpp"
+  DEPENDS "cpyTest/cpyTest.hpp"
+)
+
+add_custom_command(
+  OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/cpyTest2.hpp"
+  COMMAND mycpy "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest/cpyTest2.hpp" "${CMAKE_CURRENT_BINARY_DIR}/cpyTest2.hpp"
+  DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest/cpyTest2.hpp"
+)
+
+add_custom_command(
+  OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/cpyTest3.hpp"
+  COMMAND mycpy cpyTest3.hpp "${CMAKE_CURRENT_BINARY_DIR}/cpyTest3.hpp"
+  DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest/cpyTest3.hpp"
+  WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest"
+)
+
+add_subdirectory(cpyTest ccppyyTTeesstt)
+
+add_library(cmModLib SHARED cmMod.cpp genTest.cpp cpyBase.cpp cpyBase.hpp cpyNext.cpp cpyNext.hpp cpyTest.cpp cpyTest.hpp cpyTest2.hpp cpyTest3.hpp)
 include(GenerateExportHeader)
 generate_export_header(cmModLib)
 
@@ -99,9 +121,9 @@ set(ARGS_TEST ${ARGS_TEST} arg2)
 add_executable(macro_name macro_name.cpp)
 add_executable(args_test args_test.cpp)
 add_custom_target(args_test_cmd
-  COMMAND args_test ARGS ${ARGS_TEST}
+  COMMAND args_test ${ARGS_TEST}
 )
 add_custom_target(macro_name_cmd COMMAND macro_name)
 
-add_dependencies(cmModLib args_test_cmd)
-add_dependencies(args_test_cmd macro_name_cmd)
+add_dependencies(cmModLib args_test_cmd tgtCpyTest4)
+add_dependencies(args_test_cmd macro_name_cmd;gen;mycpy)

test cases/cmake/8 custom command/subprojects/cmMod/cmMod.cpp

@@ -2,6 +2,7 @@
 #include "genTest.hpp"
 #include "cpyBase.hpp"
 #include "cpyNext.hpp"
+#include "cpyTest.hpp"
 #include "cmModLib.hpp"
 
 #ifndef FOO
@@ -19,5 +20,5 @@ string cmModClass::getStr() const {
 }
 
 string cmModClass::getOther() const {
-  return "Srings:\n - " + getStrCpy() + "\n - " + getStrNext();
+  return "Srings:\n - " + getStrCpy() + "\n - " + getStrNext() + "\n - " + getStrCpyTest();
 }

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest.cpp

@@ -0,0 +1,8 @@
+#include "cpyTest.hpp"
+#include "cpyTest2.hpp"
+#include "cpyTest3.hpp"
+#include "ccppyyTTeesstt/cpyTest4.hpp"
+
+std::string getStrCpyTest() {
+  return CPY_TEST_STR_2 CPY_TEST_STR_3 CPY_TEST_STR_4;
+}

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest/CMakeLists.txt

@@ -0,0 +1,7 @@
+add_custom_command(
+  OUTPUT cpyTest4.hpp
+  COMMAND mycpy "${CMAKE_CURRENT_SOURCE_DIR}/cpyTest4.hpp" cpyTest4.hpp
+  DEPENDS cpyTest4.hpp
+)
+
+add_custom_target(tgtCpyTest4 DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/cpyTest4.hpp")

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest/cpyTest.hpp

@@ -0,0 +1,5 @@
+#pragma once
+
+#include <string>
+
+std::string getStrCpyTest();

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest/cpyTest2.hpp

@@ -0,0 +1,3 @@
+#pragma once
+
+#define CPY_TEST_STR_2 "Hello "

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest/cpyTest3.hpp

@@ -0,0 +1,3 @@
+#pragma once
+
+#define CPY_TEST_STR_3 "CopyFile"

test cases/cmake/8 custom command/subprojects/cmMod/cpyTest/cpyTest4.hpp

@@ -0,0 +1,3 @@
+#pragma once
+
+#define CPY_TEST_STR_4 " test"