-
Notifications
You must be signed in to change notification settings - Fork 7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
esp32: improve toolchain management #38540
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,23 +10,48 @@ set(COMPILER gcc) | |
set(LINKER ld) | ||
set(BINTOOLS gnu) | ||
|
||
# find some toolchain | ||
file(GLOB toolchain_paths | ||
LIST_DIRECTORIES true | ||
${TOOLCHAIN_HOME} | ||
${TOOLCHAIN_HOME}/*-esp32*/*-elf | ||
${TOOLCHAIN_HOME}/*-esp*/*-elf | ||
) | ||
|
||
# Old toolchain installation path has been deprecated in 2.7. | ||
# This code and related code depending on ESPRESSIF_DEPRECATED_PATH can be removed after two releases. | ||
if(NOT toolchain_paths) | ||
# find some toolchain | ||
file(GLOB toolchain_paths | ||
LIST_DIRECTORIES true | ||
${TOOLCHAIN_HOME} | ||
) | ||
|
||
set(ESPRESSIF_DEPRECATED_PATH TRUE) | ||
endif() | ||
|
||
if(toolchain_paths) | ||
list(GET toolchain_paths 0 soc_toolchain_path) | ||
list(GET toolchain_paths 0 some_toolchain_path) | ||
|
||
get_filename_component(one_toolchain_root "${some_toolchain_path}" DIRECTORY) | ||
get_filename_component(one_toolchain "${some_toolchain_path}" NAME) | ||
|
||
get_filename_component(soc_toolchain "${soc_toolchain_path}" NAME) | ||
set(CROSS_COMPILE_TARGET ${one_toolchain}) | ||
set(SYSROOT_TARGET ${one_toolchain}) | ||
|
||
if(ESPRESSIF_DEPRECATED_PATH) | ||
set(CROSS_COMPILE ${ESPRESSIF_TOOLCHAIN_PATH}/bin/${CROSS_COMPILE_TARGET}-) | ||
set(SYSROOT_DIR ${ESPRESSIF_TOOLCHAIN_PATH}/${SYSROOT_TARGET}) | ||
else() | ||
set(CROSS_COMPILE ${one_toolchain_root}/bin/${CROSS_COMPILE_TARGET}-) | ||
set(SYSROOT_DIR ${one_toolchain_root}/${SYSROOT_TARGET}) | ||
endif() | ||
|
||
set(CROSS_COMPILE_TARGET ${soc_toolchain}) | ||
set(SYSROOT_TARGET ${soc_toolchain}) | ||
endif() | ||
|
||
set(CROSS_COMPILE ${TOOLCHAIN_HOME}/bin/${CROSS_COMPILE_TARGET}-) | ||
set(SYSROOT_DIR ${TOOLCHAIN_HOME}/${SYSROOT_TARGET}) | ||
if(NOT CROSS_COMPILE_TARGET) | ||
message(FATAL_ERROR | ||
"Unable to find toolchain in ${TOOLCHAIN_HOME} " | ||
"Run `west espressif install` to download it. Then, export to path accordingly.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tejlmand This is current error if toolchain not found by either missing its installation or having previous installation version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, I see that. |
||
endif() | ||
|
||
set(TOOLCHAIN_HAS_NEWLIB ON CACHE BOOL "True if toolchain supports newlib") | ||
|
||
message(STATUS "Found toolchain: espressif (${ESPRESSIF_TOOLCHAIN_PATH})") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,29 @@ | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
# This file intentionally left blank. | ||
set_ifndef(ESPRESSIF_TOOLCHAIN_PATH "$ENV{ESPRESSIF_TOOLCHAIN_PATH}") | ||
set( ESPRESSIF_TOOLCHAIN_PATH ${ESPRESSIF_TOOLCHAIN_PATH} CACHE PATH "") | ||
assert( ESPRESSIF_TOOLCHAIN_PATH "ESPRESSIF_TOOLCHAIN_PATH is not set") | ||
|
||
set(COMPILER gcc) | ||
set(LINKER ld) | ||
set(BINTOOLS gnu) | ||
|
||
set(CROSS_COMPILE_TARGET_xtensa_esp32 xtensa-esp32-elf) | ||
set(CROSS_COMPILE_TARGET_xtensa_esp32s2 xtensa-esp32s2-elf) | ||
set(CROSS_COMPILE_TARGET_riscv_esp32c3 riscv32-esp-elf) | ||
|
||
set(CROSS_COMPILE_TARGET ${CROSS_COMPILE_TARGET_${ARCH}_${CONFIG_SOC}}) | ||
set(SYSROOT_TARGET ${CROSS_COMPILE_TARGET}) | ||
|
||
if(ESPRESSIF_DEPRECATED_PATH) | ||
set(TOOLCHAIN_HOME ${ESPRESSIF_TOOLCHAIN_PATH}) | ||
else() | ||
set(TOOLCHAIN_HOME ${ESPRESSIF_TOOLCHAIN_PATH}/${CROSS_COMPILE_TARGET}) | ||
endif() | ||
|
||
set(CROSS_COMPILE ${TOOLCHAIN_HOME}/bin/${CROSS_COMPILE_TARGET}-) | ||
set(SYSROOT_DIR ${TOOLCHAIN_HOME}/${SYSROOT_TARGET}) | ||
|
||
set(TOOLCHAIN_HAS_NEWLIB ON CACHE BOOL "True if toolchain supports newlib") | ||
|
||
message(STATUS "Found toolchain: espressif (${ESPRESSIF_TOOLCHAIN_PATH})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious.
It seems the espressif toolchain is now expected to be installed under a different folder per default.
(
/.espressif/tools/zephyr/riscv32-esp-elf/...
vs./.espressif/tools/riscv32-esp-elf/...
)Is that part of the espressif installer itself, or is there some missing doc update on toolchain installation ?
Also what about users already having the espressif toolchain installed ?
How will they be aware that they must now move their current
.espressif/tools/riscv32-esp-elf
into.espressif/tools/zephyr/riscv32-esp-elf
instead.Also, is
1.24.0.123_64eb9ff-8.4.0
no longer part of the toolchain installation path ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tejlmand I have explained that already in this PR. Can you check previous comments? The main reason is having the same export PATH for Espressif toolchain to all SoCs, not depending on folder names and versioning as it is right now.
So, as you can see in the document update in this PR, no more
1.24.0.123_64eb9ff-8.4.0
or toolchain name explicit in folder path.west espressif install
is the download agent responsible to download and extract the toolchain in proper place (or user can manually download it as well).When building, it will give them cmake error with a message to re-configure toolchain path.
Is there any recommendation on how to inform users about this change besides changing the ESP32 documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the output when running
west espressif install
. It also informs the user how to export path into the environment.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you refer to this comment #38540 (comment), then you only describe why, but my question was about who / how that path was ensured.
The installer or a guide ?
Now that question you answer with the
west espressif install
sentence.No, you give them an error message requesting users to re-install something that is already installed.
I think we should ensure that if users have a setup with the old style then that will still be supported.
That will allow users to keep working and update the toolchain when a new revision is released and they update anyway.
By the way, your
west espressif install
fails on my machine.because I only have
python3
and nopython
link.Why not use the python that was already invoked with
west
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then we can remove support for the old style at a later release, similar to what is done for other things we deprecate.