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/05/19 04:34:33 UTC
[GitHub] [incubator-nuttx] liuguo09 opened a new pull request #1072: Update make dependency to enable configure.sh stage with parallel build
liuguo09 opened a new pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072
## Summary
Make './configure.sh config -jn' pass build, it also reduce configure.sh stage time since with multiple jobs in parallel build.
## Impact
## Testing
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427890945
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
Let' dirlinks depends on clear_context?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427508919
##########
File path: tools/Makefile.unix
##########
@@ -506,42 +506,28 @@ pass2dep: context tools/mkdeps$(HOSTEXEEXT) tools/cnvwindeps$(HOSTEXEEXT)
# location: https://bitbucket.org/nuttx/tools/downloads/. See README.txt
# file in the NuttX tools GIT repository for additional information.
-do_config: dirlinks apps_preconfig
+config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf Kconfig
-config: do_config clean_context
-
-do_oldconfig: dirlinks apps_preconfig
+oldconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --oldconfig Kconfig
-oldconfig: do_oldconfig clean_context
-
-do_olddefconfig: dirlinks apps_preconfig
+olddefconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig
-olddefconfig: do_olddefconfig clean_context
-
-do_menuconfig: dirlinks apps_preconfig
+menuconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-mconf Kconfig
-menuconfig: do_menuconfig clean_context
Review comment:
In apps `clean_context` deletes *.pdat and *.bdat files. When removing an app through menuconfig for example, without `clean_context` the app won't be removed.
And a `make` after a `clean` would fail with an undefined reference, since the `built_list/proto.h` are removed (by `clean`) then repopulated (during `depend`) with all the *.pdat and *.bdat file.
Is removing `clean_context` necessary in a parallel build scenario?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r431031494
##########
File path: tools/copydir.sh
##########
@@ -96,5 +96,6 @@ fi
cp -a "${src}" "${dest}" || \
{ echo "Failed to create link: $dest" ; rm -rf ${dest} ; exit 1 ; }
+touch "${src}"
Review comment:
Well, I'll update them into Makefile later.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r428012784
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
Now that Liu removed dirlinks from the kconfig targets (which were the primary users of dirlinks + clean_context), is it still an issue?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#issuecomment-635063650
LGTM.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r429741377
##########
File path: tools/Makefile.unix
##########
@@ -315,6 +321,7 @@ include/arch/chip: include/arch
ifneq ($(CONFIG_ARCH_CHIP),)
@echo "LN: include/arch/chip to $(ARCH_INC)/$(CONFIG_ARCH_CHIP)"
$(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_INC)/$(CONFIG_ARCH_CHIP) include/arch/chip
+ $(Q) touch $@
Review comment:
How about we move touch to link.sh?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r431563729
##########
File path: tools/copydir.sh
##########
@@ -96,5 +96,6 @@ fi
cp -a "${src}" "${dest}" || \
{ echo "Failed to create link: $dest" ; rm -rf ${dest} ; exit 1 ; }
+touch "${src}"
Review comment:
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427942023
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
> Let' dirlinks depends on clear_context?
Wouldn't that mess up context? It would need some re-organization in it's prerequisites.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427725359
##########
File path: tools/Makefile.unix
##########
@@ -506,42 +506,28 @@ pass2dep: context tools/mkdeps$(HOSTEXEEXT) tools/cnvwindeps$(HOSTEXEEXT)
# location: https://bitbucket.org/nuttx/tools/downloads/. See README.txt
# file in the NuttX tools GIT repository for additional information.
-do_config: dirlinks apps_preconfig
+config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf Kconfig
-config: do_config clean_context
-
-do_oldconfig: dirlinks apps_preconfig
+oldconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --oldconfig Kconfig
-oldconfig: do_oldconfig clean_context
-
-do_olddefconfig: dirlinks apps_preconfig
+olddefconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig
-olddefconfig: do_olddefconfig clean_context
-
-do_menuconfig: dirlinks apps_preconfig
+menuconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-mconf Kconfig
-menuconfig: do_menuconfig clean_context
Review comment:
Since './tools/configure.sh config' stage depends on the 'dirlinks' stage and do not depend on the 'context' stage. From './tools/configure.sh config V=1', it show no 'context' built. So I think 'clean_context' may not necessary.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427971032
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
Yes, the right sequences is:
```
clear_context: .config
include/arch: clear_context
....
dirlinks: include/arch ...
```
the old rule is wrong:
```
dirlinks: clear_context include/arch ...
```
because clear_context and include/arch hasn't relationship in the concurrent build.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r430923288
##########
File path: tools/copydir.sh
##########
@@ -96,5 +96,6 @@ fi
cp -a "${src}" "${dest}" || \
{ echo "Failed to create link: $dest" ; rm -rf ${dest} ; exit 1 ; }
+touch "${src}"
Review comment:
i guess these belong to Makefile if any.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427856742
##########
File path: tools/Makefile.unix
##########
@@ -506,42 +506,28 @@ pass2dep: context tools/mkdeps$(HOSTEXEEXT) tools/cnvwindeps$(HOSTEXEEXT)
# location: https://bitbucket.org/nuttx/tools/downloads/. See README.txt
# file in the NuttX tools GIT repository for additional information.
-do_config: dirlinks apps_preconfig
+config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf Kconfig
-config: do_config clean_context
-
-do_oldconfig: dirlinks apps_preconfig
+oldconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --oldconfig Kconfig
-oldconfig: do_oldconfig clean_context
-
-do_olddefconfig: dirlinks apps_preconfig
+olddefconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig
-olddefconfig: do_olddefconfig clean_context
-
-do_menuconfig: dirlinks apps_preconfig
+menuconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-mconf Kconfig
-menuconfig: do_menuconfig clean_context
Review comment:
Done, please take a look. Thanks
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r428031192
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
But it isn't good to launch a new instance(same make file) to build dirlinks target.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r428062115
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
Is this PR going to be updated with these changes?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r428405262
##########
File path: tools/Makefile.win
##########
@@ -565,7 +561,7 @@ endif
# apps_distclean: Perform the distclean operation only in the user application
# directory.
-apps_preconfig:
+apps_preconfig: clean_context
Review comment:
Yes, I'm doing with these changes, it may need more test and verify locally.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427968151
##########
File path: tools/Makefile.unix
##########
@@ -506,42 +506,28 @@ pass2dep: context tools/mkdeps$(HOSTEXEEXT) tools/cnvwindeps$(HOSTEXEEXT)
# location: https://bitbucket.org/nuttx/tools/downloads/. See README.txt
# file in the NuttX tools GIT repository for additional information.
-do_config: dirlinks apps_preconfig
+config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf Kconfig
-config: do_config clean_context
-
-do_oldconfig: dirlinks apps_preconfig
+oldconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --oldconfig Kconfig
-oldconfig: do_oldconfig clean_context
-
-do_olddefconfig: dirlinks apps_preconfig
+olddefconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig
-olddefconfig: do_olddefconfig clean_context
-
-do_menuconfig: dirlinks apps_preconfig
+menuconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-mconf Kconfig
-menuconfig: do_menuconfig clean_context
Review comment:
Thanks, Liu. It looks okay as far as the scenario I was talking about is concerned.
Does this fix your issue?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r427728199
##########
File path: tools/Makefile.unix
##########
@@ -506,42 +506,28 @@ pass2dep: context tools/mkdeps$(HOSTEXEEXT) tools/cnvwindeps$(HOSTEXEEXT)
# location: https://bitbucket.org/nuttx/tools/downloads/. See README.txt
# file in the NuttX tools GIT repository for additional information.
-do_config: dirlinks apps_preconfig
+config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf Kconfig
-config: do_config clean_context
-
-do_oldconfig: dirlinks apps_preconfig
+oldconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --oldconfig Kconfig
-oldconfig: do_oldconfig clean_context
-
-do_olddefconfig: dirlinks apps_preconfig
+olddefconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig
-olddefconfig: do_olddefconfig clean_context
-
-do_menuconfig: dirlinks apps_preconfig
+menuconfig: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-mconf Kconfig
-menuconfig: do_menuconfig clean_context
Review comment:
@Ouss4 Sorry, just get your point. I'll think about it how to update the PR later.
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r430867899
##########
File path: tools/copydir.sh
##########
@@ -96,5 +96,6 @@ fi
cp -a "${src}" "${dest}" || \
{ echo "Failed to create link: $dest" ; rm -rf ${dest} ; exit 1 ; }
+touch "${src}"
Review comment:
why?
----------------------------------------------------------------
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
[GitHub] [incubator-nuttx] liuguo09 commented on a change in pull request #1072: Update make dependency to enable configure.sh stage with parallel build
Posted by GitBox <gi...@apache.org>.
liuguo09 commented on a change in pull request #1072:
URL: https://github.com/apache/incubator-nuttx/pull/1072#discussion_r430917555
##########
File path: tools/copydir.sh
##########
@@ -96,5 +96,6 @@ fi
cp -a "${src}" "${dest}" || \
{ echo "Failed to create link: $dest" ; rm -rf ${dest} ; exit 1 ; }
+touch "${src}"
Review comment:
Just want to make sure include/arch etc targets are newer than .clean_context in dirlink stage. To avoid link more times in build. It seems link.sh/copydir.sh scripts only used in Makefile.unix dirlinks.
----------------------------------------------------------------
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