You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "simbit18 (via GitHub)" <gi...@apache.org> on 2024/09/05 14:17:04 UTC
[PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
simbit18 opened a new pull request, #13301:
URL: https://github.com/apache/nuttx/pull/13301
## 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 to remove nxtmpdir folder with third-party files.
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/bl602/Make.defs
but you can also add other packages (maybe even apps)
tools/configure.sh:
added flag -S to remove nxtmpdir folder with third-party files.
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/bl602/Make.defs
but you can also add other packages (maybe even apps)
@no1wudi @tmedicci
## Impact
There should be no impact.
## Testing
CI
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2340080756
>> 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 HAL commitment but I think to make it generic it has to be found in a script.
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.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2334375152
> 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?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1748038198
##########
tools/Unix.mk:
##########
@@ -19,6 +19,14 @@
############################################################################
export TOPDIR := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+WSDIR := ${shell cd "${TOPDIR}"/.. && pwd -P}
+
+export NXTMPDIR := $(WSDIR)/nxtmpdir
+$(info $$NXTMPDIR = $(NXTMPDIR))
+
+ifeq ($(wildcard $(NXTMPDIR)),)
+$(shell mkdir -p $(NXTMPDIR))
Review Comment:
but, it isn't good to always create a temp folder if esp isn't enable in defconfig.
BTW, it should be contained in ci specific script, Unix.mk it's too general.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2354816939
====================================================================================
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.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2332314555
Thanks @simbit18 . Let me test on our internal CI.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "hartmannathan (via GitHub)" <gi...@apache.org>.
hartmannathan commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2332179402
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?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2334417079
> > 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 !!! :)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1757223629
##########
tools/configure.sh:
##########
@@ -217,6 +223,19 @@ if [ -r ${dest_config} ]; then
fi
fi
+if [ "X${store_nxtmpdir}" = "Xy" ]; then
+ if [ ! -d "${WSDIR}/nxtmpdir" ]; then
+ mkdir -p "${WSDIR}/nxtmpdir"
+ echo "Folder nxtmpdir create."
Review Comment:
Should we print the whole path for the folder?
##########
tools/Config.mk:
##########
@@ -608,6 +620,42 @@ define DOWNLOAD
$(ECHO_END)
endef
+# 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))
Review Comment:
I would call it `CHECK_COMMITSHA`. Also, I think the feature of this function should be different: instead of using `git branch --contains`, I think we should use `git rev-parse HEAD` to get the latest commit's SHA-1 and compare it with the provided one. Note: the branch may contain the commit but may be at a newer HEAD and, then, we wouldn't be using the same commit as expected. To solve that we should stick with the HEAD only.
##########
tools/configure.sh:
##########
@@ -217,6 +223,19 @@ if [ -r ${dest_config} ]; then
fi
fi
+if [ "X${store_nxtmpdir}" = "Xy" ]; then
+ if [ ! -d "${WSDIR}/nxtmpdir" ]; then
+ mkdir -p "${WSDIR}/nxtmpdir"
+ echo "Folder nxtmpdir create."
Review Comment:
typo: `created`.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2354871986
> please add commit messages
Hi @jerpelea after passing the CI I will squash the commits into a single commit with message.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1751521548
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +146,14 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ $(Q) if [ -d "$(NXTMPDIR)" ]; then \
+ if [ ! -d "$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)" ]; then \
+ git clone --quiet $(ESP_HAL_3RDPARTY_URL) $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO); \
+ fi; \
+ cp -fr $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO) chip/$(ESP_HAL_3RDPARTY_REPO); \
+ else \
Review Comment:
@xiaoxiang781216 Yes, I think it needs to be optimized and later for cmake as well.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1745951820
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +146,10 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ $(Q) if [ ! -d "$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)" ]; then \
+ git clone --quiet $(ESP_HAL_3RDPARTY_URL) $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO); \
+ fi
+ $(Q) cp -fr $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO) chip/$(ESP_HAL_3RDPARTY_REPO)
Review Comment:
Or any equivalent logic: if the package exists and the version is the same, use it. Otherwise git clone it again.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2333645542
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 the case of compilation on GITHUB at each restart of the workflow for each job is:
1 created the temporary storage folder
2 cloned 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 compilation
1 the first compilation 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 third-party hal 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
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2338401103
Update
changed logic -S now creates the nxtmpdir folder for third-party packages.
.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
@tmedicci you can run a test on your internal CI.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #13301:
URL: https://github.com/apache/nuttx/pull/13301
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2355310713
@jerpelea done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2345808550
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.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1757258335
##########
tools/Config.mk:
##########
@@ -608,6 +620,42 @@ define DOWNLOAD
$(ECHO_END)
endef
+# 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))
Review Comment:
Forget about it: if it contains the commit, we would checkout to it soon. So, just some minor changes:
```suggestion
# CHECK_COMMITSHA - Check if the branch contains the commit SHA-1.
# 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 a unique SHA-1 hash value.
# Example: $(call CHECK_COMMITSHA,$(GIT_FOLDER),$(COMMIT_SHA-1))
```
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1750443665
##########
tools/Unix.mk:
##########
@@ -19,6 +19,14 @@
############################################################################
export TOPDIR := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+WSDIR := ${shell cd "${TOPDIR}"/.. && pwd -P}
+
+export NXTMPDIR := $(WSDIR)/nxtmpdir
+$(info $$NXTMPDIR = $(NXTMPDIR))
+
+ifeq ($(wildcard $(NXTMPDIR)),)
+$(shell mkdir -p $(NXTMPDIR))
Review Comment:
Hi @xiaoxiang781216
my idea to speed up build is not only about esp but in general all parts where git and curl is used even to apps (draft for lvgl https://github.com/apache/nuttx-apps/commit/cbc3cc42985efcfcc2e999500498f557f282c9e3) when possible.
Updated
.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
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2338936313
> 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 https://github.com/apache/nuttx/pull/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.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1745950926
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +146,10 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ $(Q) if [ ! -d "$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)" ]; then \
+ git clone --quiet $(ESP_HAL_3RDPARTY_URL) $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO); \
+ fi
+ $(Q) cp -fr $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO) chip/$(ESP_HAL_3RDPARTY_REPO)
Review Comment:
Let us combine the logic with the one in https://github.com/apache/nuttx/pull/13297. See: if we don't run `./tools/configure.sh -S` between two different NuttX versions, the build system will try to use the existing folder at the temp folder, but a different version may be required and this can be very dangerous (and hard to debug). So, we should an add an `else` to this clause to check if the version is the same.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1751162203
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +146,14 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ $(Q) if [ -d "$(NXTMPDIR)" ]; then \
+ if [ ! -d "$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)" ]; then \
+ git clone --quiet $(ESP_HAL_3RDPARTY_URL) $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO); \
+ fi; \
+ cp -fr $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO) chip/$(ESP_HAL_3RDPARTY_REPO); \
+ else \
Review Comment:
should we add a download function to avoid the code dup? cmake version need similar change too.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2333972925
> 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!
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1757282136
##########
tools/Config.mk:
##########
@@ -608,6 +620,42 @@ define DOWNLOAD
$(ECHO_END)
endef
+# 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))
Review Comment:
Also, I think we can add more verbosity when the folder is non-existing and when it exists but contains no reference for the commit.
##########
tools/Config.mk:
##########
@@ -608,6 +620,42 @@ define DOWNLOAD
$(ECHO_END)
endef
+# 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
Review Comment:
Same here: we can add more verbosity for the cases where we provide the storage folder and copy from the storage folder to the final path.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1750443665
##########
tools/Unix.mk:
##########
@@ -19,6 +19,14 @@
############################################################################
export TOPDIR := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+WSDIR := ${shell cd "${TOPDIR}"/.. && pwd -P}
+
+export NXTMPDIR := $(WSDIR)/nxtmpdir
+$(info $$NXTMPDIR = $(NXTMPDIR))
+
+ifeq ($(wildcard $(NXTMPDIR)),)
+$(shell mkdir -p $(NXTMPDIR))
Review Comment:
Hi @xiaoxiang781216
my idea to speed up build is not only about esp but in general all parts where git and curl is used even to apps (draft for lvgl https://github.com/apache/nuttx-apps/commit/cbc3cc42985efcfcc2e999500498f557f282c9e3) when possible.
update
.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
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1750443665
##########
tools/Unix.mk:
##########
@@ -19,6 +19,14 @@
############################################################################
export TOPDIR := ${shell echo $(CURDIR) | sed -e 's/ /\\ /g'}
+WSDIR := ${shell cd "${TOPDIR}"/.. && pwd -P}
+
+export NXTMPDIR := $(WSDIR)/nxtmpdir
+$(info $$NXTMPDIR = $(NXTMPDIR))
+
+ifeq ($(wildcard $(NXTMPDIR)),)
+$(shell mkdir -p $(NXTMPDIR))
Review Comment:
Hi @xiaoxiang781216
my idea to speed up build is not only about esp but in general all parts where git and curl is used even to apps (draft for lvgl https://github.com/apache/nuttx-apps/commit/cbc3cc42985efcfcc2e999500498f557f282c9e3) when possible.
.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
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2341299344
> 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 ;)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1752194937
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +146,14 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ $(Q) if [ -d "$(NXTMPDIR)" ]; then \
+ if [ ! -d "$(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO)" ]; then \
+ git clone --quiet $(ESP_HAL_3RDPARTY_URL) $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO); \
+ fi; \
+ cp -fr $(NXTMPDIR)/$(ESP_HAL_3RDPARTY_REPO) chip/$(ESP_HAL_3RDPARTY_REPO); \
+ else \
Review Comment:
but it may better start from this patch, since the dup already happen in three places
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2346737173
> 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
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#discussion_r1757320979
##########
arch/risc-v/src/common/espressif/Make.defs:
##########
@@ -146,7 +148,12 @@ endif
chip/$(ESP_HAL_3RDPARTY_REPO):
$(Q) echo "Cloning Espressif HAL for 3rd Party Platforms"
- $(Q) git clone --quiet $(ESP_HAL_3RDPARTY_URL) chip/$(ESP_HAL_3RDPARTY_REPO)
+ifeq ($(STORAGETMP),y)
Review Comment:
could we simply call one function with the enough arguments, which could do all thing internally?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] CI: Improvement to speed up compilation and reduce download errors. [nuttx]
Posted by "simbit18 (via GitHub)" <gi...@apache.org>.
simbit18 commented on PR #13301:
URL: https://github.com/apache/nuttx/pull/13301#issuecomment-2353017447
HI @tmedicci @xiaoxiang781216 I updated the commit and also added arch/risc-v/src/esp32c3-legacy/Make.defs
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org