-
Notifications
You must be signed in to change notification settings - Fork 2k
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
dist/tools/esptools: add macOS support to install/export scripts #18385
dist/tools/esptools: add macOS support to install/export scripts #18385
Conversation
Since macOS doesn't have `ldconfig` command, it must not be used outside the `install_qemu` and `export_qemu` function.
Since the only platform support by `qemu-esp32` is `linux-amd64`, a platform test is added to the `export_qemu` function.
@benpicco Do you know who has Apple macOS and could test it? |
I'd recommend changing the first if-statement in
Otherwise, it will block the rest of the function from working properly.
(Please note that I'm not familiar with bash scripting) |
Hm, the precompiled |
It's not Bash syntax. Used interpreter is just |
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.
ACK. I do not have a macOS machine to test this. But few RIOT developers have. So I guess we can only provide best effort support for macOs only.
@leandrolanzieri: Does it make sense to rush this into the release? I doubt that testing of this will get any better by delaying it and it is a bug fix, so I'd say yes. What do you think? |
@maribu Thanks. |
19125: dist/tools/esptools/export.sh: fixes IDF_TOOLS_PATH default setting r=benpicco a=gschorcht ### Contribution description This PR re-adds the setting of the default value of the variable `IDF_TOOLS_PATH` which was accidentally removed with PR #18385. The `IDF_TOOLS_PATH` variable is used by the ESP-IDF to define the path where the Espressif toolchain is installed. Reusing this variable to set the path of the toolchain in RIOT allows the use of an existing Espressif toolchain if it has already been installed with ESP-IDF. If the variable is not defined it is set to `${HOME}/.espressif` as described in [documentation](https://doc.riot-os.org/group__cpu__esp32.html#esp32_local_toolchain_installation): ``` if [ -z ${IDF_TOOLS_PATH} ]; then IDF_TOOLS_PATH=${HOME}/.espressif fi ``` Unfortunatly were these 3 lines be removed in `dist/tools/esptools/export.sh` accidentially with #18385 so that it became necessary to set this variable manually. If the variable was not set, the `dist/tools/esptools/export.sh` script didn't work for the Espressif toolchain installed with the companion script `dist/tools/esptools/install.sh`. ### Testing procedure ``` dist/tools/esptools/install.sh esp32 . dist/tools/esptools/export.sh esp32 ``` Without the PR, the export doesn't work, with the PR it works. ### Issues/PRs references Fixes PR #18385 Co-authored-by: Gunar Schorcht <[email protected]>
Contribution description
This PR provides the folling changes in
dist/tools/esptools/{install,export}.sh
to support macOS:ldconfig
command, the test forlibncursesw.so
version using theldconfig
command is moved to functioninstall_qemu
.qemu-esp32
islinux-amd64
, a platform test is added to theexport_qemu
function.Darwin-x86_64
added to the installation script.The problem was found in https://forum.riot-os.org/t/esp32-undefined-reference-to-esp-mesh-is-scan-allowed/3680
Please note: I don't know whether the changes really work on macOS because of missing hardware.
Testing procedure
dist/tools/esptools/{install,export}.sh all
should work as before on Linux.dist/tools/esptools/{install,export}.sh all
should now work on macOS as well.Issues/PRs references