From: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
To: gitster@pobox.com
Cc: git@vger.kernel.org, sbeller@google.com
Subject: [PATCH v2/RFC] hook: use correct logical variable
Date: Thu, 17 Aug 2017 08:20:24 +0530 [thread overview]
Message-ID: <20170817025024.6517-1-kaarticsivaraam91196@gmail.com> (raw)
In-Reply-To: <1502938058.1710.14.camel@gmail.com>
In general, a 'Sign-off' added should be that of the *committer* and not
that of the *commit's author*. As the 3rd part of the 'prepare-commit-msg'
hook appended the sign-off of the *commit's author* it worked weirdly in
some cases. For example 'git commit --amend -s' when coupled with that part
of the script woked weirdly as illustrated by the following scenario in which
the last commit's log message has the following trailer,
...
Signed-off-by: Random Developer <developer@example.com>
and the commit's author is "Random Developer <developer@example.com>". Assume
that the commit is trying to be amended by another developer who's identity is
"Another Developer <another-dev@example.com>". When he tries to do
$ git commit --amend -s
with the 3rd part of the hook enabled then the trailer he would see in his editor
would be,
...
Signed-off-by: Random Developer <developer@example.com>
Signed-off-by: Another Developer <another-dev@example.com>
Signed-off-by: Random Developer <developer@example.com>
This is because,
* the hook is invoked only after the sign-off is appended by the '-s' option
* the script tries to add the sign-off of the *commit's author* using interpret-trailers
and 'interpret-trailers' in it's default configuration tries to adds the trailer
when the *neighbouring* trailer isn't the same as the one trying to be added.
This is just an example and this kind of issue could repeat if similar conditions are
satisified for other cases.
Moreover the rest of Git adds the sign-off of the *committer* using sequencer.c::append_signoff().
So, use the correct logical variable that identifies the committer to append the sign-off
in the sample hook script.
Bottom line: Being consistent prevents all sorts of weird issues.
Signed-off-by: Kaartic Sivaraam <kaarticsivaraam91196@gmail.com>
---
Changes in v2:
- updated the commit message
Suggestions regarding ways to improve the message are most welcome.
templates/hooks--prepare-commit-msg.sample | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/templates/hooks--prepare-commit-msg.sample b/templates/hooks--prepare-commit-msg.sample
index a84c3e5a8..12dd8fd88 100755
--- a/templates/hooks--prepare-commit-msg.sample
+++ b/templates/hooks--prepare-commit-msg.sample
@@ -34,7 +34,7 @@ SHA1=$3
# *) ;;
# esac
-# SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+# SOB=$(git var GIT_COMMITTER_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
# git interpret-trailers --in-place --trailer "$SOB" "$COMMIT_MSG_FILE"
# if test -z "$COMMIT_SOURCE"
# then
--
2.14.0.rc1.434.g6eded367a
next prev parent reply other threads:[~2017-08-17 2:49 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-30 15:43 [PATCH] hooks: add signature to the top of the commit message Kaartic Sivaraam
2017-06-30 16:44 ` Junio C Hamano
2017-07-01 14:15 ` Kaartic Sivaraam
2017-07-01 16:16 ` "git intepret-trailers" vs. "sed script" to add the signature Kaartic Sivaraam
2017-07-01 17:32 ` [PATCH/RFC] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-03 16:58 ` "git intepret-trailers" vs. "sed script" to add the signature Junio C Hamano
2017-07-04 19:16 ` Kaartic Sivaraam
2017-07-05 1:48 ` Junio C Hamano
2017-07-05 17:00 ` [PATCH] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-05 17:35 ` Kaartic Sivaraam
2017-07-05 19:37 ` Junio C Hamano
2017-07-05 20:14 ` Ramsay Jones
2017-07-06 14:30 ` Kaartic Sivaraam
2017-07-01 17:36 ` [PATCH] hooks: add signature to the top of the commit message Junio C Hamano
2017-07-01 18:40 ` Philip Oakley
2017-07-01 20:28 ` Junio C Hamano
2017-07-01 21:00 ` Philip Oakley
2017-07-01 18:52 ` Kaartic Sivaraam
2017-07-01 20:31 ` Junio C Hamano
2017-07-02 11:19 ` Kaartic Sivaraam
2017-07-02 11:27 ` [PATCH/RFC] hooks: replace irrelevant hook sample Kaartic Sivaraam
2017-07-05 16:51 ` [PATCH] " Kaartic Sivaraam
2017-07-05 19:50 ` Junio C Hamano
2017-07-07 11:53 ` Kaartic Sivaraam
2017-07-07 15:05 ` Junio C Hamano
2017-07-07 15:24 ` Kaartic Sivaraam
2017-07-07 16:07 ` [PATCH 1/2] " Kaartic Sivaraam
2017-07-07 16:07 ` [PATCH 2/2] hooks: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-07 18:27 ` [PATCH 1/2] hooks: replace irrelevant hook sample Junio C Hamano
2017-07-10 14:17 ` [PATCH 1/4] hook: cleanup script Kaartic Sivaraam
2017-07-10 14:17 ` [PATCH 2/4] hook: name the positional variables Kaartic Sivaraam
2017-07-10 19:51 ` Junio C Hamano
2017-07-10 14:17 ` [PATCH 3/4] hook: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-10 15:13 ` Ramsay Jones
2017-07-10 19:53 ` Junio C Hamano
2017-07-11 14:11 ` [PATCH 1/4] hook: cleanup script Kaartic Sivaraam
2017-07-11 14:11 ` [PATCH 2/4] hook: name the positional variables Kaartic Sivaraam
2017-07-11 14:11 ` [PATCH 3/4] hook: add sign-off using "interpret-trailers" Kaartic Sivaraam
2017-08-14 8:46 ` [PATCH] hook: use correct logical variable Kaartic Sivaraam
2017-08-14 17:54 ` Stefan Beller
2017-08-14 18:19 ` Junio C Hamano
2017-08-15 9:31 ` Kaartic Sivaraam
2017-08-15 17:28 ` Junio C Hamano
2017-08-17 2:47 ` Kaartic Sivaraam
2017-08-17 2:50 ` Kaartic Sivaraam [this message]
2017-08-15 9:32 ` Kaartic Sivaraam
2017-07-11 14:11 ` [PATCH 4/4] hook: add a simple first example Kaartic Sivaraam
2017-07-11 14:30 ` Kaartic Sivaraam
2017-07-11 13:10 ` [PATCH 3/4] hook: add signature using "interpret-trailers" Kaartic Sivaraam
2017-07-11 13:18 ` Kaartic Sivaraam
2017-07-10 14:17 ` [PATCH 4/4] hook: add a simple first example Kaartic Sivaraam
2017-07-10 20:02 ` Junio C Hamano
2017-07-11 13:29 ` Kaartic Sivaraam
2017-07-11 16:03 ` Junio C Hamano
2017-07-11 18:04 ` Kaartic Sivaraam
2017-07-11 18:06 ` Kaartic Sivaraam
2017-07-10 19:50 ` [PATCH 1/4] hook: cleanup script Junio C Hamano
2017-07-02 11:29 ` [PATCH] hooks: add script to HOOKS that allows adding notes from commit message Kaartic Sivaraam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170817025024.6517-1-kaarticsivaraam91196@gmail.com \
--to=kaarticsivaraam91196@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).