Merge branch 'bugfix/build_spaces_in_path_kconfig_ldgen_esptool' into 'master'

kconfig, ldgen, esptool, cmake: changes to support spaces in paths

See merge request espressif/esp-idf!15429

.gitlab/ci/host-test.yml

@@ -339,6 +339,12 @@ test_detect_python:
     - "zsh -c '. tools/detect_python.sh && echo Our Python: ${ESP_PYTHON?Python is not set}'"
     - "fish -c 'source tools/detect_python.fish && echo Our Python: $ESP_PYTHON'"
 
+test_split_path_by_spaces:
+  extends: .host_test_template
+  script:
+    - cd ${IDF_PATH}/tools
+    - python -m unittest split_paths_by_spaces.py
+
 test_nvs_page:
   extends: .host_test_template
   script:

components/bootloader/subproject/CMakeLists.txt

@@ -37,7 +37,9 @@ set(COMPONENTS
 # Make EXTRA_COMPONENT_DIRS variable to point to the bootloader_components directory
 # of the project being compiled
 set(PROJECT_EXTRA_COMPONENTS "${PROJECT_SOURCE_DIR}/bootloader_components")
-list(APPEND EXTRA_COMPONENT_DIRS "${PROJECT_EXTRA_COMPONENTS}")
+if(EXISTS ${PROJECT_EXTRA_COMPONENTS})
+    list(APPEND EXTRA_COMPONENT_DIRS "${PROJECT_EXTRA_COMPONENTS}")
+endif()
 
 # Consider each directory in project's bootloader_components as a component to be compiled
 file(GLOB proj_components RELATIVE ${PROJECT_EXTRA_COMPONENTS} ${PROJECT_EXTRA_COMPONENTS}/*)

components/esptool_py/project_include.cmake

@@ -148,23 +148,25 @@ endif()
 
 add_custom_target(erase_flash
     COMMAND ${CMAKE_COMMAND}
-    -D IDF_PATH="${idf_path}"
+    -D "IDF_PATH=${idf_path}"
-    -D SERIAL_TOOL="${ESPTOOLPY}"
+    -D "SERIAL_TOOL=${ESPTOOLPY}"
-    -D SERIAL_TOOL_ARGS="erase_flash"
+    -D "SERIAL_TOOL_ARGS=erase_flash"
     -P run_serial_tool.cmake
     WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
     USES_TERMINAL
+    VERBATIM
     )
 
 add_custom_target(monitor
     COMMAND ${CMAKE_COMMAND}
-    -D IDF_PATH="${idf_path}"
+    -D "IDF_PATH=${idf_path}"
-    -D SERIAL_TOOL="${ESPMONITOR}"
+    -D "SERIAL_TOOL=${ESPMONITOR}"
-    -D SERIAL_TOOL_ARGS="--target;${target};${monitor_rev_args};${elf_dir}/${elf}"
+    -D "SERIAL_TOOL_ARGS=--target;${target};${monitor_rev_args};${elf_dir}/${elf}"
-    -D WORKING_DIRECTORY="${build_dir}"
+    -D "WORKING_DIRECTORY=${build_dir}"
     -P run_serial_tool.cmake
     WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
     USES_TERMINAL
+    VERBATIM
     )
 
 set(esptool_flash_main_args "--before=${CONFIG_ESPTOOLPY_BEFORE}")
@@ -347,13 +349,14 @@ function(esptool_py_flash_target target_name main_args sub_args)
 
     add_custom_target(${target_name}
         COMMAND ${CMAKE_COMMAND}
-        -D IDF_PATH="${idf_path}"
+        -D "IDF_PATH=${idf_path}"
-        -D SERIAL_TOOL="${ESPTOOLPY}"
+        -D "SERIAL_TOOL=${ESPTOOLPY}"
-        -D SERIAL_TOOL_ARGS="${main_args};write_flash;@${target_name}_args"
+        -D "SERIAL_TOOL_ARGS=${main_args};write_flash;@${target_name}_args"
-        -D WORKING_DIRECTORY="${build_dir}"
+        -D "WORKING_DIRECTORY=${build_dir}"
         -P ${esptool_py_dir}/run_serial_tool.cmake
         WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
         USES_TERMINAL
+        VERBATIM
         )
 
     set_target_properties(${target_name} PROPERTIES SUB_ARGS "${sub_args}")
@@ -388,13 +391,14 @@ $<JOIN:$<TARGET_PROPERTY:${target_name},IMAGES>,\n>")
     if(${encrypted})
         add_custom_target(encrypted-${target_name}
             COMMAND ${CMAKE_COMMAND}
-            -D IDF_PATH="${idf_path}"
+            -D "IDF_PATH=${idf_path}"
-            -D SERIAL_TOOL="${ESPTOOLPY}"
+            -D "SERIAL_TOOL=${ESPTOOLPY}"
-            -D SERIAL_TOOL_ARGS="${main_args};write_flash;@encrypted_${target_name}_args"
+            -D "SERIAL_TOOL_ARGS=${main_args};write_flash;@encrypted_${target_name}_args"
-            -D WORKING_DIRECTORY="${build_dir}"
+            -D "WORKING_DIRECTORY=${build_dir}"
             -P ${esptool_py_dir}/run_serial_tool.cmake
             WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
             USES_TERMINAL
+            VERBATIM
             )
 
         # Generate the parameters for esptool.py command

components/esptool_py/run_serial_tool.cmake

@@ -22,6 +22,8 @@ if(DEFINED ENV{IDF_ENV_FPGA})
     message("Note: IDF_ENV_FPGA is set, propagating to esptool with ESPTOOL_ENV_FPGA = 1")
 endif()
 
+set(serial_tool_cmd ${SERIAL_TOOL})
+
 # Main purpose of this script: we can't expand these environment variables in the main IDF CMake build,
 # because we want to expand them at flashing time not at CMake runtime (so they can change
 # without needing a CMake re-run)
@@ -30,7 +32,7 @@ if(NOT ESPPORT)
     message("Note: ${SERIAL_TOOL} will search for a serial port. "
             "To specify a port, set the ESPPORT environment variable.")
 else()
-    set(port_arg "-p ${ESPPORT}")
+    list(APPEND serial_tool_cmd -p ${ESPPORT})
 endif()
 
 set(ESPBAUD $ENV{ESPBAUD})
@@ -38,13 +40,10 @@ if(NOT ESPBAUD)
     message("Note: ${SERIAL_TOOL} will attempt to set baud rate automatically. "
             "To specify a baud rate, set the ESPBAUD environment variable.")
 else()
-    set(baud_arg "-b ${ESPBAUD}")
+    list(APPEND serial_tool_cmd -b ${ESPBAUD})
 endif()
 
-set(serial_tool_cmd "${SERIAL_TOOL} ${port_arg} ${baud_arg} ${SERIAL_TOOL_ARGS}")
+list(APPEND serial_tool_cmd ${SERIAL_TOOL_ARGS})
-
-include("${IDF_PATH}/tools/cmake/utilities.cmake")
-spaces2list(serial_tool_cmd)
 
 execute_process(COMMAND ${serial_tool_cmd}
     WORKING_DIRECTORY "${WORKING_DIRECTORY}"

tools/ci/check_copyright_ignore.txt

@@ -3182,7 +3182,6 @@ tools/idf_py_actions/tools.py
 tools/idf_py_actions/uf2_ext.py
 tools/kconfig_new/confserver.py
 tools/kconfig_new/gen_kconfig_doc.py
-tools/kconfig_new/prepare_kconfig_files.py
 tools/kconfig_new/test/confgen/test_confgen.py
 tools/kconfig_new/test/confserver/test_confserver.py
 tools/kconfig_new/test/gen_kconfig_doc/test_kconfig_out.py

tools/ci/test_build_system_cmake.sh

@@ -468,6 +468,17 @@ function run_tests()
     mv main/main/main/* main
     rm -rf main/main
 
+    print_status "Non-existent paths in EXTRA_COMPONENT_DIRS are not allowed"
+    clean_build_dir
+    ! idf.py -DEXTRA_COMPONENT_DIRS="extra_components" reconfigure || failure "Build should fail when non-existent component path is added"
+
+    print_status "Component names may contain spaces"
+    clean_build_dir
+    mkdir -p "extra component"
+    echo "idf_component_register" > "extra component/CMakeLists.txt"
+    idf.py -DEXTRA_COMPONENT_DIRS="extra component;main" || failure "Build should succeed when a component name contains space"
+    rm -rf "extra component"
+
     print_status "sdkconfig should have contents of all files: sdkconfig, sdkconfig.defaults, sdkconfig.defaults.IDF_TARGET"
     idf.py clean > /dev/null
     idf.py fullclean > /dev/null
@@ -662,9 +673,10 @@ endmenu\n" >> ${IDF_PATH}/Kconfig
 
     # idf.py subcommand options, (using monitor with as example)
     print_status "Can set options to subcommands: print_filter for monitor"
+    clean_build_dir
     mv ${IDF_PATH}/tools/idf_monitor.py ${IDF_PATH}/tools/idf_monitor.py.tmp
     echo "import sys;print(sys.argv[1:])" > ${IDF_PATH}/tools/idf_monitor.py
-    idf.py build || "Failed to build project"
+    idf.py build || failure "Failed to build project"
     idf.py monitor --print-filter="*:I" -p tty.fake | grep "'--print_filter', '\*:I'" || failure "It should process options for subcommands (and pass print-filter to idf_monitor.py)"
     mv ${IDF_PATH}/tools/idf_monitor.py.tmp ${IDF_PATH}/tools/idf_monitor.py
 

tools/cmake/kconfig.cmake

@@ -94,10 +94,6 @@ function(__kconfig_generate_config sdkconfig sdkconfig_defaults)
     idf_build_get_property(idf_path IDF_PATH)
     idf_build_get_property(idf_env_fpga __IDF_ENV_FPGA)
 
-    string(REPLACE ";" " " kconfigs "${kconfigs}")
-    string(REPLACE ";" " " kconfig_projbuilds "${kconfig_projbuilds}")
-    string(REPLACE ";" " " sdkconfig_renames "${sdkconfig_renames}")
-
     # These are the paths for files which will contain the generated "source" lines for COMPONENT_KCONFIGS and
     # COMPONENT_KCONFIGS_PROJBUILD
     set(kconfigs_projbuild_path "${CMAKE_CURRENT_BINARY_DIR}/kconfigs_projbuild.in")
@@ -130,10 +126,12 @@ function(__kconfig_generate_config sdkconfig sdkconfig_defaults)
 
     set(prepare_kconfig_files_command
         ${python} ${idf_path}/tools/kconfig_new/prepare_kconfig_files.py
+        --list-separator=semicolon
         --env-file ${config_env_path})
 
     set(confgen_basecommand
         ${python} ${idf_path}/tools/kconfig_new/confgen.py
+        --list-separator=semicolon
         --kconfig ${root_kconfig}
         --sdkconfig-rename ${root_sdkconfig_rename}
         --config ${sdkconfig}

tools/cmake/ldgen.cmake

@@ -46,35 +46,31 @@ function(__ldgen_process_template template output)
     # Create command to invoke the linker script generator tool.
     idf_build_get_property(sdkconfig SDKCONFIG)
     idf_build_get_property(root_kconfig __ROOT_KCONFIG)
-    idf_build_get_property(kconfigs KCONFIGS)
-    idf_build_get_property(kconfig_projbuilds KCONFIG_PROJBUILDS)
 
     idf_build_get_property(python PYTHON)
 
-    string(REPLACE ";" " " kconfigs "${kconfigs}")
-    string(REPLACE ";" " " kconfig_projbuilds "${kconfig_projbuilds}")
-
     idf_build_get_property(config_env_path CONFIG_ENV_PATH)
 
     if($ENV{LDGEN_CHECK_MAPPING})
         set(ldgen_check "--check-mapping"
-            "--check-mapping-exceptions=${idf_path}/tools/ci/check_ldgen_mapping_exceptions.txt")
+            "--check-mapping-exceptions" "${idf_path}/tools/ci/check_ldgen_mapping_exceptions.txt")
         message(STATUS "Mapping check enabled in ldgen")
     endif()
 
     add_custom_command(
         OUTPUT ${output}
-        COMMAND ${python} ${idf_path}/tools/ldgen/ldgen.py
+        COMMAND ${python} "${idf_path}/tools/ldgen/ldgen.py"
-        --config    ${sdkconfig}
+        --config    "${sdkconfig}"
-        --fragments "$<JOIN:${ldgen_fragment_files},\t>"
+        --fragments-list "${ldgen_fragment_files}"
-        --input     ${template}
+        --input     "${template}"
-        --output    ${output}
+        --output    "${output}"
-        --kconfig   ${root_kconfig}
+        --kconfig   "${root_kconfig}"
         --env-file  "${config_env_path}"
-        --libraries-file ${build_dir}/ldgen_libraries
+        --libraries-file "${build_dir}/ldgen_libraries"
-        --objdump   ${CMAKE_OBJDUMP}
+        --objdump   "${CMAKE_OBJDUMP}"
         ${ldgen_check}
         DEPENDS     ${template} ${ldgen_fragment_files} ${ldgen_depends} ${SDKCONFIG}
+        VERBATIM
     )
 
     get_filename_component(_name ${output} NAME)

tools/cmake/project.cmake

@@ -63,6 +63,38 @@ function(__project_get_revision var)
     set(${var} "${PROJECT_VER}" PARENT_SCOPE)
 endfunction()
 
+
+# paths_with_spaces_to_list
+#
+# Replacement for spaces2list in cases where it was previously used on
+# directory lists.
+#
+# If the variable doesn't contain spaces, (e.g. is already a CMake list)
+# then the variable is unchanged. Otherwise an external Python script is called
+# to try to split the paths, and the variable is updated with the result.
+#
+# This feature is added only for compatibility. Please do not introduce new
+# space separated path lists.
+#
+function(paths_with_spaces_to_list variable_name)
+    if("${${variable_name}}" MATCHES "[ \t]")
+        idf_build_get_property(python PYTHON)
+        idf_build_get_property(idf_path IDF_PATH)
+        execute_process(
+            COMMAND ${python}
+                "${idf_path}/tools/split_paths_by_spaces.py"
+                "--var-name=${variable_name}"
+                "${${variable_name}}"
+            WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}"
+            OUTPUT_VARIABLE result
+            RESULT_VARIABLE ret)
+        if(NOT ret EQUAL 0)
+            message(FATAL_ERROR "Failed to parse ${variable_name}, see diagnostics above")
+        endif()
+        set("${variable_name}" "${result}" PARENT_SCOPE)
+    endif()
+endfunction()
+
 #
 # Output the built components to the user. Generates files for invoking idf_monitor.py
 # that doubles as an overview of some of the more important build properties.
@@ -182,9 +214,13 @@ function(__project_init components_var test_components_var)
     # extra directories, etc. passed from the root CMakeLists.txt.
     if(COMPONENT_DIRS)
         # User wants to fully override where components are pulled from.
-        spaces2list(COMPONENT_DIRS)
+        paths_with_spaces_to_list(COMPONENT_DIRS)
         idf_build_set_property(__COMPONENT_TARGETS "")
         foreach(component_dir ${COMPONENT_DIRS})
+            get_filename_component(component_abs_path ${component_dir} ABSOLUTE)
+            if(NOT EXISTS ${component_abs_path})
+                message(FATAL_ERROR "Directory specified in COMPONENT_DIRS doesn't exist: ${component_abs_path}")
+            endif()
             __project_component_dir(${component_dir})
         endforeach()
     else()
@@ -192,8 +228,12 @@ function(__project_init components_var test_components_var)
             __project_component_dir("${CMAKE_CURRENT_LIST_DIR}/main")
         endif()
 
-        spaces2list(EXTRA_COMPONENT_DIRS)
+        paths_with_spaces_to_list(EXTRA_COMPONENT_DIRS)
         foreach(component_dir ${EXTRA_COMPONENT_DIRS})
+            get_filename_component(component_abs_path ${component_dir} ABSOLUTE)
+            if(NOT EXISTS ${component_abs_path})
+                message(FATAL_ERROR "Directory specified in EXTRA_COMPONENT_DIRS doesn't exist: ${component_abs_path}")
+            endif()
             __project_component_dir("${component_dir}")
         endforeach()
 

tools/cmake/utilities.cmake

@@ -22,8 +22,8 @@ endfunction()
 # Take a variable whose value was space-delimited values, convert to a cmake
 # list (semicolon-delimited)
 #
-# Note: if using this for directories, keeps the issue in place that
+# Note: do not use this for directories or full paths, as they may contain
-# directories can't contain spaces...
+# spaces.
 #
 # TODO: look at cmake separate_arguments, which is quote-aware
 function(spaces2list variable_name)

tools/kconfig_new/confgen.py

@@ -223,6 +223,10 @@ def main():
                         help='Optional file to load environment variables from. Contents '
                              'should be a JSON object where each key/value pair is a variable.')
 
+    parser.add_argument('--list-separator', choices=['space', 'semicolon'],
+                        default='space',
+                        help='Separator used in environment list variables (COMPONENT_SDKCONFIG_RENAMES)')
+
     args = parser.parse_args()
 
     for fmt, filename in args.output:
@@ -247,8 +251,12 @@ def main():
     config.warn_assign_redun = False
     config.warn_assign_override = False
 
+    sdkconfig_renames_sep = ';' if args.list_separator == 'semicolon' else ' '
+
     sdkconfig_renames = [args.sdkconfig_rename] if args.sdkconfig_rename else []
-    sdkconfig_renames += os.environ.get('COMPONENT_SDKCONFIG_RENAMES', '').split()
+    sdkconfig_renames_from_env = os.environ.get('COMPONENT_SDKCONFIG_RENAMES')
+    if sdkconfig_renames_from_env:
+        sdkconfig_renames += sdkconfig_renames_from_env.split(sdkconfig_renames_sep)
     deprecated_options = DeprecatedOptions(config.config_prefix, path_rename_files=sdkconfig_renames)
 
     if len(args.defaults) > 0:

tools/kconfig_new/confserver.py

@@ -74,7 +74,9 @@ def main():
 def run_server(kconfig, sdkconfig, sdkconfig_rename, default_version=MAX_PROTOCOL_VERSION):
     config = kconfiglib.Kconfig(kconfig)
     sdkconfig_renames = [sdkconfig_rename] if sdkconfig_rename else []
-    sdkconfig_renames += os.environ.get('COMPONENT_SDKCONFIG_RENAMES', '').split()
+    sdkconfig_renames_from_env = os.environ.get('COMPONENT_SDKCONFIG_RENAMES')
+    if sdkconfig_renames_from_env:
+        sdkconfig_renames += sdkconfig_renames_from_env.split(';')
     deprecated_options = confgen.DeprecatedOptions(config.config_prefix, path_rename_files=sdkconfig_renames)
     f_o = tempfile.NamedTemporaryFile(mode='w+b', delete=False)
     try:
@@ -157,8 +159,8 @@ def run_server(kconfig, sdkconfig, sdkconfig_rename, default_version=MAX_PROTOCO
             # V2+ response, separate visibility values
             response = {'version': req['version'], 'values': values_diff, 'ranges': ranges_diff, 'visible': visible_diff}
         if error:
-            for e in error:
+            for err in error:
-                print('Error: %s' % e, file=sys.stderr)
+                print('Error: %s' % err, file=sys.stderr)
             response['error'] = error
         json.dump(response, sys.stdout)
         print('\n')

tools/kconfig_new/prepare_kconfig_files.py

@@ -1,18 +1,7 @@
 #!/usr/bin/env python
 #
-# Copyright 2019 Espressif Systems (Shanghai) PTE LTD
+# SPDX-FileCopyrightText: 2019-2021 Espressif Systems (Shanghai) CO LTD
-#
+# SPDX-License-Identifier: Apache-2.0
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#     http:#www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
 
 from __future__ import print_function, unicode_literals
 
@@ -22,7 +11,7 @@ import sys
 from io import open
 
 
-def _prepare_source_files(env_dict):
+def _prepare_source_files(env_dict, list_separator):
     """
     Prepares source files which are sourced from the main Kconfig because upstream kconfiglib doesn't support sourcing
     a file list. The inputs are the same environment variables which are used by kconfiglib:
@@ -37,18 +26,27 @@ def _prepare_source_files(env_dict):
 
     After running this function, COMPONENT_KCONFIGS_SOURCE_FILE and COMPONENT_KCONFIGS_PROJBUILD_SOURCE_FILE will
     contain a list of source statements based on the content of COMPONENT_KCONFIGS and COMPONENT_KCONFIGS_PROJBUILD,
-    respectively. For example, if COMPONENT_KCONFIGS="var1 var2 var3" and
+    respectively. For example, if COMPONENT_KCONFIGS="var1;var2;var3" and
     COMPONENT_KCONFIGS_SOURCE_FILE="/path/file.txt" then the content of file /path/file.txt will be:
         source "var1"
         source "var2"
         source "var3"
+
+    The character used to delimit paths in COMPONENT_KCONFIGS* variables is determined based on
+    presence of 'IDF_CMAKE' variable in the env_dict.
+    GNU Make build system uses a space, CMake build system uses a semicolon.
     """
 
     def _dequote(var):
         return var[1:-1] if len(var) > 0 and (var[0], var[-1]) == ('"',) * 2 else var
 
     def _write_source_file(config_var, config_file):
-        new_content = '\n'.join(['source "{}"'.format(path) for path in _dequote(config_var).split()])
+        dequoted_var = _dequote(config_var)
+        if dequoted_var:
+            new_content = '\n'.join(['source "{}"'.format(path) for path in dequoted_var.split(list_separator)])
+        else:
+            new_content = ''
+
         try:
             with open(config_file, 'r', encoding='utf-8') as f:
                 old_content = f.read()
@@ -71,8 +69,7 @@ def _prepare_source_files(env_dict):
         sys.exit(1)
 
 
-if __name__ == '__main__':
+def main():
-
     parser = argparse.ArgumentParser(description='Kconfig Source File Generator')
 
     parser.add_argument('--env', action='append', default=[],
@@ -82,6 +79,10 @@ if __name__ == '__main__':
                         help='Optional file to load environment variables from. Contents '
                              'should be a JSON object where each key/value pair is a variable.')
 
+    parser.add_argument('--list-separator', choices=['space', 'semicolon'],
+                        default='space',
+                        help='Separator used in environment list variables (COMPONENT_KCONFIGS, COMPONENT_KCONFIGS_PROJBUILD)')
+
     args = parser.parse_args()
 
     try:
@@ -93,4 +94,10 @@ if __name__ == '__main__':
     if args.env_file is not None:
         env.update(json.load(args.env_file))
 
-    _prepare_source_files(env)
+    list_separator = ';' if args.list_separator == 'semicolon' else ' '
+
+    _prepare_source_files(env, list_separator)
+
+
+if __name__ == '__main__':
+    main()

tools/ldgen/ldgen.py

@@ -50,11 +50,20 @@ def main():
         help='Linker template file',
         type=argparse.FileType('r'))
 
-    argparser.add_argument(
+    fragments_group = argparser.add_mutually_exclusive_group()
+
+    fragments_group.add_argument(
         '--fragments', '-f',
         type=argparse.FileType('r'),
         help='Input fragment files',
-        nargs='+')
+        nargs='+'
+    )
+
+    fragments_group.add_argument(
+        '--fragments-list',
+        help='Input fragment files as a semicolon-separated list',
+        type=str
+    )
 
     argparser.add_argument(
         '--libraries-file',
@@ -102,13 +111,18 @@ def main():
     args = argparser.parse_args()
 
     input_file = args.input
-    fragment_files = [] if not args.fragments else args.fragments
     libraries_file = args.libraries_file
     config_file = args.config
     output_path = args.output
     kconfig_file = args.kconfig
     objdump = args.objdump
 
+    fragment_files = []
+    if args.fragments_list:
+        fragment_files = args.fragments_list.split(';')
+    elif args.fragments:
+        fragment_files = args.fragments
+
     check_mapping = args.check_mapping
     if args.check_mapping_exceptions:
         check_mapping_exceptions = [line.strip() for line in args.check_mapping_exceptions]

tools/split_paths_by_spaces.py

@@ -0,0 +1,333 @@
+#!/usr/bin/env python
+# coding=utf-8
+#
+# SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+# This script converts space-separated EXTRA_COMPONENT_DIRS and COMPONENT_DIRS
+# CMake variables into semicolon-separated lists.
+#
+# IDF versions <=v4.3 didn't support spaces in paths to ESP-IDF or projects.
+# Therefore it was okay to use spaces as separators in EXTRA_COMPONENT_DIRS,
+# same as it was done in the legacy GNU Make based build system.
+# CMake build system used 'spaces2list' function to convert space-separated
+# variables into semicolon-separated lists, replacing every space with a
+# semicolon.
+#
+# In IDF 4.4 and later, spaces in project path and ESP-IDF path are supported.
+# This means that EXTRA_COMPONENT_DIRS and COMPONENT_DIRS variables now should
+# be semicolon-separated CMake lists.
+#
+# To provide compatibility with the projects written for older ESP-IDF versions,
+# this script attempts to convert these space-separated variables into semicolon-
+# separated ones. Note that in general this cannot be done unambiguously, so this
+# script will still report an error if there are multiple ways to interpret the
+# variable, and ask the user to fix the project CMakeLists.txt file.
+#
+
+
+import argparse
+import os
+import pprint
+import sys
+import textwrap
+import typing
+import unittest
+
+
+class PathSplitError(RuntimeError):
+    pass
+
+
+def main() -> None:
+    parser = argparse.ArgumentParser()
+    parser.add_argument('--var-name', required=True, help='Name of CMake variable, for printing errors and warnings')
+    parser.add_argument('in_variable', help='Input variable, may contain a mix of spaces and semicolons as separators')
+    args = parser.parse_args()
+
+    # Initially split the paths by semicolons
+    semicolon_separated_parts = args.in_variable.split(';')
+
+    # Every resulting part may contain space separators. Handle each part:
+    paths = []
+    ctx = dict(warnings=False)
+    errors = False
+    for part in semicolon_separated_parts:
+        def warning_cb(warning_str: str) -> None:
+            print('\n  '.join(
+                textwrap.wrap('Warning: in CMake variable {}: {}'.format(args.var_name, warning_str), width=120,
+                              break_on_hyphens=False)), file=sys.stderr)
+            ctx['warnings'] = True
+
+        try:
+            paths += split_paths_by_spaces(part, warning_cb=warning_cb)
+        except PathSplitError as e:
+            print('\n  '.join(textwrap.wrap('Error: in CMake variable {}: {}'.format(args.var_name, str(e)), width=120,
+                                            break_on_hyphens=False)), file=sys.stderr)
+            errors = True
+
+    if errors or ctx['warnings']:
+        print(textwrap.dedent("""
+            Note: In ESP-IDF v4.4 and later, COMPONENT_DIRS and EXTRA_COMPONENT_DIRS should be defined
+                  as CMake lists, not as space separated strings.
+
+            Examples:
+                * set(EXTRA_COMPONENT_DIRS path/to/components path/to/more/components)
+                  # Correct, EXTRA_COMPONENT_DIRS is defined as a CMake list, with two paths added
+
+                * list(APPEND EXTRA_COMPONENT_DIRS path/to/component)
+                  list(APPEND EXTRA_COMPONENT_DIRS path/to/more/components)
+                  # Correct, use when building EXTRA_COMPONENT_DIRS incrementally
+
+                * set(EXTRA_COMPONENT_DIRS path/to/components "another/path with space/components")
+                  # Literal path with spaces has to be quoted
+
+                * set(EXTRA_COMPONENT_DIRS $ENV{MY_PATH}/components dir/more_components)
+                  # Correct, even if MY_PATH contains spaces
+
+                * set(EXTRA_COMPONENT_DIRS ${ROOT}/component1 ${ROOT}/component2 ${ROOT}/component3)
+                  # Correct, even if ROOT contains spaces
+
+            Avoid string concatenation!
+                set(EXTRA_COMPONENT_DIRS "${EXTRA_COMPONENT_DIRS} component1")
+                set(EXTRA_COMPONENT_DIRS "${EXTRA_COMPONENT_DIRS} component2")
+                # Incorrect. String "component1 component2" may indicate a single directory
+                # name with a space, or two directory names separated by space.
+
+            Instead use:
+                list(APPEND component1)
+                list(APPEND component2)
+
+            Defining COMPONENT_DIRS and EXTRA_COMPONENT_DIRS as CMake lists is backwards compatible
+            with ESP-IDF 4.3 and below.
+
+            (If you think these variables are defined correctly in your project and this message
+            is not relevant, please report this as an issue.)
+        """), file=sys.stderr)
+
+        print('Diagnostic info: {} was invoked in {} with arguments: {}'.format(
+            sys.argv[0], os.getcwd(), sys.argv[1:]
+        ), file=sys.stderr)
+
+    if errors:
+        raise SystemExit(1)
+
+    sys.stdout.write(';'.join(paths))
+    sys.stdout.flush()
+
+
+def split_paths_by_spaces(src: str, path_exists_cb: typing.Callable[[str], bool] = os.path.exists,
+                          warning_cb: typing.Optional[typing.Callable[[str], None]] = None) -> typing.List[str]:
+    if ' ' not in src:
+        # no spaces, complete string should be the path
+        return [src]
+
+    def path_exists_or_empty(path: str) -> bool:
+        return path == '' or path_exists_cb(path)
+
+    # remove leading and trailing spaces
+    delayed_warnings = []
+    trimmed = src.lstrip(' ')
+    if trimmed != src:
+        delayed_warnings.append("Path component '{}' contains leading spaces".format(src))
+    src = trimmed
+
+    trimmed = src.rstrip(' ')
+    if trimmed != src:
+        delayed_warnings.append("Path component '{}' contains trailing spaces".format(src))
+    src = trimmed
+
+    # Enumerate all possible ways to split the string src into paths by spaces.
+    # The number of these ways is equal to sum(C(n, k), 0<=k<n) == 2^n
+    #   (where n is the number of spaces, k is the number of splits, C(n, k) are binomial coefficients)
+    #
+    # We do this by associating every space with a bit of an integer in the range [0, 2^n - 1],
+    # such that when the bit is 0 there is no split in the given space, and bit is 1 when there is a split.
+
+    parts = src.split(' ')
+    num_spaces = len(parts) - 1
+    valid_ways_to_split = []
+    all_ways_to_split = [selective_join(parts, i) for i in range(2 ** num_spaces)]
+    for paths_list in all_ways_to_split:
+        nonempty_paths = list(filter(bool, paths_list))
+        if all(map(path_exists_or_empty, nonempty_paths)):
+            valid_ways_to_split.append(nonempty_paths)
+
+    num_candidates = len(valid_ways_to_split)
+    if num_candidates == 1:
+        # Success, found only one correct way to split.
+        result = valid_ways_to_split[0]
+
+        # Report warnings
+        if warning_cb:
+            if len(result) > 1:
+                warning_cb("Path component '{}' contains a space separator. It was automatically split into {}".format(
+                    src, pprint.pformat(result)
+                ))
+            for w in delayed_warnings:
+                warning_cb(w)
+
+        return result
+
+    if num_candidates == 0:
+        raise PathSplitError(("Didn't find a valid way to split path '{}'. "
+                              'This error may be reported if one or more paths '
+                              "are separated with spaces, and at least one path doesn't exist.").format(src))
+
+    # if num_candidates > 1
+    raise PathSplitError("Found more than one valid way to split path '{}':{}".format(
+        src, ''.join('\n\t- ' + pprint.pformat(p) for p in valid_ways_to_split)
+    ))
+
+
+def selective_join(parts: typing.List[str], n: int) -> typing.List[str]:
+    """
+    Given the list of N+1 strings, and an integer n in [0, 2**N - 1] range,
+    concatenate i-th and (i+1)-th string with space inbetween if bit i is not set in n.
+    Examples:
+         selective_join(['a', 'b', 'c'], 0b00) == ['a b c']
+         selective_join(['a', 'b', 'c'], 0b01) == ['a', 'b c']
+         selective_join(['a', 'b', 'c'], 0b10) == ['a b', 'c']
+         selective_join(['a', 'b', 'c'], 0b11) == ['a', 'b', 'c']
+
+    This function is used as part of finding all the ways to split a string by spaces.
+
+    :param parts: Strings to join
+    :param n: Integer (bit map) to set the positions to join
+    :return: resulting list of strings
+    """
+    result = []
+    concatenated = [parts[0]]
+    for part in parts[1:]:
+        if n & 1:
+            result.append(' '.join(concatenated))
+            concatenated = [part]
+        else:
+            concatenated.append(part)
+        n >>= 1
+    if concatenated:
+        result.append(' '.join(concatenated))
+    return result
+
+
+class HelperTests(unittest.TestCase):
+    def test_selective_join(self) -> None:
+        self.assertListEqual(['a b c'], selective_join(['a', 'b', 'c'], 0b00))
+        self.assertListEqual(['a', 'b c'], selective_join(['a', 'b', 'c'], 0b01))
+        self.assertListEqual(['a b', 'c'], selective_join(['a', 'b', 'c'], 0b10))
+        self.assertListEqual(['a', 'b', 'c'], selective_join(['a', 'b', 'c'], 0b11))
+
+
+class SplitTests(unittest.TestCase):
+    def test_split_paths_absolute(self) -> None:
+        self.check_paths_concatenated('/absolute/path/one', '/absolute/path/two')
+
+    def test_split_paths_absolute_spaces(self) -> None:
+        self.check_paths_concatenated('/absolute/path with spaces')
+        self.check_paths_concatenated('/absolute/path  with   more   spaces')
+        self.check_paths_concatenated('/absolute/path with spaces/one', '/absolute/path with spaces/two')
+
+        self.check_paths_concatenated('/absolute/path with spaces/one',
+                                      '/absolute/path with spaces/two',
+                                      '/absolute/path with spaces/three')
+
+    def test_split_paths_absolute_relative(self) -> None:
+        self.check_paths_concatenated('/absolute/path/one', 'two')
+
+    def test_split_paths_relative(self) -> None:
+        self.check_paths_concatenated('one', 'two')
+
+    def test_split_paths_absolute_spaces_relative(self) -> None:
+        self.check_paths_concatenated('/absolute/path with spaces/one', 'two')
+
+    def test_split_paths_ambiguous(self) -> None:
+        self.check_paths_concatenated_ambiguous('/absolute/path one', 'two',
+                                                additional_paths_exist=['/absolute/path', 'one'])
+
+        self.check_paths_concatenated_ambiguous('/path    ', '/path',
+                                                additional_paths_exist=['/path    /path'])
+
+    def test_split_paths_nonexistent(self) -> None:
+        self.check_paths_concatenated_nonexistent('one', 'two')
+
+    def test_split_paths_extra_whitespace(self) -> None:
+        paths = ['/path']
+        path_exists = self.path_exists_by_list(paths)
+        self.assertListEqual(paths, split_paths_by_spaces(' /path', path_exists_cb=path_exists))
+        self.assertListEqual(paths, split_paths_by_spaces('/path ', path_exists_cb=path_exists))
+        self.assertListEqual(paths + paths, split_paths_by_spaces('/path   /path', path_exists_cb=path_exists))
+
+    def test_split_paths_warnings(self) -> None:
+        paths = ['/path']
+        ctx = {'warnings': []}  # type: typing.Dict[str, typing.List[str]]
+
+        def add_warning(warning: str) -> None:
+            ctx['warnings'].append(warning)
+
+        path_exists = self.path_exists_by_list(paths)
+
+        self.assertListEqual(paths,
+                             split_paths_by_spaces(' /path', path_exists_cb=path_exists, warning_cb=add_warning))
+        self.assertEqual(1, len(ctx['warnings']))
+        self.assertIn('leading', ctx['warnings'][0])
+
+        ctx['warnings'] = []
+        self.assertListEqual(paths,
+                             split_paths_by_spaces('/path ', path_exists_cb=path_exists, warning_cb=add_warning))
+        self.assertEqual(1, len(ctx['warnings']))
+        self.assertIn('trailing', ctx['warnings'][0])
+
+        ctx['warnings'] = []
+        self.assertListEqual(paths + paths,
+                             split_paths_by_spaces('/path /path', path_exists_cb=path_exists, warning_cb=add_warning))
+        self.assertEqual(1, len(ctx['warnings']))
+        self.assertIn('contains a space separator', ctx['warnings'][0])
+
+    @staticmethod
+    def path_exists_by_list(paths_which_exist: typing.List[str]) -> typing.Callable[[str], bool]:
+        """
+        Returns a function to check whether a path exists, similar to os.path.exists, but instead of checking
+        for files on the real filesystem it considers only the paths provided in 'paths_which_exist' argument.
+        :param paths_which_exist: list of paths which should be considered as existing
+        :return: function to check if path exists
+        """
+        all_paths = set()
+        for path in paths_which_exist or []:
+            # for path /a/b/c, add it and also add components of the path: /a, /a/b
+            end = len(path)
+            while end > 0:
+                all_paths.add(path[0:end])
+                end = path.rfind('/', 0, end)
+
+        def path_exists(path: str) -> bool:
+            return path in all_paths
+
+        return path_exists
+
+    def split_paths_concatenated_base(self, paths_to_concatentate: typing.List[str],
+                                      paths_existing: typing.List[str]) -> typing.List[str]:
+        concatenated = ' '.join(paths_to_concatentate)
+        path_exists = self.path_exists_by_list(paths_existing)
+        return split_paths_by_spaces(concatenated, path_exists_cb=path_exists)
+
+    def check_paths_concatenated(self, *args: str) -> None:
+        paths = [*args]
+        paths_split = self.split_paths_concatenated_base(paths_to_concatentate=paths, paths_existing=paths)
+        self.assertListEqual(paths, paths_split)
+
+    def check_paths_concatenated_ambiguous(self, *args: str,
+                                           additional_paths_exist: typing.Optional[typing.List[str]] = None) -> None:
+        paths = [*args]
+        self.assertRaises(PathSplitError, self.split_paths_concatenated_base, paths_to_concatentate=paths,
+                          paths_existing=paths + (additional_paths_exist or []))
+
+    def check_paths_concatenated_nonexistent(self, *args: str,
+                                             additional_paths_exist: typing.List[str] = None) -> None:
+        paths = [*args]
+        self.assertRaises(PathSplitError, self.split_paths_concatenated_base, paths_to_concatentate=paths,
+                          paths_existing=additional_paths_exist)
+
+
+if __name__ == '__main__':
+    main()