RIOT icon indicating copy to clipboard operation
RIOT copied to clipboard

dist/tools/esptools: add macOS support to install/export scripts

Open gschorcht opened this issue 2 years ago • 4 comments

Contribution description

This PR provides the folling changes in dist/tools/esptools/{install,export}.sh to support macOS:

  • Since macOS doesn't have ldconfig command, the test for libncursesw.so version using the ldconfig command is moved to function install_qemu.
  • Since the only platform support by qemu-esp32 is linux-amd64, a platform test is added to the export_qemu function.
  • Platform 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

gschorcht avatar Jul 29 '22 15:07 gschorcht

@benpicco Do you know who has Apple macOS and could test it?

gschorcht avatar Jul 29 '22 15:07 gschorcht

I'd recommend changing the first if-statement in install_qemu to something like

if [[ ${OS} != "linux-amd64" && ${OS} != "macos" ]]; then
    echo "error: QEMU for ESP32 does not support OS ${OS}"
    exit 1
fi

Otherwise, it will block the rest of the function from working properly. And the second one should check the version of ncurses/ncursesw based on the OS, something like:

if [ ${OS} != "macos" ]; then
    # qemu version depends on the version of ncurses lib
    if [ "$(ldconfig -p | grep libncursesw.so.6)" != "" ]; then
        ESP32_QEMU_VERSION="esp-develop-20220203"
    else
        ESP32_QEMU_VERSION="esp-develop-20210220"
    fi
else
    # qemu version depends on the version of ncurses lib
    if [[ # Some other method # ]]; then
        ESP32_QEMU_VERSION="esp-develop-20220203"
    else
        ESP32_QEMU_VERSION="esp-develop-20210220"
    fi
fi

(Please note that I'm not familiar with bash scripting)

br0kenpixel avatar Jul 29 '22 16:07 br0kenpixel

I'd recommend changing the first if-statement in install_qemu to something like

if [[ ${OS} != "linux-amd64" && ${OS} != "macos" ]]; then
    echo "error: QEMU for ESP32 does not support OS ${OS}"
    exit 1
fi

Hm, the precompiled qemu-esp32 is only avialble for one platform which is linux-amd64. AFAIK, linux-amd64 it is not working on macOS, correct me if I'm wrong.

gschorcht avatar Jul 30 '22 11:07 gschorcht

(Please note that I'm not familiar with bash scripting)

It's not Bash syntax. Used interpreter is just bin/sh, the Bourne Shell. Even though bin/sh is linked to the Dash on Debian-based Systems, the Dash is compliant with the Single UNIX Specification, that is, compliant with the Bourne Shell. So the script shouldn't require the Bash and should work on all UNIX compliant systems.

gschorcht avatar Jul 30 '22 11:07 gschorcht

@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 avatar Oct 13 '22 06:10 maribu

@maribu Thanks.

gschorcht avatar Oct 13 '22 13:10 gschorcht