Re: [PATCH] Makefile: don't run rm without any files
Junio C Hamano wrote: > I amended the log message like so: > > commit bd9df384b16077337fffe9836c9255976b0e7b91 > Author: Matt Kraai > Date: Wed Feb 13 07:57:48 2013 -0800 > > Makefile: don't run rm without any files > > When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler > does not support it, $(dep_dirs) becomes empty. "make clean" runs > "rm -rf $(dep_dirs)", which fails in such a case. To pedantic, that only fails on some platforms. The autoconf manual explains: It is not portable to invoke rm without options or operands. On the other hand, Posix now requires rm -f to silently succeed when there are no operands (useful for constructs like rm -rf $filelist without first checking if ‘$filelist’ was empty). But this was not always portable; at least NetBSD rm built before 2008 would fail with a diagnostic. Anyway, looks like a good fix. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't run rm without any files
Matt Kraai writes: > I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto". > The automatic detection determines that the compiler doesn't support > it, so it's then set to "no". CHECK_HEADER_DEPENDENCIES isn't set > either, so about 20 lines below the dep_dirs assignment you quoted, > dep_dirs is cleared: > > ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes) > ifndef CHECK_HEADER_DEPENDENCIES > dep_dirs = > ... > > Should I submit an updated patch with a different commit message? I amended the log message like so: commit bd9df384b16077337fffe9836c9255976b0e7b91 Author: Matt Kraai Date: Wed Feb 13 07:57:48 2013 -0800 Makefile: don't run rm without any files When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler does not support it, $(dep_dirs) becomes empty. "make clean" runs "rm -rf $(dep_dirs)", which fails in such a case. Signed-off-by: Matt Kraai Signed-off-by: Junio C Hamano -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't run rm without any files
On Wed, Feb 13, 2013 at 08:51:45AM -0800, Junio C Hamano wrote: > Matt Kraai writes: > > > From: Matt Kraai > > > > "rm -f -r" fails on QNX when not passed any files to remove. > > I do not think it is limited to QNX. > > > the clean target, since dep_dirs is empty. > > And dep_dirs being empty under some circumstance shouldn't be > limited to QNX, either. > > I think your change does no harm, may be a good change if dep_dirs > goes empty, but the justification is lacking. What caused your > dep_dirs to become empty in the first place? > > I am scratching my head because I see > > OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ > $(XDIFF_OBJS) \ > $(VCSSVN_OBJS) \ > git.o > dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto". The automatic detection determines that the compiler doesn't support it, so it's then set to "no". CHECK_HEADER_DEPENDENCIES isn't set either, so about 20 lines below the dep_dirs assignment you quoted, dep_dirs is cleared: ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes) ifndef CHECK_HEADER_DEPENDENCIES dep_dirs = ... Should I submit an updated patch with a different commit message? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Makefile: don't run rm without any files
Matt Kraai writes: > From: Matt Kraai > > "rm -f -r" fails on QNX when not passed any files to remove. I do not think it is limited to QNX. > the clean target, since dep_dirs is empty. And dep_dirs being empty under some circumstance shouldn't be limited to QNX, either. I think your change does no harm, may be a good change if dep_dirs goes empty, but the justification is lacking. What caused your dep_dirs to become empty in the first place? I am scratching my head because I see OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ $(XDIFF_OBJS) \ $(VCSSVN_OBJS) \ git.o dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS > Avoid this by merging two rm > command lines. > > Signed-off-by: Matt Kraai > --- > Makefile | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 5a2e02d..c2e3666 100644 > --- a/Makefile > +++ b/Makefile > @@ -2414,8 +2414,7 @@ clean: profile-clean > builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) > $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X > $(RM) $(TEST_PROGRAMS) > - $(RM) -r bin-wrappers > - $(RM) -r $(dep_dirs) > + $(RM) -r bin-wrappers $(dep_dirs) > $(RM) -r po/build/ > $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) > tags cscope* > $(RM) -r $(GIT_TARNAME) .doc-tmp-dir -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Makefile: don't run rm without any files
From: Matt Kraai "rm -f -r" fails on QNX when not passed any files to remove. This breaks the clean target, since dep_dirs is empty. Avoid this by merging two rm command lines. Signed-off-by: Matt Kraai --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5a2e02d..c2e3666 100644 --- a/Makefile +++ b/Makefile @@ -2414,8 +2414,7 @@ clean: profile-clean builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) - $(RM) -r bin-wrappers - $(RM) -r $(dep_dirs) + $(RM) -r bin-wrappers $(dep_dirs) $(RM) -r po/build/ $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope* $(RM) -r $(GIT_TARNAME) .doc-tmp-dir -- 1.8.1.3.570.g3074c9d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html