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/20 04:29:45 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #1088: build: Move .config check to the top Makefile
xiaoxiang781216 opened a new pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088
remove the workaround to handle the inexistence of .config/Make.defs
Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
## Summary
## 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 #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427977026
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
is it enough to only check .config? I can add more code to check Make.defs but which make Makefile a little bit hard to read. Make.defs and .config appear at the same in most case. If something go to wrong, include Make.defs will stop immediately and report the problem cleanly.
----------------------------------------------------------------
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 #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427968570
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
No, the check still inside make, but the location change from tools/Makefile.unix or tools/Makefile.win to Makefile:
```
ifeq ($(wildcard .config),)
.DEFAULT:
@echo "Nuttx has not been configured:"
@echo " tools/configure.sh <target>"
clean distclean:
@:
else
include .config
ifeq ($(CONFIG_WINDOWS_NATIVE),y)
include tools/Makefile.win
else
include tools/Makefile.unix
endif
endif
```
If .config doesn't exist, .DEFAULT will output the same message as check_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 merged pull request #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088
----------------------------------------------------------------
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 #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427980276
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
I believe .config is enough since they are both copied at about the same time. I was just trying to see if this is what you had in mind and that I understood the change correctly.
----------------------------------------------------------------
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 #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427933292
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
@xiaoxiang781216 we will relay only on configure.sh (and the configuration step) to check if Make.defs exists? (I see that you removed the check_context 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] xiaoxiang781216 commented on a change in pull request #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427977026
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
is it enough to only check .config? I can add more code to check Make.defs but which make Makefile a little bit hard to read. Make.defs and .config appear at the same in most case. If something go to wrong, include Make.defs will stop immediately and report the problem clearly.
----------------------------------------------------------------
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 #1088: build: Move .config check to the top Makefile
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1088:
URL: https://github.com/apache/incubator-nuttx/pull/1088#discussion_r427970432
##########
File path: Makefile
##########
@@ -41,9 +41,18 @@
# make -f tools/Makefile.win
#
--include .config
+ifeq ($(wildcard .config),)
Review comment:
Yes, but that only checks for `.config` not `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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org