You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/10/27 18:33:08 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2115: Store artifacts durring build

xiaoxiang781216 commented on a change in pull request #2115:
URL: https://github.com/apache/incubator-nuttx/pull/2115#discussion_r512931720



##########
File path: tools/testbuild.sh
##########
@@ -238,6 +246,11 @@ function configure {
 function build {
   echo "  Building NuttX..."
   makefunc
+  if [ ${SAVEARTIFACTS} -eq 1 ]; then
+    artifactconfigdir=$ARTIFACTDIR/$(echo $config | sed "s/:/\//")/
+    mkdir -p $artifactconfigdir
+    cp $(${MAKE} execname) $artifactconfigdir

Review comment:
       should we copy more file here? e.g. nuttx.bin, System.map.. which is useful to debug the problem.

##########
File path: Makefile
##########
@@ -45,3 +45,7 @@ else
 include tools/Makefile.unix
 endif
 endif
+
+.PHONY: execname
+execname:
+	@echo $(BIN)

Review comment:
       why not copy the binary directly? it's strange to print the binary name in Makefile

##########
File path: tools/testbuild.sh
##########
@@ -62,6 +66,7 @@ function showusage {
   echo "  -a <appsdir> provides the relative path to the apps/ directory.  Default ../apps"
   echo "  -t <topdir> provides the absolute path to top nuttx/ directory.  Default $PWD/../nuttx"
   echo "  -p only print the list of configs without running any builds"
+  echo "  -A store the build executable artifact in ARTIFACTDIR (defaults to $PWD/../buildartifacts"

Review comment:
       let's remove $PWD here and line 67 since no other place use it in the whole script

##########
File path: .github/workflows/build.yml
##########
@@ -161,8 +161,13 @@ jobs:
             export CCACHE_DIR=`pwd`/ccache
             mkdir $CCACHE_DIR
             cd sources/testing
-            ./cibuild.sh -c testlist/${{matrix.boards}}.dat
+            export ARTIFACTDIR=`pwd`/../../buildartifacts

Review comment:
       Remove all `pwd`/ since other place use the relative 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org