From 8834cb9a5e8c94b817d657fff15a4bb6b44ee278 Mon Sep 17 00:00:00 2001 From: Alexey Lapshin Date: Fri, 14 Aug 2026 18:46:32 +0700 Subject: [PATCH] fix(cmake): detect toolchain response-file recursion via path prefix Skip only @response-file flags under IDF_TOOLCHAIN_BUILD_DIR using quote-aware parse and cmake_path(IS_PREFIX). Cache the toolchain build dir as REALPATH so prefix checks compare against a stable path spelling. --- tools/cmake/toolchain.cmake | 8 +++- tools/cmake/toolchain_flags.cmake | 17 +++++-- tools/test_build_system/test_cmake.py | 65 +++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/tools/cmake/toolchain.cmake b/tools/cmake/toolchain.cmake index 7ab605cd856..efc1ea17aab 100644 --- a/tools/cmake/toolchain.cmake +++ b/tools/cmake/toolchain.cmake @@ -66,7 +66,12 @@ endif() # directory as the toolchain file. We simply set IDF_TOOLCHAIN_BUILD_DIR to # point to that existing directory. if(_idf_toolchain_dir STREQUAL _current_toolchain_dir) - set(IDF_TOOLCHAIN_BUILD_DIR "${CMAKE_BINARY_DIR}/toolchain" + # Create then REALPATH so cached value matches symlink-resolved spelling used + # later when comparing @response-file paths (see toolchain_flags.cmake). + set(_idf_toolchain_build_dir "${CMAKE_BINARY_DIR}/toolchain") + file(MAKE_DIRECTORY "${_idf_toolchain_build_dir}") + file(REAL_PATH "${_idf_toolchain_build_dir}" _idf_toolchain_build_dir) + set(IDF_TOOLCHAIN_BUILD_DIR "${_idf_toolchain_build_dir}" CACHE PATH "Path to toolchain build directory containing response files and toolchain file copy" FORCE) # Copy toolchain file into the build directory and update CMAKE_TOOLCHAIN_FILE @@ -74,7 +79,6 @@ if(_idf_toolchain_dir STREQUAL _current_toolchain_dir) # CMAKE_BINARY_DIR values between base IDF-project builds and external projects. # For external project builds, compiler response files are located in the same # directory as CMAKE_TOOLCHAIN_FILE, making them easy to find. - file(MAKE_DIRECTORY "${IDF_TOOLCHAIN_BUILD_DIR}") file(COPY "${CMAKE_TOOLCHAIN_FILE}" DESTINATION "${IDF_TOOLCHAIN_BUILD_DIR}") set(CMAKE_TOOLCHAIN_FILE "${IDF_TOOLCHAIN_BUILD_DIR}/${_toolchain_filename}") diff --git a/tools/cmake/toolchain_flags.cmake b/tools/cmake/toolchain_flags.cmake index cd273040fdb..6d88612c2f6 100644 --- a/tools/cmake/toolchain_flags.cmake +++ b/tools/cmake/toolchain_flags.cmake @@ -112,10 +112,19 @@ function(_add_flags_to_files files flags) endforeach() foreach(flag ${flags}) - # Skip flags that contain IDF_TOOLCHAIN_BUILD_DIR substring - # to avoid recursion - string(FIND "${flag}" "${IDF_TOOLCHAIN_BUILD_DIR}" found_pos) - if(found_pos EQUAL -1) + # Skip @response-file refs into IDF_TOOLCHAIN_BUILD_DIR (avoid recursion). + # IDF emits @"path", but @path and @'path' are also acceptable. + set(_skip_flag FALSE) + if(flag MATCHES "^@") + # Drop leading @ so remaining token is a path (possibly quoted). + string(SUBSTRING "${flag}" 1 -1 _resp_path) + # Unwrap "..." / '...' (shell rules); leave bare paths unchanged. + separate_arguments(_resp_path UNIX_COMMAND "${_resp_path}") + # Paranoid: flag already should use IDF_TOOLCHAIN_BUILD_DIR spelling. + file(REAL_PATH "${_resp_path}" _resp_path) + cmake_path(IS_PREFIX IDF_TOOLCHAIN_BUILD_DIR "${_resp_path}" _skip_flag) + endif() + if(NOT _skip_flag) file(APPEND "${file_path}" "${flag}\n") endif() endforeach() diff --git a/tools/test_build_system/test_cmake.py b/tools/test_build_system/test_cmake.py index ba10257d168..1413424d436 100644 --- a/tools/test_build_system/test_cmake.py +++ b/tools/test_build_system/test_cmake.py @@ -5,6 +5,7 @@ import logging import os import re import shutil +import subprocess import sys from pathlib import Path @@ -91,6 +92,70 @@ def test_build_cmake_library_with_toolchain_flags(test_app_copy: Path, request: ) +def _run_cmake_from_dir(cmd: list[str], cwd: Path) -> None: + logging.debug('running {} in {}'.format(' '.join(cmd), cwd)) + try: + subprocess.run( + cmd, + cwd=cwd, + check=True, + capture_output=True, + text=True, + encoding='utf-8', + errors='backslashreplace', + ) + except subprocess.CalledProcessError as e: + logging.error('The following cmake command has failed: {}'.format(' '.join(cmd))) + logging.error(f'Working directory: {cwd}') + logging.error(f'Stdout: {e.stdout}') + logging.error(f'Stderr: {e.stderr}') + raise + + +@pytest.mark.skipif( + sys.platform == 'win32', + reason='Symlink build directories are not exercised on Windows CI runners', +) +def test_build_cmake_library_symlink_build_dir(test_app_copy: Path, request: pytest.FixtureRequest) -> None: + """Reconfigure import_lib with CMAKE_BINARY_DIR as realpath then as a symlink. + + First configure uses -B so CMAKE_C_FLAGS records + @/toolchain/cflags. Second uses -B so + IDF_TOOLCHAIN_BUILD_DIR keeps the symlink spelling. Without REALPATH + normalization, add_flags does not recognize the cached @ref and writes it + into the response file (gcc then recurses). + + import_lib's ExternalProject also passes -DCMAKE_TOOLCHAIN_FILE, which is + the same path-mismatch surface as a nested IDF/external cmake. + """ + logging.info('Configuring import_lib with realpath then symlink build dir') + idf_path = Path(os.environ['IDF_PATH']) + is_buildv2 = request.config.getoption('buildv2', False) + if is_buildv2: + import_lib_path = idf_path / 'examples' / 'build_system' / 'cmakev2' / 'features' / 'import_lib' + else: + import_lib_path = idf_path / 'examples' / 'build_system' / 'cmake' / 'import_lib' + + real_build = test_app_copy / 'build_real' + real_build.mkdir() + link_build = test_app_copy / 'build' + link_build.symlink_to(real_build.resolve(), target_is_directory=True) + + # Do not use run_cmake() as cwd: chdir into a symlink makes Linux getcwd() + # return the physical path, which hides the second-configure spelling. + cmake_base = ['cmake', '-G', 'Ninja', '-S', str(import_lib_path)] + _run_cmake_from_dir(cmake_base + ['-B', str(real_build)], test_app_copy) + _run_cmake_from_dir(cmake_base + ['-B', str(link_build)], test_app_copy) + + # Response files must not contain @refs (recursion into toolchain response files). + toolchain_dir = link_build / 'toolchain' + for resp_name in ('cflags', 'cxxflags', 'asmflags', 'ldflags'): + resp_file = toolchain_dir / resp_name + assert resp_file.is_file(), f'missing response file {resp_file}' + for line in resp_file.read_text(encoding='utf-8').splitlines(): + assert not line.startswith('@'), f'recursive @response-file ref in {resp_name}: {line}' + + def check_flag_in_compile_commands(build_dir: Path, flag_to_find: str) -> None: with open(build_dir / 'build' / 'compile_commands.json', encoding='utf-8') as f: compile_commands = json.load(f)