{"thread":{"id":"22426","subject":"[PATCH] rerere: fix too-short initialization","startedAt":"2010-01-28T14:52:16Z","lastAt":"2010-01-31T21:08:43Z","messageCount":13,"participants":["Jeff King","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"132892","messageId":"20100128145216.GA29727@coredump.intra.peff.net","threadId":"22426","inReplyTo":null,"subject":"[PATCH] rerere: fix too-short initialization","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-28T14:52:16Z","receivedAt":"2010-01-28T14:52:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This was caused by a typo in the sizeof parameter, and meant\nwe looked at uninitialized memory. Caught by valgrind in\nt2030.\n\nSigned-off-by: Jeff King \n---\nI'm running the whole test suite under valgrind for the current\n'master'. This was the first hit, but it's very s-l-o-w, so there might\nbe more as the day progresses.\n\n rerere.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex a86d73d..d1d3e75 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -325,7 +325,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfile[i].ptr);\n \n-\tmemset(&io, 0, sizeof(&io));\n+\tmemset(&io, 0, sizeof(io));\n \tio.io.getline = rerere_mem_getline;\n \tif (output)\n \t\tio.io.output = fopen(output, \"w\");\n-- \n1.7.0.rc0.41.g538720\n"},{"id":"132945","messageId":"20100129102518.GA5875@coredump.intra.peff.net","threadId":"22426","inReplyTo":"20100128145216.GA29727@coredump.intra.peff.net","subject":"[PATCH 0/3] valgrind bug roundup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-29T10:25:18Z","receivedAt":"2010-01-29T10:25:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 28, 2010 at 09:52:16AM -0500, Jeff King wrote:\n\n> I'm running the whole test suite under valgrind for the current\n> 'master'. This was the first hit, but it's very s-l-o-w, so there might\n> be more as the day progresses.\n\nAnd here they are. The first two are actual bugs, the third silences a\nfalse positive:\n\n [1/3]: fix memcpy of overlapping area\n [2/3]: fix off-by-one allocation error\n [3/3]: add shebang line to git-mergetool--lib.sh\n\nIt really has been a while since I've run the whole test suite under\nvalgrind. The breakage in 3/3 dates back to last April. The bug in 1/3\nis also quite old (v1.5.0). I'm not sure why I didn't find it in\nprevious valgrind runs; perhaps it is only recent valgrinds that have\ngrown support for finding overlapping memcpy. The bug in 2/3 is new, and\nhas never been in a released version.\n\n-Peff\n"},{"id":"132947","messageId":"20100129102844.GA6025@coredump.intra.peff.net","threadId":"22426","inReplyTo":"20100129102518.GA5875@coredump.intra.peff.net","subject":"[PATCH 1/3] fix memcpy of overlapping area","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-29T10:28:44Z","receivedAt":"2010-01-29T10:28:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Caught by valgrind in t5500, but it is pretty obvious from\nreading the code that this is shifting elements of an array\nto the left, which needs memmove.\n\nSigned-off-by: Jeff King \n---\nThis was introduced in f53514b (allow deepening of a shallow repository,\n2006-10-30), released as part of v1.5.0. So probably 'maint'-worthy.\n\n commit.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 632061c..731191e 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -224,7 +224,7 @@ int unregister_shallow(const unsigned char *sha1)\n \tif (pos < 0)\n \t\treturn -1;\n \tif (pos + 1 < commit_graft_nr)\n-\t\tmemcpy(commit_graft + pos, commit_graft + pos + 1,\n+\t\tmemmove(commit_graft + pos, commit_graft + pos + 1,\n \t\t\t\tsizeof(struct commit_graft *)\n \t\t\t\t* (commit_graft_nr - pos - 1));\n \tcommit_graft_nr--;\n-- \n1.7.0.rc0.41.g538720\n"},{"id":"132948","messageId":"20100129103130.GB6025@coredump.intra.peff.net","threadId":"22426","inReplyTo":"20100129102518.GA5875@coredump.intra.peff.net","subject":"[PATCH 2/3] fix off-by-one allocation error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-29T10:31:30Z","receivedAt":"2010-01-29T10:31:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Caught by valgrind in t5516. Reading the code shows we\nmalloc enough for our string, but not trailing NUL.\n\nSigned-off-by: Jeff King \n---\nThis bug was introduced in f517f1f (builtin-push: add --delete as\nsyntactic sugar for :foo, 2009-12-30), not released yet but part of\n1.7.0-rc0. So no need for a 'maint' fix.\n\nAn obvious alternative would be to convert it to strbuf (which could\nalso be used to clean up other non-buggy string generation earlier in\nthe function).\n\n builtin-push.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 5df6608..5633f0a 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -52,7 +52,7 @@ static void set_refspecs(const char **refs, int nr)\n \t\t} else if (deleterefs && !strchr(ref, ':')) {\n \t\t\tchar *delref;\n \t\t\tint len = strlen(ref)+1;\n-\t\t\tdelref = xmalloc(len);\n+\t\t\tdelref = xmalloc(len+1);\n \t\t\tstrcpy(delref, \":\");\n \t\t\tstrcat(delref, ref);\n \t\t\tref = delref;\n-- \n1.7.0.rc0.41.g538720\n"},{"id":"132949","messageId":"20100129103723.GC6025@coredump.intra.peff.net","threadId":"22426","inReplyTo":"20100129102518.GA5875@coredump.intra.peff.net","subject":"[PATCH 3/3] add shebang line to git-mergetool--lib.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-29T10:37:23Z","receivedAt":"2010-01-29T10:37:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Even though this script is expected to be sourced instead of\nexecuted on its own, the #!/bin/sh line provides simple\ndocumentation about what format the file is in.\n\nIn particular, the lack of such a line was confusing the\nvalgrind support of our test scripts, which assumed that any\nexecutable without a #!-line should be intercepted and run\nthrough valgrind. So during valgrind-enabled tests, any\nscript sourcing this file actually sourced the valgrind\ninterception script instead.\n\nSigned-off-by: Jeff King \n---\nThe valgrind script could perhaps be a bit smarter instead, but checking\n#!-lines is nice and simple, and this change makes other programs like\n\"file\" happier, too.\n\nThis problem has been around since 21d0ba7 (difftool/mergetool: refactor\ncommands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But\nsince it is only about our internal tests, and even then only about\nrunning them with valgrind enabled, I don't know if it is worth a fix on\n'maint'.\n\n git-mergetool--lib.sh | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 5b62785..51dd0d6 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -1,3 +1,4 @@\n+#!/bin/sh\n # git-mergetool--lib is a library for common merge tool functions\n diff_mode() {\n \ttest \"$TOOL_MODE\" = diff\n-- \n1.7.0.rc0.41.g538720\n"},{"id":"132959","messageId":"20100129145025.GA22703@progeny.tock","threadId":"22426","inReplyTo":"20100129103723.GC6025@coredump.intra.peff.net","subject":"[PATCH] Do not install shell libraries executable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-01-29T14:50:25Z","receivedAt":"2010-01-29T14:50:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"These scripts are expected to be sourced instead of executed on\ntheir own. Avoid some confusion by not marking them executable.\n\nThe executable bit was confusing the valgrind support of our test\nscripts, which assumed that any executable without a #!-line\nshould be intercepted and run through valgrind. So during\nvalgrind-enabled tests, any script sourcing this file actually\nsourced the valgrind interception script instead.\n\nReported-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nJeff King wrote:\n\n> This problem has been around since 21d0ba7 (difftool/mergetool: refactor\n> commands to use git-mergetool--lib, 2009-04-08), released in v1.6.3. But\n> since it is only about our internal tests, and even then only about\n> running them with valgrind enabled, I don't know if it is worth a fix on\n> 'maint'.\n\nIt was also confusing dpkg-shlibdeps, so I recently came up with\nthis fix. Both fixes seem like good changes to me, and both\ncould be applied. Your fix has the virtue of being shorter,\nhence safer.\n\n Makefile | 24 ++++++++++++++++++------\n 1 files changed, 18 insertions(+), 6 deletions(-)\n mode change 100755 => 100644 git-sh-setup.sh\n\ndiff --git a/Makefile b/Makefile\nindex fd7f51e..70dcc40 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -342,6 +342,7 @@ LIB_OBJS =\n PROGRAMS =\n SCRIPT_PERL =\n SCRIPT_SH =\n+SCRIPT_LIB_SH =\n TEST_PROGRAMS =\n \n SCRIPT_SH += git-am.sh\n@@ -353,7 +354,6 @@ SCRIPT_SH += git-merge-octopus.sh\n SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n-SCRIPT_SH += git-mergetool--lib.sh\n SCRIPT_SH += git-notes.sh\n SCRIPT_SH += git-parse-remote.sh\n SCRIPT_SH += git-pull.sh\n@@ -362,11 +362,13 @@ SCRIPT_SH += git-rebase--interactive.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n-SCRIPT_SH += git-sh-setup.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n SCRIPT_SH += git-web--browse.sh\n \n+SCRIPT_LIB_SH += git-mergetool--lib.sh\n+SCRIPT_LIB_SH += git-sh-setup.sh\n+\n SCRIPT_PERL += git-add--interactive.perl\n SCRIPT_PERL += git-difftool.perl\n SCRIPT_PERL += git-archimport.perl\n@@ -1428,7 +1430,7 @@ export TAR INSTALL DESTDIR SHELL_PATH\n \n SHELL = $(SHELL_PATH)\n \n-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n@@ -1488,6 +1490,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n+$(patsubst %.sh,%,$(SCRIPT_LIB_SH)) : % : %.sh\n+\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n+\tsed -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n+\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n+\t -e $(BROKEN_PATH_FIX) \\\n+\t $@.sh >$@+ && \\\n+\tmv $@+ $@\n+\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n@@ -1792,6 +1803,7 @@ install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n ifndef NO_PERL\n@@ -1901,7 +1913,7 @@ distclean: clean\n clean:\n \t$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \\\n \t\t$(LIB_FILE) $(XDIFF_LIB)\n-\t$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X\n+\t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*\n \t$(RM) -r autom4te.cache\n@@ -1930,7 +1942,7 @@ endif\n ### Check documentation\n #\n check-docs::\n-\t@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \\\n+\t@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \\\n \tdo \\\n \t\tcase \"$$v\" in \\\n \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n@@ -1975,7 +1987,7 @@ check-docs::\n \t\tdocumented,gittutorial-2 | \\\n \t\tsentinel,not,matching,is,ok ) continue ;; \\\n \t\tesac; \\\n-\t\tcase \" $(ALL_PROGRAMS) $(BUILT_INS) git gitk \" in \\\n+\t\tcase \" $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk \" in \\\n \t\t*\" $$cmd \"*)\t;; \\\n \t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n \t\tesac; \\\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nold mode 100755\nnew mode 100644\n-- \n1.7.0.rc0\n"},{"id":"133168","messageId":"7vhbq2g3a9.fsf@alter.siamese.dyndns.org","threadId":"22426","inReplyTo":"20100129145025.GA22703@progeny.tock","subject":"Re: [PATCH] Do not install shell libraries executable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-31T07:08:14Z","receivedAt":"2010-01-31T07:08:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> @@ -1488,6 +1490,15 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n> \tchmod +x $@+ && \\\n> \tmv $@+ $@\n> \n> +$(patsubst %.sh,%,$(SCRIPT_LIB_SH)) : % : %.sh\n> +\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n> +\tsed -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n> +\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n> +\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n> +\t -e $(BROKEN_PATH_FIX) \\\n> +\t $@.sh >$@+ && \\\n> +\tmv $@+ $@\n> +\n\nCan't you do something about this repetition from existing $(SCRIPT_SH)\nmunging? Other than that the patch looks like Ok.\n"},{"id":"133175","messageId":"20100131083459.GA18561@progeny.tock","threadId":"22426","inReplyTo":"7vhbq2g3a9.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Do not install shell libraries executable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-01-31T08:34:59Z","receivedAt":"2010-01-31T08:34:59Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Some scripts are expected to be sourced instead of executed on\ntheir own. Avoid some confusion by not marking them executable.\n\nThe executable bit was confusing the valgrind support of our test\nscripts, which assumed that any executable without a #!-line\nshould be intercepted and run through valgrind. So during\nvalgrind-enabled tests, any script sourcing these files actually\nsourced the valgrind interception script instead.\n\nReported-by: Jeff King \nSigned-off-by: Jonathan Nieder \n---\nJunio C Hamano wrote:\n\n> Can't you do something about this repetition from existing $(SCRIPT_SH)\n> munging?\n\nSorry about that.\n\nLast time I also missed git-parse-remote, so while at it, I am taking\nthe opportunity to add that to SCRIPT_LIB_SH, too.\n\nGood night,\nJonathan\n\n Makefile | 39 +++++++++++++++++++++++++--------------\n 1 files changed, 25 insertions(+), 14 deletions(-)\n mode change 100755 => 100644 git-parse-remote.sh\n mode change 100755 => 100644 git-sh-setup.sh\n\ndiff --git a/Makefile b/Makefile\nindex fd7f51e..ce42d93 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -342,6 +342,7 @@ LIB_OBJS =\n PROGRAMS =\n SCRIPT_PERL =\n SCRIPT_SH =\n+SCRIPT_LIB_SH =\n TEST_PROGRAMS =\n \n SCRIPT_SH += git-am.sh\n@@ -353,20 +354,21 @@ SCRIPT_SH += git-merge-octopus.sh\n SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n-SCRIPT_SH += git-mergetool--lib.sh\n SCRIPT_SH += git-notes.sh\n-SCRIPT_SH += git-parse-remote.sh\n SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase--interactive.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n-SCRIPT_SH += git-sh-setup.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n SCRIPT_SH += git-web--browse.sh\n \n+SCRIPT_LIB_SH += git-mergetool--lib.sh\n+SCRIPT_LIB_SH += git-parse-remote.sh\n+SCRIPT_LIB_SH += git-sh-setup.sh\n+\n SCRIPT_PERL += git-add--interactive.perl\n SCRIPT_PERL += git-difftool.perl\n SCRIPT_PERL += git-archimport.perl\n@@ -1428,7 +1430,7 @@ export TAR INSTALL DESTDIR SHELL_PATH\n \n SHELL = $(SHELL_PATH)\n \n-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n@@ -1477,17 +1479,25 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt\n common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n+define cmd_munge_script\n+$(RM) $@ $@+ && \\\n+sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+ -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n+ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n+ -e $(BROKEN_PATH_FIX) \\\n+ $@.sh >$@+\n+endef\n+\n $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n-\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n-\t -e $(BROKEN_PATH_FIX) \\\n-\t $@.sh >$@+ && \\\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n+$(patsubst %.sh,%,$(SCRIPT_LIB_SH)) : % : %.sh\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n+\tmv $@+ $@\n+\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n@@ -1792,6 +1802,7 @@ install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X git-shell$X git-cvsserver '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n ifndef NO_PERL\n@@ -1901,7 +1912,7 @@ distclean: clean\n clean:\n \t$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \\\n \t\t$(LIB_FILE) $(XDIFF_LIB)\n-\t$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X\n+\t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*\n \t$(RM) -r autom4te.cache\n@@ -1930,7 +1941,7 @@ endif\n ### Check documentation\n #\n check-docs::\n-\t@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \\\n+\t@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \\\n \tdo \\\n \t\tcase \"$$v\" in \\\n \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n@@ -1975,7 +1986,7 @@ check-docs::\n \t\tdocumented,gittutorial-2 | \\\n \t\tsentinel,not,matching,is,ok ) continue ;; \\\n \t\tesac; \\\n-\t\tcase \" $(ALL_PROGRAMS) $(BUILT_INS) git gitk \" in \\\n+\t\tcase \" $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk \" in \\\n \t\t*\" $$cmd \"*)\t;; \\\n \t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n \t\tesac; \\\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nold mode 100755\nnew mode 100644\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nold mode 100755\nnew mode 100644\n-- \n1.7.0.rc1\n"},{"id":"133194","messageId":"7vy6jecb0y.fsf@alter.siamese.dyndns.org","threadId":"22426","inReplyTo":"20100131083459.GA18561@progeny.tock","subject":"Re: [PATCH v2] Do not install shell libraries executable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-31T19:46:53Z","receivedAt":"2010-01-31T19:46:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Last time I also missed git-parse-remote, so while at it, I am taking\n> the opportunity to add that to SCRIPT_LIB_SH, too.\n\nYour patch says it was generated by 1.7.0-rc1, but the change itself\nseems to be based on an older version. Curious.\n\nHow much would it hurt the distro packagers, if we don't take this patch\nbefore 1.7.0? If this would help a lot, let's give it a bit higher\npriority and make sure 1.7.0 ships with (a corrected version of) it;\notherwise I'd say we should not merge this before 1.7.0.\n\n> +SCRIPT_LIB_SH += git-mergetool--lib.sh\n> +SCRIPT_LIB_SH += git-parse-remote.sh\n> +SCRIPT_LIB_SH += git-sh-setup.sh\n> + ...\n> @@ -1792,6 +1802,7 @@ install: all\n> \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n> \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> +\t$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n\nI can understand that you didn't want to include the \"included scriptlets\"\nas part of ALL_PROGRAMS because $(INSTALL) may flip 'x' bit on.\n\nBut you should then be installing %(patsubst %.sh,%,$(SCRIPT_LIB_SH));\notherwise, you are installing git-sh-setup.sh and\n\n . git-sh-setup\n\nwould not work. Have you ever tested this?\n\n> @@ -1901,7 +1912,7 @@ distclean: clean\n> clean:\n> \t$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \\\n> \t\t$(LIB_FILE) $(XDIFF_LIB)\n> -\t$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X\n> +\t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git$X\n\nAnd this is even worse. You are removing the _source_ here. I can see\nyou didn't even test the very basic: \"make clean && make\".\n\n> @@ -1930,7 +1941,7 @@ endif\n> ### Check documentation\n> #\n> check-docs::\n> -\t@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \\\n> +\t@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk; \\\n\nLikewise.\n\n> \tdo \\\n> \t\tcase \"$$v\" in \\\n> \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n> @@ -1975,7 +1986,7 @@ check-docs::\n> \t\tdocumented,gittutorial-2 | \\\n> \t\tsentinel,not,matching,is,ok ) continue ;; \\\n> \t\tesac; \\\n> -\t\tcase \" $(ALL_PROGRAMS) $(BUILT_INS) git gitk \" in \\\n> +\t\tcase \" $(ALL_PROGRAMS) $(SCRIPT_LIB_SH) $(BUILT_INS) git gitk \" in \\\n\nLikewise.\n\nWouldn't it make a bit more sense to do it like this instead? I at least\ndid \"make clean && make\" ;-)\n\n-- >8 --\nFrom: Jonathan Nieder \nSubject: [PATCH] Do not install shell libraries executable\n\nSome scripts are expected to be sourced instead of executed on their own.\nAvoid some confusion by not marking them executable.\n\nThe executable bit was confusing the valgrind support of our test scripts,\nwhich assumed that any executable without a #!-line should be intercepted\nand run through valgrind. So during valgrind-enabled tests, any script\nsourcing these files actually sourced the valgrind interception script\ninstead.\n\nReported-by: Jeff King \nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\n Makefile | 42 ++++++++++++++++++++++++++++--------------\n 1 files changed, 28 insertions(+), 14 deletions(-)\n mode change 100755 => 100644 git-parse-remote.sh\n mode change 100755 => 100644 git-sh-setup.sh\n\ndiff --git a/Makefile b/Makefile\nindex af08c8f..6bbeb24 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -341,6 +341,7 @@ PROGRAMS =\n SCRIPT_PERL =\n SCRIPT_PYTHON =\n SCRIPT_SH =\n+SCRIPT_LIB =\n TEST_PROGRAMS =\n \n SCRIPT_SH += git-am.sh\n@@ -352,20 +353,21 @@ SCRIPT_SH += git-merge-octopus.sh\n SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n-SCRIPT_SH += git-mergetool--lib.sh\n SCRIPT_SH += git-notes.sh\n-SCRIPT_SH += git-parse-remote.sh\n SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\n SCRIPT_SH += git-rebase--interactive.sh\n SCRIPT_SH += git-rebase.sh\n SCRIPT_SH += git-repack.sh\n SCRIPT_SH += git-request-pull.sh\n-SCRIPT_SH += git-sh-setup.sh\n SCRIPT_SH += git-stash.sh\n SCRIPT_SH += git-submodule.sh\n SCRIPT_SH += git-web--browse.sh\n \n+SCRIPT_LIB += git-mergetool--lib\n+SCRIPT_LIB += git-parse-remote\n+SCRIPT_LIB += git-sh-setup\n+\n SCRIPT_PERL += git-add--interactive.perl\n SCRIPT_PERL += git-difftool.perl\n SCRIPT_PERL += git-archimport.perl\n@@ -1454,7 +1456,7 @@ export TAR INSTALL DESTDIR SHELL_PATH\n \n SHELL = $(SHELL_PATH)\n \n-all:: shell_compatibility_test $(ALL_PROGRAMS) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: shell_compatibility_test $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n@@ -1505,17 +1507,25 @@ common-cmds.h: ./generate-cmdlist.sh command-list.txt\n common-cmds.h: $(wildcard Documentation/git-*.txt)\n \t$(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@\n \n+define cmd_munge_script\n+$(RM) $@ $@+ && \\\n+sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n+ -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n+ -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n+ -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n+ -e $(BROKEN_PATH_FIX) \\\n+ $@.sh >$@+\n+endef\n+\n $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh\n-\t$(QUIET_GEN)$(RM) $@ $@+ && \\\n-\tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n-\t -e 's|@SHELL_PATH@|$(SHELL_PATH_SQ)|' \\\n-\t -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \\\n-\t -e 's/@@NO_CURL@@/$(NO_CURL)/g' \\\n-\t -e $(BROKEN_PATH_FIX) \\\n-\t $@.sh >$@+ && \\\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n+$(SCRIPT_LIB) : % : %.sh\n+\t$(QUIET_GEN)$(cmd_munge_script) && \\\n+\tmv $@+ $@\n+\n ifndef NO_PERL\n $(patsubst %.perl,%,$(SCRIPT_PERL)): perl/perl.mak\n \n@@ -1866,6 +1876,7 @@ install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n ifndef NO_PERL\n@@ -1985,7 +1996,7 @@ distclean: clean\n clean:\n \t$(RM) *.o block-sha1/*.o ppc/*.o compat/*.o compat/*/*.o xdiff/*.o \\\n \t\t$(LIB_FILE) $(XDIFF_LIB)\n-\t$(RM) $(ALL_PROGRAMS) $(BUILT_INS) git$X\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) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h TAGS tags cscope*\n@@ -2017,7 +2028,7 @@ endif\n ### Check documentation\n #\n check-docs::\n-\t@(for v in $(ALL_PROGRAMS) $(BUILT_INS) git gitk; \\\n+\t@(for v in $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git gitk; \\\n \tdo \\\n \t\tcase \"$$v\" in \\\n \t\tgit-merge-octopus | git-merge-ours | git-merge-recursive | \\\n@@ -2060,9 +2071,12 @@ check-docs::\n \t\tdocumented,gitrepository-layout | \\\n \t\tdocumented,gittutorial | \\\n \t\tdocumented,gittutorial-2 | \\\n+\t\tdocumented,git-bisect-lk2009 | \\\n+\t\tdocumented.git-remote-helpers | \\\n+\t\tdocumented,gitworkflows | \\\n \t\tsentinel,not,matching,is,ok ) continue ;; \\\n \t\tesac; \\\n-\t\tcase \" $(ALL_PROGRAMS) $(BUILT_INS) git gitk \" in \\\n+\t\tcase \" $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git gitk \" in \\\n \t\t*\" $$cmd \"*)\t;; \\\n \t\t*) echo \"removed but $$how: $$cmd\" ;; \\\n \t\tesac; \\\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nold mode 100755\nnew mode 100644\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nold mode 100755\nnew mode 100644\n-- \n1.7.0.rc1.141.gd3fd2\n"},{"id":"133196","messageId":"20100131200028.GA4045@progeny.tock","threadId":"22426","inReplyTo":"7vy6jecb0y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Do not install shell libraries executable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-01-31T20:00:28Z","receivedAt":"2010-01-31T20:00:28Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> How much would it hurt the distro packagers, if we don't take this patch\n> before 1.7.0? If this would help a lot, let's give it a bit higher\n> priority and make sure 1.7.0 ships with (a corrected version of) it;\n> otherwise I'd say we should not merge this before 1.7.0.\n\nGiven that Peff’s fix is in, I don’t think it is needed at all. So I\nwould say, better to let it wait.\n\n> > +SCRIPT_LIB_SH += git-mergetool--lib.sh\n> > +SCRIPT_LIB_SH += git-parse-remote.sh\n> > +SCRIPT_LIB_SH += git-sh-setup.sh\n> > + ...\n> > @@ -1792,6 +1802,7 @@ install: all\n> > \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'\n> > \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> > \t$(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> > +\t$(INSTALL) -m 644 $(SCRIPT_LIB_SH) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n> \n> I can understand that you didn't want to include the \"included scriptlets\"\n> as part of ALL_PROGRAMS because $(INSTALL) may flip 'x' bit on.\n> \n> But you should then be installing %(patsubst %.sh,%,$(SCRIPT_LIB_SH));\n> otherwise, you are installing git-sh-setup.sh and\n> \n> . git-sh-setup\n> \n> would not work. Have you ever tested this?\n\nGood catches, sorry. :( I did test but clearly I was not thinking very\nwell when I looked at the result...\n\n> Wouldn't it make a bit more sense to do it like this instead?\n\nYes, that looks right. Thanks for cleaning up my mess!\n\nSorry for the trouble,\nJonathan\n"},{"id":"133197","messageId":"7vockaca5w.fsf@alter.siamese.dyndns.org","threadId":"22426","inReplyTo":"20100131200028.GA4045@progeny.tock","subject":"Re: [PATCH v2] Do not install shell libraries executable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-31T20:05:31Z","receivedAt":"2010-01-31T20:05:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> How much would it hurt the distro packagers, if we don't take this patch\n>> before 1.7.0? If this would help a lot, let's give it a bit higher\n>> priority and make sure 1.7.0 ships with (a corrected version of) it;\n>> otherwise I'd say we should not merge this before 1.7.0.\n>\n> Given that Peff’s fix is in, I don’t think it is needed at all. So I\n> would say, better to let it wait.\n\nI was referring to this from your original:\n\n It was also confusing dpkg-shlibdeps, so I recently came up with\n this fix. Both fixes seem like good changes to me, and both\n could be applied. Your fix has the virtue of being shorter,\n hence safer.\n\nIs Jeff's mergetool-lib change enough to address this issue as well?\n"},{"id":"133199","messageId":"20100131210052.GA4435@progeny.tock","threadId":"22426","inReplyTo":"7vockaca5w.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Do not install shell libraries executable","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-01-31T21:00:52Z","receivedAt":"2010-01-31T21:00:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n>\n>> Junio C Hamano wrote:\n>>\n>>> How much would it hurt the distro packagers, if we don't take this patch\n>>> before 1.7.0? If this would help a lot, let's give it a bit higher\n>>> priority and make sure 1.7.0 ships with (a corrected version of) it;\n>>> otherwise I'd say we should not merge this before 1.7.0.\n>>\n>> Given that Peff’s fix is in, I don’t think it is needed at all. So I\n>> would say, better to let it wait.\n>\n> I was referring to this from your original:\n> \n> It was also confusing dpkg-shlibdeps, so I recently came up with\n> this fix. Both fixes seem like good changes to me, and both\n> could be applied. Your fix has the virtue of being shorter,\n> hence safer.\n> \n> Is Jeff's mergetool-lib change enough to address this issue as well?\n\nI just checked; looks like I was confusing a few issues.\n\n - dpkg-shlibdeps does not like to be fed scripts, period. That\n has nothing to do with this.\n\n - debian/rules in the git-core package feeds every file in\n /usr/bin and gitexecdir that doesn’t start with #! to 'strip'.\n Jeff's change helps that; my fix has nothing to do with it.\n\n - some other tool must have been happier with these files not\n being executable, but I cannot reproduce this or find it now.\n\nI wrote that patch late at night, and unfortunately, I cannot justify it\nto myself now. With Jeff’s change applied, there is no obvious breakage\nthat it fixes. If a problem comes up again, I will let you know.\n\nEmbarrassed,\nJonathan\n"},{"id":"133200","messageId":"7veil6c78k.fsf@alter.siamese.dyndns.org","threadId":"22426","inReplyTo":"20100131210052.GA4435@progeny.tock","subject":"Re: [PATCH v2] Do not install shell libraries executable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-31T21:08:43Z","receivedAt":"2010-01-31T21:08:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> I wrote that patch late at night, and unfortunately, I cannot justify it\n> to myself now. With Jeff’s change applied, there is no obvious breakage\n> that it fixes. If a problem comes up again, I will let you know.\n\nThanks for a thorough write-up. Really appreciated.\n"}]}