From dd60e20ea017dc66ec7c441b9136e9d4c22ef475 Mon Sep 17 00:00:00 2001 From: Marek Fiala Date: Fri, 16 May 2025 11:19:52 +0200 Subject: [PATCH 1/3] fix(tools): idf_tools.py install tool@version --- tools/idf_tools.py | 47 +++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/tools/idf_tools.py b/tools/idf_tools.py index 0c4cfa34d1..4a49c7f635 100755 --- a/tools/idf_tools.py +++ b/tools/idf_tools.py @@ -1008,14 +1008,37 @@ class IDFTool(object): def get_preferred_installed_version(self) -> Optional[str]: """ - Get the preferred installed version of the tool. If more versions installed, return the highest. + Get the preferred installed version of the tool. + If more versions installed, return recommended version if exists, otherwise return the highest supported version """ - recommended_versions = [k for k in self.versions_installed - if self.versions[k].status == IDFToolVersion.STATUS_RECOMMENDED - and self.versions[k].compatible_with_platform(self._platform)] - assert len(recommended_versions) <= 1 - if recommended_versions: - return recommended_versions[0] + + try: + self.find_installed_versions() + except ToolBinaryError: + pass + + if self.get_recommended_version() in self.versions_installed: + return self.get_recommended_version() + + supported_installed_versions = [ + k + for k in self.versions_installed + if self.versions[k].status == IDFToolVersion.STATUS_SUPPORTED + and self.versions[k].compatible_with_platform(self._platform) + ] + sorted_supported_installed_versions = sorted( + supported_installed_versions, key=lambda x: self.versions[x], reverse=True + ) + if sorted_supported_installed_versions: + warn( + ''.join( + [ + f'Using supported version {sorted_supported_installed_versions[0]} for tool {self.name} ', + f'as recommended version {self.get_recommended_version()} is not installed.', + ] + ) + ) + return sorted_supported_installed_versions[0] return None def find_installed_versions(self) -> None: @@ -1062,8 +1085,7 @@ class IDFTool(object): else: if ver_str != version: warn(f'tool {self.name} version {version} is installed, but has reported version {ver_str}') - else: - self.versions_installed.append(version) + self.versions_installed.append(version) if tool_error: raise ToolBinaryError @@ -1758,6 +1780,9 @@ def expand_tools_arg(tools_spec: List[str], overall_tools: OrderedDict, targets: # Filtering by ESP_targets tools = [k for k in tools if overall_tools[k].is_supported_for_any_of_targets(targets)] + + # Processing specific version of tool - defined with '@' + tools.extend([tool_pattern for tool_pattern in tools_spec if '@' in tool_pattern]) return tools @@ -2093,10 +2118,6 @@ def process_tool( tool_export_paths: List[str] = [] tool_export_vars: Dict[str, str] = {} - try: - tool.find_installed_versions() - except ToolBinaryError: - pass recommended_version_to_use = tool.get_preferred_installed_version() if not tool.is_executable and recommended_version_to_use: From 207d728348f79e41eba5ed429776d66e730f778b Mon Sep 17 00:00:00 2001 From: Marek Fiala Date: Mon, 2 Jun 2025 15:08:44 +0200 Subject: [PATCH 2/3] test(tools): Added test for installing supported tool version Added test_export_supported_version_cmake in `test_idf_tools.py`, that installs and exports supported version of tool - cmake. --- .gitlab/ci/host-test.yml | 3 ++- .gitlab/ci/test-win.yml | 3 ++- tools/test_idf_tools/test_idf_tools.py | 26 ++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/.gitlab/ci/host-test.yml b/.gitlab/ci/host-test.yml index b7238c202d..6f3bc2f8a6 100644 --- a/.gitlab/ci/host-test.yml +++ b/.gitlab/ci/host-test.yml @@ -133,7 +133,8 @@ test_cli_installer: script: # Tools must be downloaded for testing # We could use "idf_tools.py download all", but we don't want to install clang because of its huge size - - python3 ${IDF_PATH}/tools/idf_tools.py download required qemu-riscv32 qemu-xtensa cmake + # cmake@version that is supported + - python3 ${IDF_PATH}/tools/idf_tools.py download required qemu-riscv32 qemu-xtensa cmake cmake@3.16.3 - cd ${IDF_PATH}/tools/test_idf_tools - python3 -m pip install jsonschema - python3 ./test_idf_tools.py -v diff --git a/.gitlab/ci/test-win.yml b/.gitlab/ci/test-win.yml index 42e2446933..5e0a197e06 100644 --- a/.gitlab/ci/test-win.yml +++ b/.gitlab/ci/test-win.yml @@ -31,7 +31,8 @@ test_cli_installer_win: IDF_PATH: "$CI_PROJECT_DIR" script: # Tools must be downloaded for testing - - python ${IDF_PATH}\tools\idf_tools.py download required qemu-riscv32 qemu-xtensa cmake + # cmake@version that is supported + - python ${IDF_PATH}\tools\idf_tools.py download required qemu-riscv32 qemu-xtensa cmake cmake@3.16.3 - cd ${IDF_PATH}\tools\test_idf_tools - python -m pip install jsonschema - python .\test_idf_tools.py diff --git a/tools/test_idf_tools/test_idf_tools.py b/tools/test_idf_tools/test_idf_tools.py index 1275430ef0..947889dd8e 100755 --- a/tools/test_idf_tools/test_idf_tools.py +++ b/tools/test_idf_tools/test_idf_tools.py @@ -303,6 +303,32 @@ class TestUsage(TestUsageBase): self.assertIn(os.path.join(tool_to_test, tool_version), output) + def test_export_supported_version_cmake(self): + tool_to_test = 'cmake' + supported_version = '' + recommended_version = '' + for tool in self.tools_dict['tools']: + if tool['name'] != tool_to_test: + continue + for version in tool['versions']: + if version['status'] == 'supported': + supported_version = version['name'] + elif version['status'] == 'recommended': + recommended_version = version['name'] + + self.run_idf_tools_with_action(['install']) + output = self.run_idf_tools_with_action(['install', f'{tool_to_test}@{supported_version}']) + self.assert_tool_installed(output, tool_to_test, supported_version) + + # Remove the recommended version folder installed by install command (in case of Windows) + recommended_version_folder = os.path.join(self.temp_tools_dir, 'tools', tool_to_test, recommended_version) + if os.path.exists(recommended_version_folder): + shutil.rmtree(recommended_version_folder) + + output = self.run_idf_tools_with_action(['export']) + self.assertIn(os.path.join(tool_to_test, supported_version), output) + self.assertNotIn(os.path.join(tool_to_test, recommended_version), output) + def test_export_prefer_system_cmake(self): tool_to_test = 'cmake' self.run_idf_tools_with_action(['install']) From 17346297e669d63f7783b0b79de6ea9b48d107f6 Mon Sep 17 00:00:00 2001 From: Marek Fiala Date: Mon, 2 Jun 2025 16:07:24 +0200 Subject: [PATCH 3/3] fix(tools): idf_tools.py uninstall decide based on preferred tool version idf_tools.py uninstall now doesn't take only recommended version, but makes the decision based on preferred installed versions. --- tools/idf_tools.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/idf_tools.py b/tools/idf_tools.py index 4a49c7f635..2c88edcb21 100755 --- a/tools/idf_tools.py +++ b/tools/idf_tools.py @@ -2914,8 +2914,10 @@ def action_uninstall(args: Any) -> None: for tool in installed_tools: tool_versions = os.listdir(os.path.join(tools_path, tool)) if os.path.isdir(os.path.join(tools_path, tool)) else [] try: - unused_versions = ([x for x in tool_versions if x != tools_info[tool].get_recommended_version()]) - except KeyError: # When tool that is not supported by tools_info (tools.json) anymore, remove the whole tool file + unused_versions = [x for x in tool_versions if x != tools_info[tool].get_preferred_installed_version()] + except ( + KeyError + ): # When tool that is not supported by tools_info (tools.json) anymore, remove the whole tool file unused_versions = [''] if unused_versions: unused_tools_versions[tool] = unused_versions @@ -2960,7 +2962,7 @@ def action_uninstall(args: Any) -> None: tool_name, tool_version = tool_spec.split('@', 1) tool_obj = tools_info_for_platform[tool_name] if tool_version is None: - tool_version = tool_obj.get_recommended_version() + tool_version = tool_obj.get_preferred_installed_version() # mypy-checks if tool_version is not None: archive_version = tool_obj.versions[tool_version].get_download_for_platform(CURRENT_PLATFORM)