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/11/10 17:25:55 UTC
[GitHub] [incubator-nuttx-apps] zouboan opened a new pull request, #1407: Fix various error in Windows native build
zouboan opened a new pull request, #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407
## Summary
These patch fine raised NuttX Windows native build
## Impact
Windows native build
## 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
[GitHub] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1042847487
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
Review Comment:
sorry, ci failed, we move line 21 as else statement at line 30, because ../apps/Make.defs will use `APPDIR`
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038433017
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+APPDIR = ${shell echo %CD%}
Review Comment:
Yes, you are right.
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029329680
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
@zouboan what's $(DELIM) on Windows native build environment? I think you can adjust $(DELIM) to the right value instead, which is more simpler than replacing it in many place.
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021698747
##########
Application.mk:
##########
@@ -189,8 +192,23 @@ install:: $(PROGLIST)
else
-MAINNAME := $(addsuffix _main,$(PROGNAME))
+MAINNAME := $(subst ",,$(addsuffix _main,$(PROGNAME)))
Review Comment:
quite right sir!
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029461232
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
In Windows environment `DELIM := \` but` \ `has two role in Windows environment:
first: `\ `as directory separator, and second `\` as Escape character
in Windows environment:
`BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))`
equivalent to
`BUILDIRS := $(dir $(wildcard $(APPDIR)\*\Make.defs))`
and the `\` of `\*` was identified to be Escape character, which result the `* ` was identified to be ordinary characters and not used for pattern. and then result the BUILDIRS to be empty.
As described in: [https://www.gnu.org/software/make/manual/html_node/Text-Functions.html](url)
we can use double $(DELIM) to solve problem, and i succeed resolved a similar problem in [https://github.com/apache/incubator-nuttx/pull/7572/commits/f91c2383d67ff89f694e51bc59d11f226764be46](url)
but when i use same method in apps/Make.defs , it give extra problem
@hartmannathan @xiaoxiang781216
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038199589
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+APPDIR = ${shell echo %CD%}
Review Comment:
do you means change Line 31: `APPDIR = ${shell echo %CD%}` to export `APPDIR = ${shell echo %CD%}` ?
--
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] hartmannathan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029335200
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
I think the reason for `$(DELIM)` is that paths on Windows use `\` and paths on Unix use `/`.
I'm not sure what problem the OP had with `$(DELIM)` but I wonder, could it be the choice of parenthesis? What's the difference between writing `$(DELIM)` or writing `${DELIM}`?
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1042386668
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
Done,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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1042770689
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
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.
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021549899
##########
Application.mk:
##########
@@ -62,7 +62,10 @@ ifeq ($(BUILD_MODULE),y)
endif
SUFFIX = $(subst $(DELIM),.,$(CWD))
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),)
PROGNAME := $(shell echo $(PROGNAME))
Review Comment:
can we change `$(subst ",,$(PROGNAME))` to avoid ifeq
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1019706048
##########
Directory.mk:
##########
@@ -22,11 +22,15 @@ include $(APPDIR)/Make.defs
# Sub-directories that have been built or configured.
-SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
-CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+SUBDIRS := $(dir $(wildcard */Makefile))
Review Comment:
result CONFIGSUBDIRS and CLEANSUBDIRS be empty set in Windows native 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1037923239
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+APPDIR = ${shell echo %CD%}
Review Comment:
if so, should we replace line 21 directly
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter $(4) of function REGISTER in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use $(subst ",,$(4)) to strip "" for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
but failed, may be we need more tricks here.
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021551238
##########
Application.mk:
##########
@@ -189,8 +192,23 @@ install:: $(PROGLIST)
else
-MAINNAME := $(addsuffix _main,$(PROGNAME))
+MAINNAME := $(subst ",,$(addsuffix _main,$(PROGNAME)))
Review Comment:
Do we still need change this line after https://github.com/apache/incubator-nuttx-apps/pull/1407/files#r1021549899
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038157687
##########
Makefile:
##########
@@ -83,11 +93,15 @@ else
# symbol table is required.
ifeq ($(CONFIG_BUILD_LOADABLE),)
-
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
+ $(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )
+else
$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
$(Q) for app in ${CONFIGURED_APPS}; do \
Review Comment:
the usage of `FOR` loop in Windows is:
`FOR %variable IN (set) DO command [command-parameters]`
to use the FOR command in a batch program, specify `%%variable` instead of `%variable`
[https://www.computerhope.com/forhlp.htm#:~:text=FOR%20%25variable%20IN%20%28set%29%20DO%20command%20%5Bcommand-parameters%5D%20To,are%20case-sensitive%2C%20so%20%25i%20is%20different%20from%20%25I.](url)
I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )` or I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C ${%%G} archive )` in Linux, but failed
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter `$(4)` of function `REGISTER` in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use `$(subst ",,$(4))` to strip `""` for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
in Linux but failed, may be we need more tricks here.
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter `$(4)` of function `REGISTER` in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use $(subst ",,$(4)) to strip "" for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
but failed, may be we need more tricks here.
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029461232
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
In Windows environment `DELIM := \` but` \ `has two role in Windows environment:
first: `\ `as directory separator, and second `\` as Escape character
in Windows environment:
`BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))`
equivalent to
`BUILDIRS := $(dir $(wildcard $(APPDIR)\*\Make.defs))`
and the `\` of `\*` was identified to be Escape character, which result the `* ` was identified to be ordinary characters not used for pattern. and then result the BUILDIRS to be empty.
As described in: [https://www.gnu.org/software/make/manual/html_node/Text-Functions.html](url)
we can use double $(DELIM) to solve problem, and i succeed resolved a similar problem in #f91c2383d67ff89f694e51bc59d11f226764be46
but when i use same method in apps/Make.defs , it give extra problem
@hartmannathan @xiaoxiang781216
--
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] [nuttx-apps] xiaoxiang781216 merged pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038157687
##########
Makefile:
##########
@@ -83,11 +93,15 @@ else
# symbol table is required.
ifeq ($(CONFIG_BUILD_LOADABLE),)
-
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
+ $(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )
+else
$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
$(Q) for app in ${CONFIGURED_APPS}; do \
Review Comment:
the usage of `FOR` loop in Windows is:
`FOR %variable IN (set) DO command [command-parameters]`
to use the FOR command in a batch program, specify `%%variable` instead of `%variable`
[https://www.computerhope.com/forhlp.htm#:~:text=FOR%20%25variable%20IN%20(set)%20DO%20command%20[command-parameters]%20To,are%20case-sensitive%2C%20so%20%25i%20is%20different%20from%20%25I.](url)
I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )` or `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C ${%%G} archive )` in Linux, but failed
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter `$(4)` of function `REGISTER` in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use `$(subst ",,$(4))` to strip "" for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
but failed, may be we need more tricks here.
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038432756
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
Ok, let's wait your result.
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1022368320
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
We can try whether windows style work on Linux/macOS and remove the ifeq if it work.
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021704509
##########
Application.mk:
##########
@@ -62,7 +62,10 @@ ifeq ($(BUILD_MODULE),y)
endif
SUFFIX = $(subst $(DELIM),.,$(CWD))
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),)
PROGNAME := $(shell echo $(PROGNAME))
Review Comment:
tested both on linux and windows native
--
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] hartmannathan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1024062479
##########
Directory.mk:
##########
@@ -22,11 +22,15 @@ include $(APPDIR)/Make.defs
# Sub-directories that have been built or configured.
-SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
-CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+SUBDIRS := $(dir $(wildcard */Makefile))
Review Comment:
Is `$(DELIM)` obsolete? Do all operating systems (including Windows) recognize `/` delimiter now?
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029461232
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
In Windows environment `DELIM := \` but` \ `has two role in Windows environment:
first: `\ `as directory separator, and second `\` as Escape character
in Windows environment:
`BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))`
equivalent to
`BUILDIRS := $(dir $(wildcard $(APPDIR)\*\Make.defs))`
and the `\` of `\*` was identified to be Escape character, which result the `* ` was identified to be ordinary characters not used for pattern. and then result the BUILDIRS to be empty.
As described in: [https://www.gnu.org/software/make/manual/html_node/Text-Functions.html](url)
we can use double $(DELIM) to solve problem, and i succeed resolved a similar problem in f91c2383d67ff89f694e51bc59d11f226764be46
but when i use same method in apps/Make.defs , it give extra problem
@hartmannathan @xiaoxiang781216
--
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] hartmannathan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029577374
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
@zouboan thanks for explaining. The `\` doing a double purpose here is indeed a problem.
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result "int nsh_main(int argc, char *argv[]);" in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter $(4) of function REGISTER in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use $(subst ",,$(4)) to strip "" for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
but failed, may be we need more tricks here.
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038208846
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
is a good idea! although i'm not expert in .bat and .sh but I wouldn't mind having a try
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter `$(4)` of function `REGISTER` in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use `$(subst ",,$(4))` to strip `""` for Windows, otherwise there will be: "nsh"_main and sh_main and "hello"_main, the first and the third will give error.
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
in Linux but failed, may be we need more tricks here.
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038770468
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
but should we use the same approach(replace / with \?
--
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] pkarashchenko commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1019697370
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
-CLEANDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Makefile))
+BUILDIRS := $(dir $(wildcard $(APPDIR)/*/Make.defs))
Review Comment:
ditto
##########
Directory.mk:
##########
@@ -22,11 +22,15 @@ include $(APPDIR)/Make.defs
# Sub-directories that have been built or configured.
-SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
-CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+SUBDIRS := $(dir $(wildcard */Makefile))
Review Comment:
Why `$(DELIM)` is not working here?
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1037922304
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
can we share the same code, the difference look like very small.
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021553795
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
can we unify the code as much as possible instead check CONFIG_WINDOWS_NATIVE in many place?
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1023740880
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
I'm tried, but failed sadly!
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1024159910
##########
Directory.mk:
##########
@@ -22,11 +22,15 @@ include $(APPDIR)/Make.defs
# Sub-directories that have been built or configured.
-SUBDIRS := $(dir $(wildcard *$(DELIM)Makefile))
-CONFIGSUBDIRS := $(filter-out $(dir $(wildcard *$(DELIM)Kconfig)),$(SUBDIRS))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).depend))
-CLEANSUBDIRS += $(dir $(wildcard *$(DELIM).kconfig))
+SUBDIRS := $(dir $(wildcard */Makefile))
Review Comment:
> Is `$(DELIM)` obsolete? Do all operating systems (including Windows) recognize `/` delimiter now?
Windows is not a POSIX environment, some makefile command was not support in Windows, As suggest in nuttx/README we use GnuWin32 to supply this windows version Makefile commad(for example: wildcard), some special cases we must use / rather than \ ,for here, we must use /, otherwise CONFIGSUBDIRS and CLEANSUBDIRS would be empty set in Windows native build, and will give fatal error, but use / we can build succesfully.
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029355658
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
because define.bat is different from the define.sh, and usage is also different this is usage of define.bat:
`echo USAGE:%progname% [-h] ^<compiler-path^> [-val ^<^val1^>] [^<def2^> [-val ^<val2^>] [^<def3^> [-val ^<val3^>] ...]]
echo Where:"
echo ^<compiler-path^>
echo The full path to your ccpath
echo ^<def1^> ^<def2^> ^<def3^> ...
echo A list of pre-preprocesser variable names to be defined.
echo [-val ^<val1^>] [-val ^<val2^>] [-val ^<val3^>] ...
echo optional values to be assigned to each pre-processor variable.
echo If not supplied, the variable will be defined with no explicit value.`
without -val, there will fatal error in Windows native build, for example:
Dnsh_main become Dnsh Dmain
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029329680
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
@zouboan what's `$(DELIM)` on Windows native build environment? I think you can adjust `$(DELIM)` to the right value instead, which is more simpler than replacing it in many place.
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029355658
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
@xiaoxiang781216
because define.bat is different from the define.sh, and usage is also different.
this is usage of define.bat:
`echo USAGE:%progname% [-h] ^<compiler-path^> [-val ^<^val1^>] [^<def2^> [-val ^<val2^>] [^<def3^> [-val ^<val3^>] ...]]
echo Where:"
echo ^<compiler-path^>
echo The full path to your ccpath
echo ^<def1^> ^<def2^> ^<def3^> ...
echo A list of pre-preprocesser variable names to be defined.
echo [-val ^<val1^>] [-val ^<val2^>] [-val ^<val3^>] ...
echo optional values to be assigned to each pre-processor variable.
echo If not supplied, the variable will be defined with no explicit value.`
without -val, there will fatal error in Windows native build, for example:
Dnsh_main become Dnsh Dmain
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1042506618
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
Review Comment:
```suggestion
include $(CURDIR)/Make.defs
```
and move line 21 as else statement at line 30
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029461232
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
In Windows environment `DELIM := \` but` \ `has two role in Windows environment:
first: `\ `as directory separator, and second `\` as Escape character
in Windows environment:
`BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))`
equivalent to
`BUILDIRS := $(dir $(wildcard $(APPDIR)\*\Make.defs))`
and the `\` of `\*` was identified to be Escape character, which result the `* ` was identified to be ordinary characters not used for pattern. and then result the BUILDIRS to be empty.
As described in: [https://www.gnu.org/software/make/manual/html_node/Text-Functions.html](url)
we can use double $(DELIM) to solve problem, and i succeed resolved a similar problem in [https://github.com/apache/incubator-nuttx/pull/7572/commits/f91c2383d67ff89f694e51bc59d11f226764be46](url)
but when i use same method in apps/Make.defs , it give extra problem
@hartmannathan @xiaoxiang781216
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038192710
##########
Make.defs:
##########
@@ -85,14 +80,24 @@ endif
BUILTIN_REGISTRY = $(APPDIR)$(DELIM)builtin$(DELIM)registry
DEPCONFIG = $(TOPDIR)$(DELIM).config
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
define REGISTER
$(Q) echo Register: $1
- $(Q) echo { \"$1\", $2, $3, $4 }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
+ $(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
+
+ $(Q) touch $(BUILTIN_REGISTRY)$(DELIM).updated"
+endef
+else
+define REGISTER
+ $(Q) echo "Register: $1"
+ $(Q) echo "{ \"$1\", $2, $3, $4 }," > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
Review Comment:
In Windows:
`echo aaaaaaaaaaa `result `aaaaaaaaaaa`
`echo 'bbbbbb'` result `'bbbbbb'`
`echo "ccccccccc"` result `"ccccccccc"`
so:
`echo "int nsh_main(int argc, char *argv[]);" > "$(BUILTIN_REGISTRY)$(DELIM)nsh.pdat";` result `"int nsh_main(int argc, char *argv[]);"` in nsh.pdat, and will give an build error.
what's more nsh name generated by Kconfig is "nsh"
in ../apps/system/nsh/Makefile:
`PROGNAME = $(CONFIG_SYSTEM_NSH_PROGNAME) sh`
in ../apps/example/hello/Makefile:
`PROGNAME = $(CONFIG_EXAMPLES_HELLO_PROGNAME)`
and CONFIG_EXAMPLES_HELLO_PROGNAME in Kconfig is "hello"
which result the parameter `$(4)` of function `REGISTER` in ../apps/Make.defs are:
"nsh" sh "hello" ,we have use `$(subst ",,$(4))` to strip `""` for Windows
I tried to use :
```
$(Q) echo { "$(subst ",,$(1))", $2, $3, $(subst ",,$(4)) }, > "$(BUILTIN_REGISTRY)$(DELIM)$1.bdat"
$(Q) echo int $(subst ",,$(4))(int argc, char *argv[]); > "$(BUILTIN_REGISTRY)$(DELIM)$1.pdat"
```
but failed, may be we need more tricks here.
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1037921466
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
@zouboan it's betteer to sync define.bat and define.sh usage to avoid the duplication here.
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038157687
##########
Makefile:
##########
@@ -83,11 +93,15 @@ else
# symbol table is required.
ifeq ($(CONFIG_BUILD_LOADABLE),)
-
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
+ $(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )
+else
$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
$(Q) for app in ${CONFIGURED_APPS}; do \
Review Comment:
the usage of `FOR` loop in Windows is:
`FOR %variable IN (set) DO command [command-parameters]`
to use the FOR command in a batch program, specify `%%variable` instead of `%variable`
[https://www.computerhope.com/forhlp.htm#:~:text=FOR%20%25variable%20IN%20(set)%20DO%20command%20[command-parameters]%20To,are%20case-sensitive%2C%20so%20%25i%20is%20different%20from%20%25I.](url)
I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )` or I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C ${%%G} archive )` in Linux, but failed
--
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 #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1025500954
##########
Application.mk:
##########
@@ -191,6 +192,21 @@ else
MAINNAME := $(addsuffix _main,$(PROGNAME))
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(MAINCXXOBJ): %$(CXXEXT)$(SUFFIX)$(OBJEXT): %$(CXXEXT)
+ $(eval $<_CXXFLAGS += ${shell $(DEFINE) "$(CXX)" main -val $(firstword $(MAINNAME))})
Review Comment:
why need -var, do you use a special toolchain?
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021702907
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
i have no good idea now, perhaps we can improve it in future?
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1021697815
##########
Application.mk:
##########
@@ -62,7 +62,10 @@ ifeq ($(BUILD_MODULE),y)
endif
SUFFIX = $(subst $(DELIM),.,$(CWD))
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),)
PROGNAME := $(shell echo $(PROGNAME))
Review Comment:
is really a good idea!
--
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1019706274
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
-CLEANDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Makefile))
+BUILDIRS := $(dir $(wildcard $(APPDIR)/*/Make.defs))
Review Comment:
result CONFIGDIRS and BUILDIRS be empty set in Windows native 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.
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] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1407#discussion_r1029461232
##########
Make.defs:
##########
@@ -21,27 +21,22 @@
TOPDIR ?= $(APPDIR)/import
include $(TOPDIR)/Make.defs
-# The GNU make CURDIR will always be a POSIX-like path with forward slashes
-# as path segment separators. This is fine for the above inclusions but
-# will cause problems later for the native build. If we know that this is
-# a native build, then we need to fix up the APPDIR path for subsequent
-# use
-
-ifeq ($(CONFIG_WINDOWS_NATIVE),y)
-APPDIR := ${shell echo %CD%}
-endif
-
# Application Directories
# BUILDIRS is the list of top-level directories containing Make.defs files
# CLEANDIRS is the list of all top-level directories containing Makefiles.
# It is used only for cleaning.
-BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))
-BUILDIRS := $(filter-out $(APPDIR)$(DELIM)import$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(APPDIR)$(DELIM)builtin$(DELIM),$(BUILDIRS))
-CONFIGDIRS := $(filter-out $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Kconfig)),$(CONFIGDIRS))
Review Comment:
In Windows environment `DELIM := \` but` \ `has two role in Windows environment:
first: `\ `as directory separator, and second `\` as Escape character
in Windows environment:
`BUILDIRS := $(dir $(wildcard $(APPDIR)$(DELIM)*$(DELIM)Make.defs))`
equivalent to
`BUILDIRS := $(dir $(wildcard $(APPDIR)\*\Make.defs))`
and the `\` of `\*` was identified to be Escape character, which result the `* ` was identified to be ordinary characters not used for pattern. and then result the BUILDIRS to be empty.
As described in: [https://www.gnu.org/software/make/manual/html_node/Text-Functions.html](url)
we can use double $(DELIM) to solve problem, and i succeed resolved a similar problem in commit f91c2383d67ff89f694e51bc59d11f226764be46
but when i use same method in apps/Make.defs , it give extra problem
@hartmannathan @xiaoxiang781216
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1040802577
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
we can replace / with \ like other place, instead `${shell echo %CD%}`.
and it's better to merge with line 31
--
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] [nuttx-apps] zouboan commented on pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#issuecomment-1339537307
thanks for your advise，let me try!
---Original---
From: "Xiang ***@***.***>
Date: Tue, Dec 6, 2022 18:45 PM
To: ***@***.***>;
Cc: ***@***.******@***.***>;
Subject: Re: [apache/nuttx-apps] Fix various error in Windows native build (PR #1407)
@xiaoxiang781216 commented on this pull request.
In Makefile:
> @@ -21,6 +21,16 @@ export APPDIR = $(CURDIR) include $(APPDIR)/Make.defs +# The GNU make CURDIR will always be a POSIX-like path with forward slashes +# as path segment separators. This is fine for the above inclusions but +# will cause problems later for the native build. If we know that this is +# a native build, then we need to fix up the APPDIR path for subsequent +# use + +ifeq ($(CONFIG_WINDOWS_NATIVE),y)
we can replace / with \ like other place, instead ${shell echo %CD%}.
and it's better to merge with line 31
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1042847487
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
Review Comment:
sorry, ci failed, we can't move line 21 as else statement at line 30, because ../apps/Make.defs will use `APPDIR`
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038157687
##########
Makefile:
##########
@@ -83,11 +93,15 @@ else
# symbol table is required.
ifeq ($(CONFIG_BUILD_LOADABLE),)
-
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
+ $(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )
+else
$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
$(Q) for app in ${CONFIGURED_APPS}; do \
Review Comment:
the usage of `FOR` loop in Windows is:
`FOR %variable IN (set) DO command [command-parameters]`
to use the FOR command in a batch program, specify `%%variable` instead of `%variable`
I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )` or I tried to use `$(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C ${%%G} archive )` in Linux, but failed
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038199589
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+APPDIR = ${shell echo %CD%}
Review Comment:
do you means change Line 31: `APPDIR = ${shell echo %CD%}` to `export APPDIR = ${shell echo %CD%}` ?
--
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] [nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1037924651
##########
Makefile:
##########
@@ -83,11 +93,15 @@ else
# symbol table is required.
ifeq ($(CONFIG_BUILD_LOADABLE),)
-
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
+$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
+ $(Q) for %%G in ($(CONFIGURED_APPS)) do ( $(MAKE) -C %%G archive )
+else
$(BIN): $(foreach SDIR, $(CONFIGURED_APPS), $(SDIR)_all)
$(Q) for app in ${CONFIGURED_APPS}; do \
Review Comment:
what's problem is fixed by change app to %%G
--
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] [nuttx-apps] zouboan commented on a diff in pull request #1407: Fix various error in Windows native build
Posted by GitBox <gi...@apache.org>.
zouboan commented on code in PR #1407:
URL: https://github.com/apache/nuttx-apps/pull/1407#discussion_r1038771434
##########
Makefile:
##########
@@ -21,6 +21,16 @@
export APPDIR = $(CURDIR)
include $(APPDIR)/Make.defs
+# The GNU make CURDIR will always be a POSIX-like path with forward slashes
+# as path segment separators. This is fine for the above inclusions but
+# will cause problems later for the native build. If we know that this is
+# a native build, then we need to fix up the APPDIR path for subsequent
+# use
+
+ifeq ($(CONFIG_WINDOWS_NATIVE),y)
Review Comment:
Sorry, what do you mean? I don't follow...
Can you give me some hints?
--
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