{"thread":{"id":"32899","subject":"[PATCH] Makefile: don't run rm without any files","startedAt":"2013-02-13T15:57:48Z","lastAt":"2013-02-13T20:12:44Z","messageCount":5,"participants":["Matt Kraai","Junio C Hamano","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"209475","messageId":"1360771068-505-1-git-send-email-kraai@ftbfs.org","threadId":"32899","inReplyTo":null,"subject":"[PATCH] Makefile: don't run rm without any files","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-02-13T15:57:48Z","receivedAt":"2013-02-13T15:57:48Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"From: Matt Kraai \n\n\"rm -f -r\" fails on QNX when not passed any files to remove. This breaks\nthe clean target, since dep_dirs is empty. Avoid this by merging two rm\ncommand lines.\n\nSigned-off-by: Matt Kraai \n---\n Makefile | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5a2e02d..c2e3666 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2414,8 +2414,7 @@ clean: profile-clean\n \t\tbuiltin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n-\t$(RM) -r bin-wrappers\n-\t$(RM) -r $(dep_dirs)\n+\t$(RM) -r bin-wrappers $(dep_dirs)\n \t$(RM) -r po/build/\n \t$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope*\n \t$(RM) -r $(GIT_TARNAME) .doc-tmp-dir\n-- \n1.8.1.3.570.g3074c9d\n"},{"id":"209480","messageId":"7vtxpg9mxq.fsf@alter.siamese.dyndns.org","threadId":"32899","inReplyTo":"1360771068-505-1-git-send-email-kraai@ftbfs.org","subject":"Re: [PATCH] Makefile: don't run rm without any files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-13T16:51:45Z","receivedAt":"2013-02-13T16:51:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt Kraai writes:\n\n> From: Matt Kraai \n>\n> \"rm -f -r\" fails on QNX when not passed any files to remove.\n\nI do not think it is limited to QNX.\n\n> the clean target, since dep_dirs is empty.\n\nAnd dep_dirs being empty under some circumstance shouldn't be\nlimited to QNX, either.\n\nI think your change does no harm, may be a good change if dep_dirs\ngoes empty, but the justification is lacking. What caused your\ndep_dirs to become empty in the first place?\n\nI am scratching my head because I see\n\n OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n\t$(XDIFF_OBJS) \\\n\t$(VCSSVN_OBJS) \\\n\tgit.o\n dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n\n\n\n> Avoid this by merging two rm\n> command lines.\n>\n> Signed-off-by: Matt Kraai \n> ---\n> Makefile | 3 +--\n> 1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 5a2e02d..c2e3666 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2414,8 +2414,7 @@ clean: profile-clean\n> \t\tbuiltin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)\n> \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n> \t$(RM) $(TEST_PROGRAMS)\n> -\t$(RM) -r bin-wrappers\n> -\t$(RM) -r $(dep_dirs)\n> +\t$(RM) -r bin-wrappers $(dep_dirs)\n> \t$(RM) -r po/build/\n> \t$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) tags cscope*\n> \t$(RM) -r $(GIT_TARNAME) .doc-tmp-dir\n"},{"id":"209482","messageId":"20130213170028.GA410@ftbfs.org","threadId":"32899","inReplyTo":"7vtxpg9mxq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: don't run rm without any files","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2013-02-13T17:00:28Z","receivedAt":"2013-02-13T17:00:28Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Wed, Feb 13, 2013 at 08:51:45AM -0800, Junio C Hamano wrote:\n> Matt Kraai writes:\n> \n> > From: Matt Kraai \n> >\n> > \"rm -f -r\" fails on QNX when not passed any files to remove.\n> \n> I do not think it is limited to QNX.\n> \n> > the clean target, since dep_dirs is empty.\n> \n> And dep_dirs being empty under some circumstance shouldn't be\n> limited to QNX, either.\n> \n> I think your change does no harm, may be a good change if dep_dirs\n> goes empty, but the justification is lacking. What caused your\n> dep_dirs to become empty in the first place?\n> \n> I am scratching my head because I see\n> \n> OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n> \t$(XDIFF_OBJS) \\\n> \t$(VCSSVN_OBJS) \\\n> \tgit.o\n> dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n\nI don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to \"auto\".\nThe automatic detection determines that the compiler doesn't support\nit, so it's then set to \"no\". CHECK_HEADER_DEPENDENCIES isn't set\neither, so about 20 lines below the dep_dirs assignment you quoted,\ndep_dirs is cleared:\n\n ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)\n ifndef CHECK_HEADER_DEPENDENCIES\n dep_dirs =\n ...\n\nShould I submit an updated patch with a different commit message?\n"},{"id":"209491","messageId":"7vehgk6l11.fsf@alter.siamese.dyndns.org","threadId":"32899","inReplyTo":"20130213170028.GA410@ftbfs.org","subject":"Re: [PATCH] Makefile: don't run rm without any files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-13T20:01:14Z","receivedAt":"2013-02-13T20:01:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matt Kraai writes:\n\n> I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to \"auto\".\n> The automatic detection determines that the compiler doesn't support\n> it, so it's then set to \"no\". CHECK_HEADER_DEPENDENCIES isn't set\n> either, so about 20 lines below the dep_dirs assignment you quoted,\n> dep_dirs is cleared:\n>\n> ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)\n> ifndef CHECK_HEADER_DEPENDENCIES\n> dep_dirs =\n> ...\n>\n> Should I submit an updated patch with a different commit message?\n\nI amended the log message like so:\n\ncommit bd9df384b16077337fffe9836c9255976b0e7b91\nAuthor: Matt Kraai \nDate: Wed Feb 13 07:57:48 2013 -0800\n\n Makefile: don't run rm without any files\n \n When COMPUTE_HEADER_DEPENDENCIES is set to \"auto\" and the compiler\n does not support it, $(dep_dirs) becomes empty. \"make clean\" runs\n \"rm -rf $(dep_dirs)\", which fails in such a case.\n \n Signed-off-by: Matt Kraai \n Signed-off-by: Junio C Hamano \n"},{"id":"209492","messageId":"20130213201244.GD3381@google.com","threadId":"32899","inReplyTo":"7vehgk6l11.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Makefile: don't run rm without any files","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-02-13T20:12:44Z","receivedAt":"2013-02-13T20:12:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I amended the log message like so:\n>\n> commit bd9df384b16077337fffe9836c9255976b0e7b91\n> Author: Matt Kraai \n> Date: Wed Feb 13 07:57:48 2013 -0800\n>\n> Makefile: don't run rm without any files\n>\n> When COMPUTE_HEADER_DEPENDENCIES is set to \"auto\" and the compiler\n> does not support it, $(dep_dirs) becomes empty. \"make clean\" runs\n> \"rm -rf $(dep_dirs)\", which fails in such a case.\n\nTo pedantic, that only fails on some platforms. The autoconf manual\nexplains:\n\n\tIt is not portable to invoke rm without options or operands. On the\n\tother hand, Posix now requires rm -f to silently succeed when there are\n\tno operands (useful for constructs like rm -rf $filelist without first\n\tchecking if ‘$filelist’ was empty). But this was not always portable; at\n\tleast NetBSD rm built before 2008 would fail with a diagnostic.\n\nAnyway, looks like a good fix. Thanks.\n"}]}