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/03/22 15:06:39 UTC
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request
#136: Let install depend on the target binary
xiaoxiang781216 opened a new pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136
to ensure the binary get built and install once
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] Ouss4 merged pull request #136: Let install
depend on the target binary
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396138947
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
Certainly this is a big risk because I don't see any reference to mkdir in any other Make* or *.mk file. We need to be careful 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396137838
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
1) mkdir should not be used explicitly. This is incompatible with Window native builds. You should define something.
2) If my recollection is correct, mkdir -p does not work on some Unix platforms, I think macOS.
Update: I am probably wrong about the incompatibilities. Tht would requrire some additional investigation to be certain of the compatibiltiey issues. Still usage of explict mkdir is not a good style.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396137838
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
1) mkdir should not be used explicitly. This is incompatible with Window native builds. You should define something.
2) If my recollection is correct, mkdir -p does not work on some Unix platforms, I think macOS.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396138947
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
Certainly this is a big risk because I don't see any reference to mkdir in any other Make* or *.mk file. We need to be careful here.
Hmm.. something wrong with my grep:
Makefile.unix: $(Q) mkdir -p staging
Makefile.win: $(Q) mkdir -p staging
Maybe there is no issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396137838
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
1) mkdir should not be used explicitly. This is incompatible with Window native builds. You should define something.
2) If my recollection is correct, mkdir -p does not work on some Unix platforms, I think macOS.
Update: I am probably wrong about the incompatibilities. Tht would requrire some additional investigation to be certain of the compatibiltiey issues. Still usage of explict mkdir is not a good style.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx-apps] patacongo commented on a change in pull
request #136: Let install depend on the target binary
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #136: Let install depend on the target binary
URL: https://github.com/apache/incubator-nuttx-apps/pull/136#discussion_r396138947
##########
File path: Application.mk
##########
@@ -151,6 +151,7 @@ PROGLIST := $(addprefix $(BINDIR)$(DELIM),$(PROGLIST))
PROGOBJ := $(MAINOBJ)
$(PROGLIST): $(MAINOBJ)
+ $(Q) mkdir -p $(BINDIR)
Review comment:
Certainly this is a big risk because I don't see any reference to mkdir in any other Make* or *.mk file. We need to be careful here.
Hmm.. something wrong with my grep:
Makefile.unix: $(Q) mkdir -p staging
Makefile.win: $(Q) mkdir -p staging
Maybe there is no issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services