CI: Improvement to speed up compilation and reduce download errors.

[simbit18]

Summary

The simple improvement is designed to speed up compilation and reduce download errors on github and local.

Added a folder nxtmpdir for storing third-party packages

nuttxworkspace
|
|- nuttx
|- apps
|- nxtmpdir

tools/Unix.mk:
added nxtmpdir folder creation

tools/configure.sh:
added option -S creates the nxtmpdir folder for third-party packages.

For now I added in the folder this package

ESP_HAL_3RDPARTY_URL = https://github.com/espressif/esp-hal-3rdparty.git

ARCH
arch/xtensa/src/esp32/Make.defs
arch/xtensa/src/esp32s2/Make.defs
arch/xtensa/src/esp32s3/Make.defs
arch/risc-v/src/common/espressif/Make.defs
arch/risc-v/src/esp32c3-legacy/Make.defs

but you can also add other packages (maybe also of apps)

@no1wudi @tmedicci

Impact

There should be no impact.

Testing

CI

[hartmannathan]

Do GitHub Actions download packages each time they run?

If so, can the packages be downloaded once and archived somehow, so they don't have to be re-downloaded every time the Action runs?

Is that what this PR is doing?

[tmedicci]

Thanks @simbit18 . Let me test on our internal CI.

[simbit18]

Hi @hartmannathan

Let's use as an example architectures with expressif SoC.
( espressif developers i love you !!! :) )

For these jobs Linux (risc-v-01), Linux (xtensa-01), Linux (xtensa-02) and for board name:board configuration with the espressif ESP-HAL-3RDPARTY is cloned and then cleaned up each time even if it doesn't change until the job is finished.

So in this case but in general this logic slows down the compilation and can be more prone to network error resulting in workflow failure.

This is why I thought of a temporary storage folder in this PR.

In the case of compiling on GITHUB at each restart of the workflow for each job it does these steps:

1 create the temporary storage folder
2 clone the repository the espressif/ESP-HAL-3RDPARTY only for the first time

the previous workflow does not change (logic of moving to branch-commit).

In the case of local build
1 the first compiling creates the temporary storage folder and clone espressif/ESP-HAL-3RDPARTY
2 the -S option removes the temporary folder with each new configuration .

the previous workflow does not change (logic of moving to branch-commit).

Rightly as @tmedicci noted, since the espressif/ESP-HAL-3RDPARTY is not in the nuttx tree and we can't be aware of changes in the repository (logic of moving to branch-commit) users who don't update the configuration without the -S option might have an outdated repository locally.

Time comparison

Without this PR

https://github.com/apache/nuttx/actions/runs/10730602568/usage

With this PR

https://github.com/apache/nuttx/actions/runs/10722417518/usage

[tmedicci]

Hi @hartmannathan

Let's use as an example architectures with expressif SoC. ( espressif developers i love you !!! :) )

For these jobs Linux (risc-v-01), Linux (xtensa-01), Linux (xtensa-02) and for board name:board configuration with the espressif ESP-HAL-3RDPARTY is cloned and then cleaned up each time even if it doesn't change until the job is finished.

So in this case but in general this logic slows down the compilation and can be more prone to network error resulting in workflow failure.

This is why I thought of a temporary storage folder in this PR.

In the case of compiling on GITHUB at each restart of the workflow for each job it does these steps:

1 created the temporary storage folder 2 clone the repository the espressif/ESP-HAL-3RDPARTY only for the first time

the previous workflow does not change (logic of moving to branch-commit).

In the case of local build 1 the first compiling creates the temporary storage folder and clone espressif/ESP-HAL-3RDPARTY 2 the -S option removes the temporary folder with each new configuration .

the previous workflow does not change (logic of moving to branch-commit).

Rightly as @tmedicci noted, since the espressif/ESP-HAL-3RDPARTY is not in the nuttx tree and we can't be aware of changes in the repository (logic of moving to branch-commit) users who don't update the configuration without the -S option might have an outdated repository locally.

Time comparison

Without this PR

https://github.com/apache/nuttx/actions/runs/10730602568/usage

With this PR https://github.com/apache/nuttx/actions/runs/10722417518/usage

Yeah, this is an important improvement for our CI!

[tmedicci]

Rightly as @tmedicci noted, since the espressif/ESP-HAL-3RDPARTY is not in the nuttx tree and we can't be aware of changes in the repository (logic of moving to branch-commit) users who don't update the configuration without the -S option might have an outdated repository locally.

Just to make sure, do you agree about the problem of the not-in-sync repository? Are you planning to implement a workaround?

[simbit18]

Just to make sure, do you agree about the problem of the not-in-sync repository? Are you planning to implement a workaround?

HI @tmedicci only locally if user does not use ./tools/configure.sh -S esp32-devkitc:<config_name> could happen.

When I have time I will see if it is possible to find solution to correct this case.

Of course comments and suggestions are welcome !!! :)

[simbit18]

Updated

changed logic -S now creates the nxtmpdir folder for third-party packages.
without -S the previous workflow does not change.

.github/workflows/build.yml:
added option -S adds the nxtmpdir folder for third-party packages.
./cibuild.sh -c -A -N -R -S testlist/${{matrix.boards}}.dat

Hi @tmedicci you can run a test on your internal CI.

[tmedicci]

Updated

changed logic -S now creates the nxtmpdir folder for third-party packages. without -S the previous workflow does not change.

.github/workflows/build.yml: added option -S adds the nxtmpdir folder for third-party packages. ./cibuild.sh -c -A -N -R -S testlist/${{matrix.boards}}.dat

Hi @tmedicci you can run a test on your internal CI.

Hi @simbit18, thanks for updating it. Particularly, I liked the solution of creating the temp folder only if -S is passed for ./tools/configure.sh. However, I'm still worried about the mismatch (if an user updates NuttX and the build system uses an outdated version of the HAL). But I think we can address it just like suggested by #13297 (@no1wudi ), what do you think? I think we can use check for the commit of the HAL in the temp folder and copy only if it is the same as the one we need.

[simbit18]

Hi @simbit18, thanks for updating it. Particularly, I liked the solution of creating the temp folder only if -S is passed for ./tools/configure.sh. However, I'm still worried about the mismatch (if an user updates NuttX and the build system uses an outdated version of the HAL). But I think we can address it just like suggested by #13297 (@no1wudi ), what do you think? I think we can use check for the commit of the HAL in the temp folder and copy only if it is the same as the one we need.

Hi @tmedicci I will also try to check the commit of the HAL but I think to make it generic it needs to be transferred to a shell scripting.

The idea of speeding up the NuttX build and using the tmp folder is not only about esp HAL but in general in all parts where git and curl are used.

Because surely the archs, boards and configs will increase in the future, and with them the time to finish jobs will also increase.

[tmedicci]

Hi @tmedicci I will also try to check the commit of the HAL but I think to make it generic it needs to be transferred to a shell scripting.

I agree! This is a starting point. From my perspective, it's totally acceptable to check the git version of the HAL in the makefile as the starting point ;)

[simbit18]

HI @tmedicci @xiaoxiang781216 I simplified by adding these macros to config.mk.

# CLONE - Git clone repository. Initializes a new Git repository in the 
#         folder on your local machine and populates it with the contents
#         of the central repository.
#         The third argument is an storage path. The second argument is used
#         if it is not provided or is empty.
# Example: $(call CLONE,$(URL_BASE),$(PATH),$(STORAGE_FOLDER))

define CLONE
	$(ECHO_BEGIN)"Clone: $(if $3,$3,$2) "
	if [ -z $3 ]; then \
		git clone --quiet $1 $2; \
	else \
		if [ ! -d $3 ]; then \
			git clone --quiet $1 $3; \
		fi; \
		cp -fr $3 $2; \
	fi
	$(ECHO_END)
endef

# COMMITSHA - Check if the branche contain the named commit.
#         Remove the folder if the commit is not present in the branch.
#         The first argument is the repository folder on the local machine.
#         The second argument is unique SHA-1 hash value.
# Example: $(call COMMITSHA,$(GIT_FOLDER),$(COMMIT_SHA-1))

define COMMITSHA
	$(ECHO_BEGIN)"COMMIT SHA-1: $2 "
	if [ -d $1 ]; then \
		if ! git -C $1 branch --contains $2 > /dev/null 2>&1; then \
			rm -rf $1; \
		fi \
	fi
	$(ECHO_END)
endef

@tmedicci Now there is also version control.

[tmedicci]

HI @tmedicci @xiaoxiang781216 I simplified by adding these macros to config.mk.

# CLONE - Git clone repository. Initializes a new Git repository in the 
#         folder on your local machine and populates it with the contents
#         of the central repository.
#         The third argument is an storage path. The second argument is used
#         if it is not provided or is empty.
# Example: $(call CLONE,$(URL_BASE),$(PATH),$(STORAGE_FOLDER))

define CLONE
	$(ECHO_BEGIN)"Clone: $(if $3,$3,$2) "
	if [ -z $3 ]; then \
		git clone --quiet $1 $2; \
	else \
		if [ ! -d $3 ]; then \
			git clone --quiet $1 $3; \
		fi; \
		cp -fr $3 $2; \
	fi
	$(ECHO_END)
endef

# COMMITSHA - Check if the branche contain the named commit.
#         Remove the folder if the commit is not present in the branch.
#         The first argument is the repository folder on the local machine.
#         The second argument is unique SHA-1 hash value.
# Example: $(call COMMITSHA,$(GIT_FOLDER),$(COMMIT_SHA-1))

define COMMITSHA
	$(ECHO_BEGIN)"COMMIT SHA-1: $2 "
	if [ -d $1 ]; then \
		if ! git -C $1 branch --contains $2 > /dev/null 2>&1; then \
			rm -rf $1; \
		fi \
	fi
	$(ECHO_END)
endef

@tmedicci Now there is also version control.

That's fine, thanks! I will test it in our internal CI

[xiaoxiang781216]

could we simply call one function with the enough arguments, which could do all thing internally?

[simbit18]

HI @tmedicci @xiaoxiang781216 I updated the commit and also added arch/risc-v/src/esp32c3-legacy/Make.defs

[tmedicci]

LGTM!

[jerpelea]

please add commit messages

[simbit18]

Configuration/Tool: stm32f4discovery/nsh,CONFIG_ARM_TOOLCHAIN_GNU_EABI
2024-09-17 07:47:16

Cleaning...
Configuring...
make: *** [olddefconfig] Error 1
Enabling CONFIG_ARM_TOOLCHAIN_GNU_EABI
make: *** [olddefconfig] Error 1
Building NuttX...
make[1]: *** No rule to make target `context'.

It does not depend on this PR.
I will rebase to see if this can fix the CI error.

[simbit18]

please add commit messages

Hi @jerpelea after passing the CI I will squash the commits into a single commit with message.

[simbit18]

@jerpelea done