threads / patch / 6047

patchRe: [PATCH] hooks/pre-commit: add example to add Signed-off-by line to message

Subject: Re: [PATCH] hooks/pre-commit: add example to add Signed-off-by line to message

## tl;dr

11 messages between Nov 29, 2006 and Dec 22, 2006. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Johannes Schindelin· Nov 29, 2006, 11:20 UTC · lore

[PATCH] hooks/pre-commit: add example to add Signed-off-by line to message

It is good to have an example of how to supply a template for the commit message anyway.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 templates/hooks--pre-commit |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to templates/hooks--pre-commit +7 −0
diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
index 723a9ef..931f127 100644
--- a/templates/hooks--pre-commit
+++ b/templates/hooks--pre-commit
@@ -7,6 +7,13 @@
 #
 # To enable this hook, make this file executable.
 
+# Uncomment this to add a Signed-off-by line to the message.
+
+# . git-sh-setup
+# git var GIT_AUTHOR_IDENT | \
+# sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p" \
+# > $GIT_DIR/SQUASH_MSG
+
 # This is slightly modified from Andrew Morton's Perfect Patch.
 # Lines you introduce should not have trailing whitespace.
 # Also check for an indentation that has SP before a TAB.

-- 
1.4.4.1.gb475
Andy Parkins· Dec 20, 2006, 15:54 UTC · re: Johannes Schindelin · lore
On Wednesday 2006 November 29 11:20, Johannes Schindelin wrote:
> +# . git-sh-setup
> +# git var GIT_AUTHOR_IDENT | \
> +# sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p" \
> +# > $GIT_DIR/SQUASH_MSG

I've been using this; and very useful it is too. However, I just noticed that I was getting "Not a git repository" errors when I tried to commit in a subdirectory. I traced the problem to my use of this fragment. It seems that calling git-sh-setup inside the pre-commit hook was changing GIT_DIR (don't know why). When I changed this to

git var GIT_AUTHOR_IDENT | \
sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p" \
 >> $(git-rev-parse --git-dir)/SQUASH_MSG
It worked correctly again.
Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
Andy Parkins· Dec 20, 2006, 16:04 UTC · re: Johannes Schindelin · lore

[PATCH] hooks/pre-commit: add example to add Signed-off-by line to message

Based on Johannes Schindelin's earlier patch to perform the same function.

The call to git-sh-setup was causing "Not a git repository" errors after the pre-commit script had run. I removed the call and used $(git-rev-parse --git-dir) instead, which seems to have fixed the problem.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 templates/hooks--pre-commit |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to templates/hooks--pre-commit +5 −0
diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
index 723a9ef..87d9ccc 100644
--- a/templates/hooks--pre-commit
+++ b/templates/hooks--pre-commit
@@ -7,6 +7,11 @@
 #
 # To enable this hook, make this file executable.
 
+# Uncomment the below to add a Signed-off-by line to the message.
+#git var GIT_AUTHOR_IDENT | \
+#sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p" \
+# >> $(git-rev-parse --git-dir)/SQUASH_MSG
+
 # This is slightly modified from Andrew Morton's Perfect Patch.
 # Lines you introduce should not have trailing whitespace.
 # Also check for an indentation that has SP before a TAB.

-- 
1.4.4.2.g120e3
Junio C Hamano· Dec 20, 2006, 17:29 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 10 quoted lines
> Hi,
>
> On Wed, 20 Dec 2006, Andy Parkins wrote:
>
>> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
>
> Acked-by: Johannes E. Schindelin <johannes.schindelin@gmx.de>
>
> Ciao,
> Dscho

Why SQUASH_MSG? Doesn't it confuse git-explain when it wants to detect that you are in the middle of a squash merge?

Johannes Schindelin· Dec 20, 2006, 17:37 UTC · re: Junio C Hamano · lore
Hi,
On Wed, 20 Dec 2006, Junio C Hamano wrote:
Show 15 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Hi,
> >
> > On Wed, 20 Dec 2006, Andy Parkins wrote:
> >
> >> Signed-off-by: Andy Parkins <andyparkins@gmail.com>
> >
> > Acked-by: Johannes E. Schindelin <johannes.schindelin@gmx.de>
> >
> > Ciao,
> > Dscho
> 
> Why SQUASH_MSG?  Doesn't it confuse git-explain when it wants to
> detect that you are in the middle of a squash merge?

Yes, it confuses git-explain. Which did not existed when I concocted the first (broken) version of this patch. There are only two ways to give commit message templates AFAICT: misusing SQUASH_MSG and misusing MERGE_MSG.

Ciao, Dscho

Junio C Hamano· Dec 20, 2006, 17:44 UTC · re: Johannes Schindelin · lore
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 7 quoted lines
>> Why SQUASH_MSG?  Doesn't it confuse git-explain when it wants to
>> detect that you are in the middle of a squash merge?
>
> Yes, it confuses git-explain. Which did not existed when I concocted the 
> first (broken) version of this patch. There are only two ways to give 
> commit message templates AFAICT: misusing SQUASH_MSG and misusing 
> MERGE_MSG.
I wonder...

# An example hook script to check the commit log message. # Called by git-commit with one argument, the name of the file # that has the commit message. The hook should exit with non-zero # status after issuing an appropriate message if it wants to stop the # commit. The hook is allowed to edit the commit message file.

Johannes Schindelin· Dec 21, 2006, 08:26 UTC · re: Junio C Hamano · lore
Hi,
On Wed, 20 Dec 2006, Junio C Hamano wrote:
Show 17 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> Why SQUASH_MSG?  Doesn't it confuse git-explain when it wants to
> >> detect that you are in the middle of a squash merge?
> >
> > Yes, it confuses git-explain. Which did not existed when I concocted the 
> > first (broken) version of this patch. There are only two ways to give 
> > commit message templates AFAICT: misusing SQUASH_MSG and misusing 
> > MERGE_MSG.
> 
> I wonder...
> 
> # An example hook script to check the commit log message.
> # Called by git-commit with one argument, the name of the file
> # that has the commit message.  The hook should exit with non-zero
> # status after issuing an appropriate message if it wants to stop the
> # commit.  The hook is allowed to edit the commit message file.
Yes, I missed that.

Ciao, Dscho

Andy Parkins· Dec 21, 2006, 09:24 UTC · re: Junio C Hamano · lore

[PATCH] hooks/commit-msg: add example to add Signed-off-by line to message

After checking to see if the commit message already has the target signed-off-by (for example in --amend commits), this patch generates a signed off by line from the repository owner and adds it to the commit message.

Based on Johannes Schindelin's earlier patch to perform the same function.

Originally, this was done in the pre-commit hook but Junio pointed out that the commit-msg hook allows the message to be edited. This has the aditional advantage that the commit-msg hook gets passed the name of the message file as a parameter, so it doesn't have to figure out GIT_DIR for itself.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 templates/hooks--commit-msg |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Show changes to templates/hooks--commit-msg +5 −0
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index 0b906ca..ce76bdf 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -8,6 +8,11 @@
 #
 # To enable this hook, make this file executable.
 
+# Uncomment the below to add a Signed-off-by line to the message.
+#SOB=$(git var GIT_AUTHOR_IDENT | \
+#	sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p")
+#grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
+
 # This example catches duplicate Signed-off-by lines.
 
 test "" = "$(grep '^Signed-off-by: ' "$1" |

-- 
1.4.4.2.g120e3
Junio C Hamano· Dec 22, 2006, 05:45 UTC · re: Andy Parkins · lore

Re: [PATCH] hooks/commit-msg: add example to add Signed-off-by line to message

Andy Parkins <andyparkins@gmail.com> writes:
> +# Uncomment the below to add a Signed-off-by line to the message.
> +#SOB=$(git var GIT_AUTHOR_IDENT | \
> +#	sed -n "s/^\(.*\) [0-9]\+ [-+][0-9]\+$/Signed-off-by: \1/p")
> +#grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
I suspect this sed script is quite wrong.  Did you test it?
 * quoting with dq means you are losing one level of backslash
   to the shell and sed is not seeing them.
 * one-or-more regexp '+' is typically not found in sed.

Also if this is meant to also handle cases where somebody commits a patch received from an e-mail chain that multiple people signed off their own changes to the patch, your name may already appear earlier in the chain of existing Signed-off-by: lines to sign off an earlier edition of the patch, but what you are committing now would include your own changes you just made on top of what all these people did, and you would want to sign off that new change as well, I think. So checking the last instance of the existing signed-off-by line, ignoring earlier ones, may be more preferrable.

Andy Parkins· Dec 22, 2006, 09:28 UTC · re: Junio C Hamano · lore

Re: [PATCH] hooks/commit-msg: add example to add Signed-off-by line to message

On Friday 2006 December 22 05:45, Junio C Hamano wrote:
> I suspect this sed script is quite wrong.  Did you test it?

I plead not guilty; cut and pasted from Johannes. In it's defence; I've been using it since he gave it to me without trouble. My patch is wrapping it a little differently, but the sed is certainly the same.

Amazingly it does work though...
>  * quoting with dq means you are losing one level of backslash
>    to the shell and sed is not seeing them.
Unusual.  It doesn't seem to apply to non-recognised escapes:

$ echo "\1\(\)\+$/" \1\(\)\+$/

>  * one-or-more regexp '+' is typically not found in sed.
Seems to be in mine (GNU sed version 4.1.5).

$ echo "xxxxxx" | sed -n 's/\(x\+\)$/MATCH/p' MATCH

> off that new change as well, I think.  So checking the last
> instance of the existing signed-off-by line, ignoring earlier
> ones, may be more preferrable.
Accepted.

Notwithstanding the fact that it seems to be working for me; I'll fix the problems you highlight and re-patch.

Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

← back to recent threads