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/09/18 16:51:31 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
anchao opened a new pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843
## Summary
make/clean: clean the OBJ directly if declare in subdirectory
Discussion on https://github.com/apache/incubator-nuttx-apps/pull/395
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
make clean
## Testing
build nutxx and make clean
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#issuecomment-716169470
Don't need after #2101 get merged.
----------------------------------------------------------------
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] anchao commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491106203
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
Yes, I tried. Clean the *$(OBJEXT) is necessary ,the new logic will only clear the objects with VPATH prefix. Removing the clean operation will cause the objects in the nuttx repo to not be deleted
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491472483
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
Not sure what you mean. Do you mean that we might have leftover .o's if you: make, configure (disable something), clean?
If so, yes, it could happen, but is the goal of `clean` to give a pristine workspace or just clean what you just built?
An alternative is to have both *$(OBJEXT) and $(OBJS). That considers both scenarios although there would be overlap between those two in some cases.
----------------------------------------------------------------
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 #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491481405
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
If we don't consider reconfigure which may change $(OBJS), it's enough to just "rm $(OBJS)". On the other hand, we have to "rm *$(OBJEXT)" for the current working directory and it's child directories.
OBJEXT and LIBEXIT is always defined here:
https://github.com/apache/incubator-nuttx/blob/master/tools/Config.mk#L108
so it should be safe.
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491460194
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
Can you explain why you mention this didn't work for you?
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491110240
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
Mmm, I think I would have to look further into this. Can you give me some time to try it?
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491089510
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
Can you remove $(OBJEXT) here? The above rule should clean all objs, that is what I want to 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491460145
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
hi, I'm testing the approach of just cleaning $(OBJS) and $(BIN) like:
```
define CLEAN
$(Q) rm -f *~ .*.swp $(OBJS) $(BIN)
endef
```
It is mostly working (there is one particular app whose objects remain, I'm looking into it). It is even cleaning a third-party application I have, which I'm building by doing something like `CSRCS += subdirectory/file.c` without adding subdirectory to VPATH.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491471878
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
> hi, I'm testing the approach of just cleaning $(OBJS) and $(BIN) like:
>
> ```
> define CLEAN
> $(Q) rm -f *~ .*.swp $(OBJS) $(BIN)
> endef
> ```
Doesn't this have the possibility of doing a back CLEAN after a reconfiguration? The OBJS many not be the same after a reconfiguration as they were before the reconfiguration.
----------------------------------------------------------------
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 closed pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491473802
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
So, I'm confused on what do you think is best. I agree that not having *$(OBJEXT) is better and safer, and for me it works just using $(OBJS) (but potentially leaves extra .o's there). However, you then mentioned to add it in a second step, so I'm not following your 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491481823
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
But $(OBJS) will always include the objects for current directory. As I mentioned, if you only remove $(OBJS) no stray objects remain (unless you reconfigure in the middle). Maybe distclean should use a separate macro that uses the wildcard, however it should also recurse into every subdirectory if we do not want ANY .o remaining.
----------------------------------------------------------------
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] v01d commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491089349
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
Review comment:
My idea was not to use VPATH for this. The $(OBJ) should have a path relative to where this CLEAN macro is executed if I understand 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] btashton commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491481695
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
> > An alternative is to have both *$(OBJEXT) and $(OBJS). That considers both scenarios although there would be overlap between those two in some cases.
>
> *$(OBJEXT) is not safe. What if it has no value? Catastrophe follows (this has happened to me!)
>
> There would be no overlap if if the wildcard based clean were done second.
Can we conditionally do the rm if $(OBJEXT) is set or wrap it with a helper and error? Seems like $(OBJEXT) not being set is a sign something went very wrong.
----------------------------------------------------------------
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] patacongo commented on a change in pull request #1843: make/clean: clean the OBJ directly if declare in subdirectory
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1843:
URL: https://github.com/apache/incubator-nuttx/pull/1843#discussion_r491473196
##########
File path: tools/Config.mk
##########
@@ -458,7 +458,8 @@ define CLEAN
endef
else
define CLEAN
- $(Q) rm -f *$(OBJEXT) *$(LIBEXT) *~ .*.swp
+ $(call DELFILE, $(wildcard $(foreach obj, $(OBJS), $(addsuffix /$(obj), $(subst :, ,$(VPATH))))))
+ $(call DELFILE, *$(OBJEXT) *$(LIBEXT) *~ .*.swp)
Review comment:
> An alternative is to have both *$(OBJEXT) and $(OBJS). That considers both scenarios although there would be overlap between those two in some cases.
*$(OBJEXT) is not safe. What if it has no value? Catastrophe follows (this has happened to me!)
There would be no overlap if if the wildcard based clean were done second.
----------------------------------------------------------------
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