From a558ad506e8ae8999e1c37621648638419c5139b Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Mon, 6 Mar 2023 13:52:29 +0100 Subject: [PATCH] tools: fix custom sdkconfig renaming in set-target Currently the set-target has sdkconfig file name hardcoded to the default one and doesn't honor custom config paths or names. IMHO the only place where we can really now the config file name is in cmake. But also the config should be really renamed only if the set-target action is running. This moves the config file renaming into cmake and it's performed only when _IDF_PY_SET_TARGET_ACTION env. var. is set to 'ON'. This should hopefully guarantee that it's really renamed only while set-target is running. Signed-off-by: Frantisek Hrbata --- tools/cmake/project.cmake | 29 ++++++++++++++++++++--------- tools/cmake/targets.cmake | 10 ++-------- tools/idf_py_actions/core_ext.py | 11 +++-------- tools/idf_py_actions/tools.py | 5 +++-- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/tools/cmake/project.cmake b/tools/cmake/project.cmake index 00d94c8a6a..469a573488 100644 --- a/tools/cmake/project.cmake +++ b/tools/cmake/project.cmake @@ -1,12 +1,29 @@ # Designed to be included from an IDF app's CMakeLists.txt file cmake_minimum_required(VERSION 3.16) +# Get the currently selected sdkconfig file early, so this doesn't +# have to be done multiple times on different places. +if(SDKCONFIG) + get_filename_component(sdkconfig "${SDKCONFIG}" ABSOLUTE) +else() + set(sdkconfig "${CMAKE_SOURCE_DIR}/sdkconfig") +endif() + +# Check if the cmake was started as part of the set-target action. +# If so, check for existing sdkconfig file and rename it. +# This is done before __target_init, so the existing IDF_TARGET from sdkconfig +# is not considered for consistence checking. +if("$ENV{_IDF_PY_SET_TARGET_ACTION}" EQUAL "1" AND EXISTS "${sdkconfig}") + file(RENAME "${sdkconfig}" "${sdkconfig}.old") + message(STATUS "Existing sdkconfig '${sdkconfig}' renamed to '${sdkconfig}.old'.") +endif() + include(${CMAKE_CURRENT_LIST_DIR}/targets.cmake) # Initialize build target for this build using the environment variable or # value passed externally. -__target_init() +__target_init("${sdkconfig}") -# The mere inclusion of this CMake file sets up some interal build properties. +# The mere inclusion of this CMake file sets up some internal build properties. # These properties can be modified in between this inclusion the the idf_build_process # call. include(${CMAKE_CURRENT_LIST_DIR}/idf.cmake) @@ -316,7 +333,7 @@ function(__project_init components_var test_components_var) set(${test_components_var} "${test_components}" PARENT_SCOPE) endfunction() -# Trick to temporarily redefine project(). When functions are overriden in CMake, the originals can still be accessed +# Trick to temporarily redefine project(). When functions are overridden in CMake, the originals can still be accessed # using an underscore prefixed function of the same name. The following lines make sure that __project calls # the original project(). See https://cmake.org/pipermail/cmake/2015-October/061751.html. function(project) @@ -431,12 +448,6 @@ macro(project project_name) list(APPEND sdkconfig_defaults ${sdkconfig_default}) endforeach() - if(SDKCONFIG) - get_filename_component(sdkconfig "${SDKCONFIG}" ABSOLUTE) - else() - set(sdkconfig "${CMAKE_CURRENT_LIST_DIR}/sdkconfig") - endif() - if(BUILD_DIR) get_filename_component(build_dir "${BUILD_DIR}" ABSOLUTE) if(NOT EXISTS "${build_dir}") diff --git a/tools/cmake/targets.cmake b/tools/cmake/targets.cmake index 75b466f34c..e4c3f195b1 100644 --- a/tools/cmake/targets.cmake +++ b/tools/cmake/targets.cmake @@ -70,7 +70,7 @@ endfunction() # # Set the target used for the standard project build. # -macro(__target_init) +macro(__target_init config_file) # Input is IDF_TARGET environement variable set(env_idf_target $ENV{IDF_TARGET}) @@ -101,14 +101,8 @@ macro(__target_init) endif() endif() - if(SDKCONFIG) - get_filename_component(sdkconfig "${SDKCONFIG}" ABSOLUTE) - else() - set(sdkconfig "${CMAKE_SOURCE_DIR}/sdkconfig") - endif() - # Check if selected target is consistent with sdkconfig - __target_from_config("${sdkconfig}" sdkconfig_target where) + __target_from_config("${config_file}" sdkconfig_target where) if(sdkconfig_target) if(NOT ${sdkconfig_target} STREQUAL ${env_idf_target}) message(FATAL_ERROR " Target '${sdkconfig_target}' in sdkconfig '${where}'" diff --git a/tools/idf_py_actions/core_ext.py b/tools/idf_py_actions/core_ext.py index 7fb66556ba..3068130d64 100644 --- a/tools/idf_py_actions/core_ext.py +++ b/tools/idf_py_actions/core_ext.py @@ -155,14 +155,9 @@ def action_extensions(base_actions: Dict, project_path: str) -> Any: "%s is still in preview. You have to append '--preview' option after idf.py to use any preview feature." % idf_target) args.define_cache_entry.append('IDF_TARGET=' + idf_target) - sdkconfig_path = os.path.join(args.project_dir, 'sdkconfig') - sdkconfig_old = sdkconfig_path + '.old' - if os.path.exists(sdkconfig_old): - os.remove(sdkconfig_old) - if os.path.exists(sdkconfig_path): - os.rename(sdkconfig_path, sdkconfig_old) - print('Set Target to: %s, new sdkconfig created. Existing sdkconfig renamed to sdkconfig.old.' % idf_target) - ensure_build_directory(args, ctx.info_name, True) + print(f'Set Target to: {idf_target}, new sdkconfig will be created.') + env = {'_IDF_PY_SET_TARGET_ACTION': '1'} + ensure_build_directory(args, ctx.info_name, True, env) def reconfigure(action: str, ctx: Context, args: PropertyDict) -> None: ensure_build_directory(args, ctx.info_name, True) diff --git a/tools/idf_py_actions/tools.py b/tools/idf_py_actions/tools.py index 10fdd89b64..f2c546f8fa 100644 --- a/tools/idf_py_actions/tools.py +++ b/tools/idf_py_actions/tools.py @@ -419,7 +419,8 @@ def _detect_cmake_generator(prog_name: str) -> Any: raise FatalError("To use %s, either the 'ninja' or 'GNU make' build tool must be available in the PATH" % prog_name) -def ensure_build_directory(args: 'PropertyDict', prog_name: str, always_run_cmake: bool=False) -> None: +def ensure_build_directory(args: 'PropertyDict', prog_name: str, always_run_cmake: bool=False, + env: Dict=None) -> None: """Check the build directory exists and that cmake has been run there. If this isn't the case, create the build directory (if necessary) and @@ -480,7 +481,7 @@ def ensure_build_directory(args: 'PropertyDict', prog_name: str, always_run_cmak cmake_args += [project_dir] hints = not args.no_hints - RunTool('cmake', cmake_args, cwd=args.build_dir, hints=hints)() + RunTool('cmake', cmake_args, cwd=args.build_dir, env=env, hints=hints)() except Exception: # don't allow partially valid CMakeCache.txt files, # to keep the "should I run cmake?" logic simple