{"thread":{"id":"43841","subject":"[PATCH] make rebase respect core.hooksPath if set","startedAt":"2016-08-14T15:58:39Z","lastAt":"2016-08-16T02:02:19Z","messageCount":6,"participants":["ryenus","Mike Rappazzo","Johannes Schindelin","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"299279","messageId":"CAKkAvaxWk6SK4EYPaWXHQoVBh9qLgHoEqAh9+dgOrjncsi5QyA@mail.gmail.com","threadId":"43841","inReplyTo":null,"subject":"[PATCH] make rebase respect core.hooksPath if set","fromName":"ryenus","fromEmail":"ryenus@gmail.com","sentAt":"2016-08-14T15:58:13Z","receivedAt":"2016-08-14T15:58:39Z","isPatch":true,"sender":{"key":"ryenus@gmail.com","avatar":"https://avatars.githubusercontent.com/u/610161?v=4"},"body":"when looking for pre-rebase and post-rewrite hooks, git-rebase\nonly looks for hooks dir using `git rev-parse --git-path hooks`,\nwhich didn't consider the path overridden by core.hooksPath.\n\nSigned-off-by: ryenus \n---\n git-rebase--interactive.sh | 14 +++++++++-----\n git-rebase--merge.sh | 4 +++-\n git-rebase.sh | 7 +++++--\n 3 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex e2da524..e8af70d 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -724,11 +724,15 @@ Commit or stash your changes, and then run\n git notes copy --for-rewrite=rebase < \"$rewritten_list\" ||\n true # we don't care if this copying failed\n } &&\n- hook=\"$(git rev-parse --git-path hooks/post-rewrite)\"\n- if test -x \"$hook\" && test -s \"$rewritten_list\"; then\n- \"$hook\" rebase < \"$rewritten_list\"\n- true # we don't care if this hook failed\n- fi &&\n+ {\n+ hooks_path=$(git config --get core.hooksPath)\n+ hooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+ hook=\"${hooks_path}/post-rewrite\"\n+ if test -x \"$hook\" && test -s \"$rewritten_list\"; then\n+ \"$hook\" rebase < \"$rewritten_list\"\n+ true # we don't care if this hook failed\n+ fi\n+ } &&\n warn \"$(eval_gettext \"Successfully rebased and updated \\$head_name.\")\"\n\n return 1 # not failure; just to break the do_rest loop\ndiff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\nindex 06a4723..df5073e 100644\n--- a/git-rebase--merge.sh\n+++ b/git-rebase--merge.sh\n@@ -96,7 +96,9 @@ finish_rb_merge () {\n if test -s \"$state_dir\"/rewritten\n then\n git notes copy --for-rewrite=rebase <\"$state_dir\"/rewritten\n- hook=\"$(git rev-parse --git-path hooks/post-rewrite)\"\n+ hooks_path=$(git config --get core.hooksPath)\n+ hooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+ hook=\"${hooks_path}/post-rewrite\"\n test -x \"$hook\" && \"$hook\" rebase <\"$state_dir\"/rewritten\n fi\n say All done.\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 04f6e44..c9ba747 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -203,10 +203,13 @@ run_specific_rebase () {\n }\n\n run_pre_rebase_hook () {\n+ hooks_path=$(git config --get core.hooksPath)\n+ hooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+ hook=\"${hooks_path}/pre-rebase\"\n if test -z \"$ok_to_skip_pre_rebase\" &&\n- test -x \"$(git rev-parse --git-path hooks/pre-rebase)\"\n+ test -x \"$hook\"\n then\n- \"$(git rev-parse --git-path hooks/pre-rebase)\" ${1+\"$@\"} ||\n+ \"$hook\" ${1+\"$@\"} ||\n die \"$(gettext \"The pre-rebase hook refused to rebase.\")\"\n fi\n }\n-- \n2.9.3\n"},{"id":"299280","messageId":"CAKkAvazV8umqbs+rTEG2399Ox0pGL1YAXsgLqHusb15RhzyH7Q@mail.gmail.com","threadId":"43841","inReplyTo":"CAKkAvaxWk6SK4EYPaWXHQoVBh9qLgHoEqAh9+dgOrjncsi5QyA@mail.gmail.com","subject":"Re: [PATCH] make rebase respect core.hooksPath if set","fromName":"ryenus","fromEmail":"ryenus@gmail.com","sentAt":"2016-08-14T16:29:33Z","receivedAt":"2016-08-14T16:29:58Z","isPatch":true,"sender":{"key":"ryenus@gmail.com","avatar":"https://avatars.githubusercontent.com/u/610161?v=4"},"body":"Patch attached.\n\nIt seems gmail ruined the white spaces.\nNot sure how to stop gmail from doing that.\nSorry for me, and for Gmail.\n\n\nFrom 67418dd8ffad7c07ed95437f7a4d1359da9d93d2 Mon Sep 17 00:00:00 2001\nFrom: ryenus \nDate: Sun, 14 Aug 2016 19:16:27 +0800\nSubject: [PATCH] make rebase respect core.hooksPath if set\n\nwhen looking for pre-rebase and post-rewrite hooks, git-rebase\nonly looks for hooks dir using `git rev-parse --git-path hooks`,\nwhich didn't consider the path overridden by core.hooksPath.\n\nSigned-off-by: ryenus \n---\n git-rebase--interactive.sh | 14 +++++++++-----\n git-rebase--merge.sh | 4 +++-\n git-rebase.sh | 7 +++++--\n 3 files changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex e2da524..e8af70d 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -724,11 +724,15 @@ Commit or stash your changes, and then run\n \t\tgit notes copy --for-rewrite=rebase < \"$rewritten_list\" ||\n \t\ttrue # we don't care if this copying failed\n \t} &&\n-\thook=\"$(git rev-parse --git-path hooks/post-rewrite)\"\n-\tif test -x \"$hook\" && test -s \"$rewritten_list\"; then\n-\t\t\"$hook\" rebase < \"$rewritten_list\"\n-\t\ttrue # we don't care if this hook failed\n-\tfi &&\n+\t{\n+\t\thooks_path=$(git config --get core.hooksPath)\n+\t\thooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+\t\thook=\"${hooks_path}/post-rewrite\"\n+\t\tif test -x \"$hook\" && test -s \"$rewritten_list\"; then\n+\t\t\t\"$hook\" rebase < \"$rewritten_list\"\n+\t\t\ttrue # we don't care if this hook failed\n+\t\tfi\n+\t} &&\n \t\twarn \"$(eval_gettext \"Successfully rebased and updated \\$head_name.\")\"\n \n \treturn 1 # not failure; just to break the do_rest loop\ndiff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\nindex 06a4723..df5073e 100644\n--- a/git-rebase--merge.sh\n+++ b/git-rebase--merge.sh\n@@ -96,7 +96,9 @@ finish_rb_merge () {\n \tif test -s \"$state_dir\"/rewritten\n \tthen\n \t\tgit notes copy --for-rewrite=rebase <\"$state_dir\"/rewritten\n-\t\thook=\"$(git rev-parse --git-path hooks/post-rewrite)\"\n+\t\thooks_path=$(git config --get core.hooksPath)\n+\t\thooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+\t\thook=\"${hooks_path}/post-rewrite\"\n \t\ttest -x \"$hook\" && \"$hook\" rebase <\"$state_dir\"/rewritten\n \tfi\n \tsay All done.\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 04f6e44..c9ba747 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -203,10 +203,13 @@ run_specific_rebase () {\n }\n \n run_pre_rebase_hook () {\n+\thooks_path=$(git config --get core.hooksPath)\n+\thooks_path=\"${hooks_path:-$(git rev-parse --git-path hooks)}\"\n+\thook=\"${hooks_path}/pre-rebase\"\n \tif test -z \"$ok_to_skip_pre_rebase\" &&\n-\t test -x \"$(git rev-parse --git-path hooks/pre-rebase)\"\n+\t test -x \"$hook\"\n \tthen\n-\t\t\"$(git rev-parse --git-path hooks/pre-rebase)\" ${1+\"$@\"} ||\n+\t\t\"$hook\" ${1+\"$@\"} ||\n \t\tdie \"$(gettext \"The pre-rebase hook refused to rebase.\")\"\n \tfi\n }\n-- \n2.9.3\n\n"},{"id":"299283","messageId":"CANoM8SWutGQaHJa6VCSJbRrFc9Dap0pKNUj74hhjm6hyHtYXtg@mail.gmail.com","threadId":"43841","inReplyTo":"CAKkAvazV8umqbs+rTEG2399Ox0pGL1YAXsgLqHusb15RhzyH7Q@mail.gmail.com","subject":"Re: [PATCH] make rebase respect core.hooksPath if set","fromName":"Mike Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-08-14T20:03:48Z","receivedAt":"2016-08-14T20:04:13Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"On Sun, Aug 14, 2016 at 12:29 PM, ryenus wrote:\n> Patch attached.\n>\n> It seems gmail ruined the white spaces.\n> Not sure how to stop gmail from doing that.\n> Sorry for me, and for Gmail.\n\nDid you use git-send-email? I don't think that the gmail ui works.\nIf you have 2-factor authentication, there are instructions on how to\nset that up in the docs in Documentation/git-format-patch.txt\n"},{"id":"299330","messageId":"alpine.DEB.2.20.1608151422210.4924@virtualbox","threadId":"43841","inReplyTo":"CAKkAvaxWk6SK4EYPaWXHQoVBh9qLgHoEqAh9+dgOrjncsi5QyA@mail.gmail.com","subject":"Re: [PATCH] make rebase respect core.hooksPath if set","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-15T12:24:59Z","receivedAt":"2016-08-15T12:25:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi ryenus,\n\nOn Sun, 14 Aug 2016, ryenus wrote:\n\n> when looking for pre-rebase and post-rewrite hooks, git-rebase\n> only looks for hooks dir using `git rev-parse --git-path hooks`,\n> which didn't consider the path overridden by core.hooksPath.\n\nWould it not be more appropriate to teach --git-path hooks to respect the\ncore.hooksPath variable? This would be in line with --git-path objects\nrespecting the GIT_OBJECT_DIRECTORY environment variable.\n\n> Signed-off-by: ryenus \n\nFrom\nhttps://github.com/git/git/blob/v2.9.3/Documentation/SubmittingPatches#L290-L291\n\n> Also notice that a real name is used in the Signed-off-by: line. Please\n> don't hide your real name.\n\nThanks,\nJohannes\n"},{"id":"299334","messageId":"20160815123150.nmgch55wzzcv5oue@sigill.intra.peff.net","threadId":"43841","inReplyTo":"alpine.DEB.2.20.1608151422210.4924@virtualbox","subject":"Re: [PATCH] make rebase respect core.hooksPath if set","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-15T12:31:50Z","receivedAt":"2016-08-15T12:31:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 15, 2016 at 02:24:59PM +0200, Johannes Schindelin wrote:\n\n> > when looking for pre-rebase and post-rewrite hooks, git-rebase\n> > only looks for hooks dir using `git rev-parse --git-path hooks`,\n> > which didn't consider the path overridden by core.hooksPath.\n> \n> Would it not be more appropriate to teach --git-path hooks to respect the\n> core.hooksPath variable? This would be in line with --git-path objects\n> respecting the GIT_OBJECT_DIRECTORY environment variable.\n\nGood idea. I think that logic is all in path.c:adjust_git_path().\n\nAnd then I suspect the manual handling of git_hooks_path in find_hook()\ncould go away (because strbuf_git_path would just do the right thing\nautomatically).\n\n-Peff\n"},{"id":"299427","messageId":"CAKkAvaxEeOvCy-8EZ=BeWVUPCLec_dFNv+dNpj9_VsECzAT2YA@mail.gmail.com","threadId":"43841","inReplyTo":"alpine.DEB.2.20.1608151422210.4924@virtualbox","subject":"Re: [PATCH] make rebase respect core.hooksPath if set","fromName":"ryenus","fromEmail":"ryenus@gmail.com","sentAt":"2016-08-16T02:01:53Z","receivedAt":"2016-08-16T02:02:19Z","isPatch":true,"sender":{"key":"ryenus@gmail.com","avatar":"https://avatars.githubusercontent.com/u/610161?v=4"},"body":"On 15 August 2016 at 20:24, Johannes Schindelin\n wrote:\n\n> Would it not be more appropriate to teach --git-path hooks to respect the\n> core.hooksPath variable? This would be in line with --git-path objects\n> respecting the GIT_OBJECT_DIRECTORY environment variable.\n\nIndeed, I've thought about that, too, due to the bad smell of duplication,\nbut not sure if that's the right position among all the abstraction layers.\n\nAlso it's more convenient to come up with a shell based fix on local\ninstallation.\n\nThanks\n"}]}