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 2022/07/08 12:49:53 UTC
[GitHub] [incubator-nuttx-apps] masayuki2009 opened a new pull request, #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
masayuki2009 opened a new pull request, #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216
## Summary
- I noticed that some files (e.g. xxx.tar.gz) which are downloaded
by 'make context' are not deleted when we build the kernel only
then we do 'make distclean' because .depend is not generated.
- This commit fixes this issue by adding directories that contain
tar files.
## Impact
- None
## Testing
- Tested with sabre-6quad:netknsh (not merged yet)
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916813553
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
But this fix isn't extensible since not all downloaded package contain *.tar.*
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916839273
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
>But this fix isn't extensible since not all downloaded package contain .tar.
@xiaoxiang781216
How about creating the .context file when downloading a file such as .tar.gz or generating a new file such as config.h
Then .context will be checked when executing 'make distclean'
--
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
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916801543
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
Ah I see. Thanks for the explanation.
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#issuecomment-1179484688
@masayuki2009 how about we invoke "make distclean) in all subfolder like https://github.com/apache/incubator-nuttx-apps/pull/1217?
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916902257
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
Yes, I think it's better to generate .context in kernel mode just like other mode.
--
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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#issuecomment-1179497735
Yes, but it could ensure all clean/distclean target get executed. On the other hand, the method used to detect whether the directories is part of the build is fragile.
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#issuecomment-1198709403
Let me close this PR, since https://github.com/apache/incubator-nuttx/pull/6719 fixes the original 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916790281
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
Not sure why is this required. I see the discussion here https://github.com/apache/incubator-nuttx/pull/6586
`.depend` is created in the argtable3 directory and thus `distclean` should execute there and delete the downloaded files.
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916839273
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
>But this fix isn't extensible since not all downloaded package contain .tar.
@xiaoxiang781216
How about creating the .context file when downloading a file such as .tar.gz or generating a new file such as config.h Then .context will be checked when executing 'make distclean'
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on a diff in pull request #1216: Directory.mk: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on code in PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#discussion_r916795262
##########
Directory.mk:
##########
@@ -26,6 +26,7 @@ SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+CLEANSUBDIRS += $(dir $(wildcard *$(DELIM)*.tar.*))
Review Comment:
@Ouss4
'.depend' is NOT created when building the kernel (CONFIG_BUILD_KERNEL=y) only and will be created when we do 'make export'
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#issuecomment-1179415721
@xiaoxiang781216
Please review this PR 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
[GitHub] [incubator-nuttx-apps] masayuki2009 commented on pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on PR #1216:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216#issuecomment-1179492788
> @masayuki2009 how about we invoke "make distclean in all subfolder like #1217?
I think the PR will take a longer time because the PR always traverse all app directories which contain Makefile even though the nuttx configuration does not use them.
--
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
[GitHub] [incubator-nuttx-apps] masayuki2009 closed pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
Posted by GitBox <gi...@apache.org>.
masayuki2009 closed pull request #1216: apps: Fix make distclean for CONFIG_BUILD_KERNEL=y
URL: https://github.com/apache/incubator-nuttx-apps/pull/1216
--
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