tmedicci commented on code in PR #17236: URL: https://github.com/apache/nuttx/pull/17236#discussion_r2497940522
########## arch/risc-v/src/common/espressif/Make.defs: ########## @@ -199,41 +199,177 @@ ifndef ESP_HAL_3RDPARTY_VERSION endif ifndef ESP_HAL_3RDPARTY_URL - ESP_HAL_3RDPARTY_URL = https://github.com/espressif/esp-hal-3rdparty.git + ESP_HAL_3RDPARTY_URL = https://github.com/espressif/esp-hal-3rdparty/archive endif -ifndef DISABLE_GIT_DEPTH -ifndef GIT_DEPTH - GIT_DEPTH=1 +ESP_HAL_3RDPARTY_ZIP = $(ESP_HAL_3RDPARTY_VERSION).zip + +ifeq ($(STORAGETMP),y) +define DOWNLOAD_ESP_HAL_3RDPARTY_REPO + $(call DOWNLOAD,$(ESP_HAL_3RDPARTY_URL),$(ESP_HAL_3RDPARTY_ZIP),chip/$(ESP_HAL_3RDPARTY_ZIP),$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_ZIP)) +endef +else +define DOWNLOAD_ESP_HAL_3RDPARTY_REPO + $(call DOWNLOAD,$(ESP_HAL_3RDPARTY_URL),$(ESP_HAL_3RDPARTY_ZIP),chip/$(ESP_HAL_3RDPARTY_ZIP)) +endef endif - GIT_DEPTH_PARAMETER = --depth=$(GIT_DEPTH) + +$(ESP_HAL_3RDPARTY_ZIP): + $(Q) $(call DOWNLOAD_ESP_HAL_3RDPARTY_REPO) + +chip/$(ESP_HAL_3RDPARTY_REPO): $(ESP_HAL_3RDPARTY_ZIP) + $(Q) echo "Unpacking: Espressif HAL for 3rd Party Platforms" + $(Q) unzip -oqq chip/$(ESP_HAL_3RDPARTY_ZIP) -d chip/ + $(Q) mv chip/$(ESP_HAL_3RDPARTY_REPO)-$(ESP_HAL_3RDPARTY_VERSION) chip/$(ESP_HAL_3RDPARTY_REPO) + +ESP_COMPONENTS_MBEDTLS_UNPACK = mbedtls +ifndef ESP_COMPONENTS_MBEDTLS_VERSION + ESP_COMPONENTS_MBEDTLS_VERSION = mbedtls-3.6.3-idf +endif + +ifndef ESP_COMPONENTS_MBEDTLS_URL + ESP_COMPONENTS_MBEDTLS_URL = https://github.com/espressif/mbedtls/archive endif +ESP_COMPONENTS_MBEDTLS_ZIP = $(ESP_COMPONENTS_MBEDTLS_VERSION).zip + ifeq ($(STORAGETMP),y) -define CLONE_ESP_HAL_3RDPARTY_REPO - $(call CHECK_COMMITSHA, $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO),$(ESP_HAL_3RDPARTY_VERSION)) - $(call CLONE, $(ESP_HAL_3RDPARTY_URL),chip/$(ESP_HAL_3RDPARTY_REPO),$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)) +define DOWNLOAD_ESP_COMPONENTS_MBEDTLS_UNPACK + $(call DOWNLOAD,$(ESP_COMPONENTS_MBEDTLS_URL),$(ESP_COMPONENTS_MBEDTLS_ZIP),chip/$(ESP_COMPONENTS_MBEDTLS_ZIP),$(NXTMPDIR)/$(ESP_COMPONENTS_MBEDTLS_ZIP)) endef else -define CLONE_ESP_HAL_3RDPARTY_REPO - $(call CLONE, $(ESP_HAL_3RDPARTY_URL),chip/$(ESP_HAL_3RDPARTY_REPO)) +define DOWNLOAD_ESP_COMPONENTS_MBEDTLS_UNPACK + $(call DOWNLOAD,$(ESP_COMPONENTS_MBEDTLS_URL),$(ESP_COMPONENTS_MBEDTLS_ZIP),chip/$(ESP_COMPONENTS_MBEDTLS_ZIP)) endef endif -chip/$(ESP_HAL_3RDPARTY_REPO): - $(Q) echo "Cloning Espressif HAL for 3rd Party Platforms" - $(Q) $(call CLONE_ESP_HAL_3RDPARTY_REPO) - $(Q) echo "Espressif HAL for 3rd Party Platforms: ${ESP_HAL_3RDPARTY_VERSION}" - $(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO) checkout --quiet $(ESP_HAL_3RDPARTY_VERSION) - $(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO) submodule --quiet update --init $(GIT_DEPTH_PARAMETER) components/mbedtls/mbedtls -ifeq ($(CONFIG_ESP_WIRELESS),y) - $(Q) echo "Espressif HAL for 3rd Party Platforms: initializing submodules..." - $(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO) submodule --quiet update --init $(GIT_DEPTH_PARAMETER) components/esp_phy/lib components/esp_wifi/lib components/bt/controller/lib_esp32c3_family components/esp_coex/lib - $(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls reset --quiet --hard +$(ESP_COMPONENTS_MBEDTLS_ZIP): + $(call DOWNLOAD_ESP_COMPONENTS_MBEDTLS_UNPACK) + +chip/$(ESP_COMPONENTS_MBEDTLS_UNPACK): $(ESP_COMPONENTS_MBEDTLS_ZIP) chip/$(ESP_HAL_3RDPARTY_REPO) + $(Q) unzip -oqq chip/$(ESP_COMPONENTS_MBEDTLS_ZIP) -d chip/ + $(Q) rm -fr chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls + $(Q) mv chip/$(ESP_COMPONENTS_MBEDTLS_UNPACK)-$(ESP_COMPONENTS_MBEDTLS_VERSION) chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls $(Q) echo "Applying patches..." - $(Q) cd chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls && git apply ../../../nuttx/patches/components/mbedtls/mbedtls/*.patch + $(Q) cd chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls && patch -p1 < ../../../nuttx/patches/components/mbedtls/mbedtls/0001-mbedtls_add_prefix.patch + $(Q) cd chip/$(ESP_HAL_3RDPARTY_REPO)/components/mbedtls/mbedtls && patch -p1 < ../../../nuttx/patches/components/mbedtls/mbedtls/0002-mbedtls_add_prefix_to_macro.patch + +ESP_COMPONENTS_ESP_PHY_LIB_UNPACK = esp-phy-lib +ifndef ESP_COMPONENTS_ESP_PHY_LIB_VERSION Review Comment: I'll address these points individually, starting from 3): > 3\. `$(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO) checkout --quiet $(ESP_HAL_3RDPARTY_VERSION)` will always force the `esp-hal-3rdparty` defined in `ESP_HAL_3RDPARTY_VERSION` to that version. Is it possible to keep the local git repository as is without taking any action? I don't recommend that because the code is tightly coupled with the HAL. I mean: a specific version of NuttX must use a specific version of the HAL. If we have nothing that binds them, it'd be very hard to debug anything related to that. This is the classical example of an error-prone approach. Please note that between different builds (without reconfiguring the defconfig), it wouldn't check the HAL's version because the path `arch/<arch>/src/chip/esp-hal-3rdparty` already exists. If you `make distclean` and `./tools/configure.sh -S` to set another board, it will rely on `../nxtmpdir` to copy the HAL content to the new `arch/<arch>/src/chip/esp-hal-3rdparty`. You can always set `ESP_HAL_3RDPARTY_VERSION` during the development workflow to set a specific hash under development. > 2\. The action `$(call CHECK_COMMITSHA, $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO),$(ESP_HAL_3RDPARTY_VERSION))` checks the commit version, and if it does not match the set ESP_HAL_3RDPARTY_VERSION, it will delete `nxtmpdir/esp-hal-3rdparty`. Is it possible to skip the check and directly use `nxtmpdir/esp-hal-3rdparty`? Same point about 3). But you have a point here. Instead of removing the `nxtmpdir`, if the `esp-hal-3rdparty` exists, it should add `ESP_HAL_3RDPARTY_URL` to remote again and fetch from it (to retrieve just the incremental changes) and, then, check again for `ESP_HAL_3RDPARTY_VERSION`. We do it externally, but could be done internally, on NuttX. > 1. Does the action `$(Q) git -C chip/$(ESP_HAL_3RDPARTY_REPO) submodule --quiet update --init` need to be repeated for every board compilation, or can it be prepared in advance so that it does not have to be executed for each board? This is rarely a problem because the `esp-hal-3rdparty`'s submodules will already be checked to the expected SHA. `git submodule update` can be improved with the `--no-fetch` argument to avoid fetching all the data. If it fails, we trigger it again to fetch all data (and check out to the required submodules' commit sha). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
