From cf8fb9e95064818c2ff437ec76fe158e454df5fe Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 21 Dec 2021 13:08:32 +0100 Subject: [PATCH 1/4] tools: fixup version references related to paths with spaces The feature will only appear in 5.0 release, not in 4.4. --- tools/kconfig_new/prepare_kconfig_files.py | 5 ++--- tools/split_paths_by_spaces.py | 10 +++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/tools/kconfig_new/prepare_kconfig_files.py b/tools/kconfig_new/prepare_kconfig_files.py index 6ab38f6e43..9ad9cfc935 100644 --- a/tools/kconfig_new/prepare_kconfig_files.py +++ b/tools/kconfig_new/prepare_kconfig_files.py @@ -32,9 +32,8 @@ def _prepare_source_files(env_dict, list_separator): 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. + The character used to delimit paths in COMPONENT_KCONFIGS* variables is set using --list-separator option. + Space separated lists are currently only used by the documentation build system (esp-docs). """ def _dequote(var): diff --git a/tools/split_paths_by_spaces.py b/tools/split_paths_by_spaces.py index 5a1b25637f..d5e7a81dec 100644 --- a/tools/split_paths_by_spaces.py +++ b/tools/split_paths_by_spaces.py @@ -1,21 +1,21 @@ #!/usr/bin/env python # coding=utf-8 # -# SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2021-2022 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. +# IDF versions <=v4.4 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. +# In IDF 5.0 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. # @@ -69,7 +69,7 @@ def main() -> None: 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 + Note: In ESP-IDF v5.0 and later, COMPONENT_DIRS and EXTRA_COMPONENT_DIRS should be defined as CMake lists, not as space separated strings. Examples: @@ -100,7 +100,7 @@ def main() -> None: list(APPEND component2) Defining COMPONENT_DIRS and EXTRA_COMPONENT_DIRS as CMake lists is backwards compatible - with ESP-IDF 4.3 and below. + with ESP-IDF 4.4 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.) From 9703b6821558c413f870cc549ea1bb4c59cadc93 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 2 May 2022 18:46:02 +0200 Subject: [PATCH 2/4] tools: export.sh: fix expansion of IDF_PATH with spaces --- export.sh | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/export.sh b/export.sh index 35e7c5270e..9eefa41549 100644 --- a/export.sh +++ b/export.sh @@ -18,7 +18,7 @@ __verbose() { } __script_dir(){ - # shellcheck disable=SC2169,SC2169,SC2039 # unreachable with 'dash' + # shellcheck disable=SC2169,SC2169,SC2039,SC3010,SC3028 # unreachable with 'dash' if [[ "$OSTYPE" == "darwin"* ]]; then # convert possibly relative path to absolute script_dir="$(__realpath "${self_path}")" @@ -48,11 +48,11 @@ __main() { # The file doesn't have executable permissions, so this shouldn't really happen. # Doing this in case someone tries to chmod +x it and execute... - # shellcheck disable=SC2128,SC2169,SC2039 # ignore array expansion warning + # shellcheck disable=SC2128,SC2169,SC2039,SC3054 # ignore array expansion warning if [ -n "${BASH_SOURCE-}" ] && [ "${BASH_SOURCE[0]}" = "${0}" ] then echo "This script should be sourced, not executed:" - # shellcheck disable=SC2039 # reachable only with bash + # shellcheck disable=SC2039,SC3054 # reachable only with bash echo ". ${BASH_SOURCE[0]}" return 1 fi @@ -68,9 +68,9 @@ __main() { self_path="${(%):-%x}" fi - script_dir=$(__script_dir) + script_dir="$(__script_dir)" # Since sh or dash shells can't detect script_dir correctly, check if script_dir looks like an IDF directory - is_script_dir_esp_idf=$(__is_dir_esp_idf ${script_dir}) + is_script_dir_esp_idf=$(__is_dir_esp_idf "${script_dir}") if [ -z "${IDF_PATH}" ] then @@ -95,7 +95,7 @@ __main() { export IDF_PATH="${script_dir}" fi # Check if this path looks like an IDF directory - is_idf_path_esp_idf=$(__is_dir_esp_idf ${IDF_PATH}) + is_idf_path_esp_idf=$(__is_dir_esp_idf "${IDF_PATH}") if [ -n "${is_idf_path_esp_idf}" ] then echo "IDF_PATH is set to '${IDF_PATH}', but it doesn't look like an ESP-IDF directory." @@ -130,7 +130,7 @@ __main() { IDF_ADD_PATHS_EXTRAS="${IDF_ADD_PATHS_EXTRAS}:${IDF_PATH}/components/partition_table" IDF_ADD_PATHS_EXTRAS="${IDF_ADD_PATHS_EXTRAS}:${IDF_PATH}/components/app_update" - idf_exports=$("$ESP_PYTHON" "${IDF_PATH}/tools/idf_tools.py" export --add_paths_extras=${IDF_ADD_PATHS_EXTRAS}) || return 1 + idf_exports=$("$ESP_PYTHON" "${IDF_PATH}/tools/idf_tools.py" export "--add_paths_extras=${IDF_ADD_PATHS_EXTRAS}") || return 1 eval "${idf_exports}" export PATH="${IDF_ADD_PATHS_EXTRAS}:${PATH}" @@ -166,7 +166,7 @@ __main() { __verbose "" __verbose "Detected installed tools that are not currently used by active ESP-IDF version." __verbose "${uninstall}" - __verbose "For free up even more space, remove installation packages of those tools. Use option '${ESP_PYTHON} ${IDF_PATH}/tools/idf_tools.py uninstall --remove-archives'." + __verbose "To free up even more space, remove installation packages of those tools. Use option '${ESP_PYTHON} ${IDF_PATH}/tools/idf_tools.py uninstall --remove-archives'." __verbose "" fi @@ -209,7 +209,7 @@ __cleanup() { __enable_autocomplete() { click_version="$(python -c 'import click; print(click.__version__.split(".")[0])')" - if [[ click_version -lt 8 ]] + if [ "${click_version}" -lt 8 ] then SOURCE_ZSH=source_zsh SOURCE_BASH=source_bash @@ -224,7 +224,8 @@ __enable_autocomplete() { elif [ -n "${BASH_SOURCE-}" ] then WARNING_MSG="WARNING: Failed to load shell autocompletion for bash version: $BASH_VERSION!" - [[ ${BASH_VERSINFO[0]} -lt 4 ]] && { echo "$WARNING_MSG"; return; } + # shellcheck disable=SC3028,SC3054,SC2086 # code block for 'bash' only + [ ${BASH_VERSINFO[0]} -lt 4 ] && { echo "$WARNING_MSG"; return; } eval "$(env LANG=en _IDF.PY_COMPLETE=$SOURCE_BASH idf.py)" || echo "$WARNING_MSG" fi } From 130bbf3d6cde4eb14f8ee3229c53b6b7a0808b3b Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 31 Aug 2021 00:32:07 +0200 Subject: [PATCH 3/4] ci: add build job to test paths with spaces --- .gitlab/ci/build.yml | 5 + .gitlab/ci/rules.yml | 1 + tools/ci/executable-list.txt | 1 + tools/ci/test_build_system_spaces.py | 220 +++++++++++++++++++++++++++ 4 files changed, 227 insertions(+) create mode 100755 tools/ci/test_build_system_spaces.py diff --git a/.gitlab/ci/build.yml b/.gitlab/ci/build.yml index 9006db3b41..420cb625d0 100644 --- a/.gitlab/ci/build.yml +++ b/.gitlab/ci/build.yml @@ -434,6 +434,11 @@ test_build_system_cmake_macos: variables: SHELL_TEST_SCRIPT: test_build_system_cmake.sh +test_build_system_spaces: + extends: .test_build_system_template + variables: + SHELL_TEST_SCRIPT: test_build_system_spaces.py + build_docker: extends: - .before_script_minimal diff --git a/.gitlab/ci/rules.yml b/.gitlab/ci/rules.yml index 0e187f09bb..c3dc165ea8 100644 --- a/.gitlab/ci/rules.yml +++ b/.gitlab/ci/rules.yml @@ -51,6 +51,7 @@ - "tools/tools.json" - "tools/requirements.json" - "tools/ci/test_build_system*.sh" + - "tools/ci/test_build_system*.py" .patterns-custom_test: &patterns-custom_test - "components/espcoredump/**/*" diff --git a/tools/ci/executable-list.txt b/tools/ci/executable-list.txt index d7b3a58d1d..5c5946a9b8 100644 --- a/tools/ci/executable-list.txt +++ b/tools/ci/executable-list.txt @@ -77,6 +77,7 @@ tools/ci/multirun_with_pyenv.sh tools/ci/push_to_github.sh tools/ci/test_autocomplete.py tools/ci/test_build_system_cmake.sh +tools/ci/test_build_system_spaces.py tools/ci/test_check_kconfigs.py tools/ci/test_configure_ci_environment.sh tools/ci/test_reproducible_build.sh diff --git a/tools/ci/test_build_system_spaces.py b/tools/ci/test_build_system_spaces.py new file mode 100755 index 0000000000..e275267f17 --- /dev/null +++ b/tools/ci/test_build_system_spaces.py @@ -0,0 +1,220 @@ +#!/usr/bin/env python +# SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: Apache-2.0 +import os +import shutil +import subprocess +import sys +import typing +import unittest + +try: + IDF_PATH = os.environ['IDF_PATH'] # type: str +except KeyError: + print('IDF_PATH must be set before running this test', file=sys.stderr) + exit(1) + +# Use the current directory for the builds +TEST_PATH = os.getcwd() + + +################## +# Helper functions +################## + +def find_python(path_var: str) -> str: + """ + Find python interpreter in the paths specified in the given PATH variable. + Returns the full path to the interpreter. + """ + res = shutil.which('python', path=path_var) + if res is None: + raise ValueError('python not found') + return res + + +def get_idf_build_env(idf_path: typing.Optional[str]) -> typing.Dict[str, str]: + """ + Get environment variables (as set by export.sh) for the specific IDF copy + :param idf_path: if set, path of the IDF copy to use; otherwise, IDF_PATH from environment is used + :return: dictionary of environment variables and their values + """ + if idf_path is None: + idf_path = IDF_PATH + cmd = [ + sys.executable, + os.path.join(idf_path, 'tools', 'idf_tools.py'), + 'export', + '--format=key-value' + ] + keys_values = subprocess.check_output(cmd).decode() + env_vars = {key: os.path.expandvars(value) for key, value in + [line.split('=') for line in keys_values.splitlines()]} + # not set by idf_tools.py, normally set by export.sh + env_vars['IDF_PATH'] = idf_path + + return env_vars + + +def idf_py(*args: str, env_vars: typing.Optional[typing.Dict[str, str]] = None, + idf_path: typing.Optional[str] = None, + workdir: typing.Optional[str] = None) -> None: + """ + Run idf.py command with given arguments, raise an exception on failure + :param args: arguments to pass to idf.py + :param env_vars: environment variables to run the build with; if not set, the default environment is used + :param idf_path: path to the IDF copy to use; if not set, IDF_PATH from the environment is used + :param workdir: directory where to run the build; if not set, the current directory is used + """ + env = dict(**os.environ) + if env_vars is not None: + env.update(env_vars) + if not workdir: + workdir = os.getcwd() + # order: function argument -> value in env dictionary -> system environment + if idf_path is not None: + inferred_idf_path = idf_path + else: + inferred_idf_path = str(env.get('IDF_PATH', IDF_PATH)) + + python = find_python(env['PATH']) + + cmd = [ + python, + os.path.join(inferred_idf_path, 'tools', 'idf.py') + ] + cmd += args # type: ignore + subprocess.check_call(cmd, env=env, cwd=workdir) + + +############ +# Test cases +############ + +class PathsWithSpaces(unittest.TestCase): + IDF_PATH_WITH_SPACES = '' + DO_CLEANUP = True + ENV: typing.Dict[str, str] = dict() + + @classmethod + def setUpClass(cls) -> None: + if ' ' in IDF_PATH: + print('IDF_PATH already contains spaces, not making a copy') + cls.IDF_PATH_WITH_SPACES = IDF_PATH + cls.DO_CLEANUP = False + else: + # Make a copy of ESP-IDF directory, with a space in its name + cls.IDF_PATH_WITH_SPACES = os.path.join(TEST_PATH, 'esp idf') + dest = cls.IDF_PATH_WITH_SPACES + print('Copying esp-idf from {} to {}'.format(IDF_PATH, dest)) + shutil.copytree(IDF_PATH, dest, + # if the CWD is inside the original esp-idf directory, make sure not to go into recursion. + ignore=shutil.ignore_patterns(os.path.basename(dest))) + + cls.ENV = get_idf_build_env(cls.IDF_PATH_WITH_SPACES) + + @classmethod + def tearDownClass(cls) -> None: + if cls.DO_CLEANUP and os.path.exists(cls.IDF_PATH_WITH_SPACES): + shutil.rmtree(cls.IDF_PATH_WITH_SPACES, ignore_errors=True) + + def test_install_export(self) -> None: + env = dict(**os.environ) + del env['IDF_PATH'] + if os.name == 'nt': + install_cmd = 'install.bat esp32' + export_cmd = 'export.bat' + else: + install_cmd = './install.sh esp32' + export_cmd = '. ./export.sh' + + subprocess.check_call(install_cmd, env=env, shell=True, cwd=self.IDF_PATH_WITH_SPACES) + + if os.name == 'nt': + subprocess.check_call(export_cmd, env=env, shell=True, cwd=self.IDF_PATH_WITH_SPACES) + else: + # The default shell used by subprocess.Popen on POSIX platforms is '/bin/sh', + # which in esp-env Docker image is 'dash'. The export script doesn't support + # IDF_PATH detection when used in dash, so we have to override the shell here. + subprocess.check_call(export_cmd, env=env, shell=True, cwd=self.IDF_PATH_WITH_SPACES, executable='/bin/bash') + + def _copy_app_to(self, app_path: str, dest_dir_name: str) -> str: + """ + Copy given app to a different directory, setting up cleanup hook. + Destination directory is first deleted if it already exists. + Returns the full path of the destination directory. + app_path: path relative to esp-idf directory + dest_dir_name: name of the destination directory, relative to the directory where the test is running + """ + dest_dir = os.path.join(TEST_PATH, dest_dir_name) + if os.path.exists(dest_dir) and os.path.isdir(dest_dir): + shutil.rmtree(dest_dir, ignore_errors=True) + src_dir = os.path.join(self.IDF_PATH_WITH_SPACES, app_path) + shutil.copytree(os.path.join(src_dir), dest_dir) + build_dir = os.path.join(dest_dir, 'build') + if os.path.exists(build_dir): + shutil.rmtree(build_dir, ignore_errors=True) + self.addCleanup(shutil.rmtree, dest_dir, ignore_errors=True) + return dest_dir + + # The tests below build different ESP-IDF apps (examples or test apps) to cover different parts + # of the build system and related scripts. + # In each test, IDF_PATH and app path both contain spaces. + + def test_build(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'get-started', 'hello_world'), 'test app') + idf_py('build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_build_ulp_fsm(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'system', 'ulp_fsm', 'ulp'), 'test app') + idf_py('build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_build_ulp_riscv(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'system', 'ulp_riscv', 'gpio'), 'test app') + idf_py('-DIDF_TARGET=esp32s2', 'build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, + workdir=build_path) + + def test_spiffsgen(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'storage', 'spiffsgen'), 'test app') + idf_py('build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_flash_encryption(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'security', 'flash_encryption'), 'test app') + idf_py('build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_secure_boot_v1(self) -> None: + build_path = self._copy_app_to(os.path.join('tools', 'test_apps', 'security', 'secure_boot'), 'test app') + idf_py('-DSDKCONFIG_DEFAULTS=sdkconfig.defaults;sdkconfig.ci.01', 'build', env_vars=self.ENV, + idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_secure_boot_v2(self) -> None: + build_path = self._copy_app_to(os.path.join('tools', 'test_apps', 'security', 'secure_boot'), 'test app') + idf_py('-DSDKCONFIG_DEFAULTS=sdkconfig.defaults;sdkconfig.ci.00', 'build', env_vars=self.ENV, + idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_app_signing(self) -> None: + build_path = self._copy_app_to(os.path.join('tools', 'test_apps', 'security', 'secure_boot'), 'test app') + idf_py('-DSDKCONFIG_DEFAULTS=sdkconfig.defaults;sdkconfig.ci.02', 'build', env_vars=self.ENV, + idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_secure_boot_release_mode(self) -> None: + build_path = self._copy_app_to(os.path.join('tools', 'test_apps', 'security', 'secure_boot'), 'test app') + idf_py('-DSDKCONFIG_DEFAULTS=sdkconfig.defaults;sdkconfig.ci.04', '-DIDF_TARGET=esp32s2', 'build', + env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_x509_cert_bundle(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'protocols', 'https_x509_bundle'), 'test app') + idf_py('build', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + def test_dfu(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'get-started', 'hello_world'), 'test app') + idf_py('-DIDF_TARGET=esp32s2', 'dfu', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, + workdir=build_path) + + def test_uf2(self) -> None: + build_path = self._copy_app_to(os.path.join('examples', 'get-started', 'hello_world'), 'test app') + idf_py('uf2', env_vars=self.ENV, idf_path=self.IDF_PATH_WITH_SPACES, workdir=build_path) + + +if __name__ == '__main__': + unittest.main() From a093ed39dfa0f3f6034a10eb4ae050025e484a88 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Tue, 6 Oct 2020 12:34:11 +0200 Subject: [PATCH 4/4] docs: add migration guide for COMPONENT_DIRS and EXTRA_COMPONENT_DIRS --- docs/en/migration-guides/build-system.rst | 25 +++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/en/migration-guides/build-system.rst b/docs/en/migration-guides/build-system.rst index b01fc66729..66e5a013db 100644 --- a/docs/en/migration-guides/build-system.rst +++ b/docs/en/migration-guides/build-system.rst @@ -1,8 +1,8 @@ Migrate Build System to ESP-IDF 5.0 =================================== -Migrating from make to cmake ----------------------------- +Migrating from GNU Make build system +------------------------------------ Please follow the :ref:`build system ` guide for migrating make-based projects no longer supported in ESP-IDF v5.0. @@ -27,3 +27,24 @@ This behavior was caused by transitive dependencies of various common components In ESP-IDF v5.0, this behavior is fixed and these components are no longer added as public requirements by default. Every component depending on one of the components which isn't part of common requirements has to declare this dependency explicitly. This can be done by adding ``REQUIRES `` or ``PRIV_REQUIRES `` in ``idf_component_register`` call inside component's ``CMakeLists.txt``. See :ref:`Component Requirements` for more information on specifying requirements. + +Setting ``COMPONENT_DIRS`` and ``EXTRA_COMPONENT_DIRS`` variables +----------------------------------------------------------------- + +.. highlight:: cmake + +ESP-IDF 5.0 includes a number of improvements to support building projects with space characters in their paths. To make that possible, there are some changes related to setting ``COMPONENT_DIRS`` and ``EXTRA_COMPONENT_DIRS`` variables in project CMakeLists.txt files. + +Adding non-existent directories to ``COMPONENT_DIRS`` or ``EXTRA_COMPONENT_DIRS`` is no longer supported and will result in an error. + +Using string concatenation to define ``COMPONENT_DIRS`` or ``EXTRA_COMPONENT_DIRS`` variables is now deprecated. These variables should be defined as CMake lists, instead. For example, use:: + + set(EXTRA_COMPONENT_DIRS path1 path2) + list(APPEND EXTRA_COMPONENT_DIRS path3) + +instead of:: + + set(EXTRA_COMPONENT_DIRS "path1 path2") + set(EXTRA_COMPONENT_DIRS "${EXTRA_COMPONENT_DIRS} path3") + +Defining these variables as CMake lists is compatible with previous IDF versions.