public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed] gcc-changelog: Use non-zero exit status on error
@ 2020-06-09 19:42 Jonathan Wakely
  2020-06-09 19:46 ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-09 19:42 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Also add comment explaining what the script does.

contrib/ChangeLog:

	* gcc-changelog/git_email.py: Set exit status on error.


Committed as obvious.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 1122 bytes --]

commit 62963c60fc19d07615afe9d4f1b897b2f60801b2
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Jun 9 20:39:39 2020 +0100

    gcc-changelog: Use non-zero exit status on error
    
    Also add comment explaining what the script does.
    
    contrib/ChangeLog:
    
            * gcc-changelog/git_email.py: Set exit status on error.

diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
index 367cf76d8ee..bf74bd8b156 100755
--- a/contrib/gcc-changelog/git_email.py
+++ b/contrib/gcc-changelog/git_email.py
@@ -70,6 +70,9 @@ class GitEmail(GitCommit):
                          strict=strict)
 
 
+# With zero arguments, process every patch file in the ./patches directory.
+# With one argument, process the named patch file.
+# Patch files must be in 'git format-patch' format.
 if __name__ == '__main__':
     if len(sys.argv) == 1:
         allfiles = []
@@ -100,3 +103,4 @@ if __name__ == '__main__':
             if not email.lines:
                 print('Error: patch contains no parsed lines', file=sys.stderr)
             email.print_errors()
+            sys.exit(1)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-09 19:42 [committed] gcc-changelog: Use non-zero exit status on error Jonathan Wakely
@ 2020-06-09 19:46 ` Jonathan Wakely
  2020-06-10  7:43   ` Martin Liška
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-09 19:46 UTC (permalink / raw)
  To: gcc-patches

On 09/06/20 20:42 +0100, Jonathan Wakely wrote:
>Also add comment explaining what the script does.
>
>contrib/ChangeLog:
>
>	* gcc-changelog/git_email.py: Set exit status on error.
>
>
>Committed as obvious.

With that change to git_email.py I can use the following change to the
contrib/prepare-commit-msg hook to avoid adding redundant mklog.py
output when amending a commit:

--- ../contrib/prepare-commit-msg       2020-06-09 19:18:01.048028044 +0100
+++ ../.git/hooks/prepare-commit-msg    2020-06-09 20:36:09.587850393 +0100
@@ -49,6 +49,17 @@
      # otherwise, assume a new commit with -C.
      if [ $SHA1 = HEAD ]; then
         cmd="diff --cached HEAD^"
+       check=contrib/gcc-changelog/git_email.py
+       f=$(mktemp /tmp/git-commit.XXXXXX) || exit 1
+       git log -1 --pretty=email HEAD > $f
+       printf '\n---\n\n' >> $f
+       git $cmd >> $f
+       if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then
+           # Existing commit message is still OK for amended commit
+           rm $f
+           exit 0
+       fi
+       rm $f
      else
         cmd="diff --cached"
      fi


This way the mklog.py command only appends a skeleton changelog if the
existing log message fails verification for the amended commit.

Is this worth adding to contrib/prepare-commit-msg?


>commit 62963c60fc19d07615afe9d4f1b897b2f60801b2
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Tue Jun 9 20:39:39 2020 +0100
>
>    gcc-changelog: Use non-zero exit status on error
>
>    Also add comment explaining what the script does.
>
>    contrib/ChangeLog:
>
>            * gcc-changelog/git_email.py: Set exit status on error.
>
>diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
>index 367cf76d8ee..bf74bd8b156 100755
>--- a/contrib/gcc-changelog/git_email.py
>+++ b/contrib/gcc-changelog/git_email.py
>@@ -70,6 +70,9 @@ class GitEmail(GitCommit):
>                          strict=strict)
>
>
>+# With zero arguments, process every patch file in the ./patches directory.
>+# With one argument, process the named patch file.
>+# Patch files must be in 'git format-patch' format.
> if __name__ == '__main__':
>     if len(sys.argv) == 1:
>         allfiles = []
>@@ -100,3 +103,4 @@ if __name__ == '__main__':
>             if not email.lines:
>                 print('Error: patch contains no parsed lines', file=sys.stderr)
>             email.print_errors()
>+            sys.exit(1)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-09 19:46 ` Jonathan Wakely
@ 2020-06-10  7:43   ` Martin Liška
  2020-06-10 12:13     ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Liška @ 2020-06-10  7:43 UTC (permalink / raw)
  To: Jonathan Wakely, gcc-patches

On 6/9/20 9:46 PM, Jonathan Wakely wrote:
> Is this worth adding to contrib/prepare-commit-msg?

I like the idea and I would make it conditional based on
an environment variable? Similarly to GCC_GIT_DIFF_FILE?

Thanks,
Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-10  7:43   ` Martin Liška
@ 2020-06-10 12:13     ` Jonathan Wakely
  2020-06-10 12:15       ` Jonathan Wakely
  2020-06-10 12:20       ` Jonathan Wakely
  0 siblings, 2 replies; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-10 12:13 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

On 10/06/20 09:43 +0200, Martin Liška wrote:
>On 6/9/20 9:46 PM, Jonathan Wakely wrote:
>>Is this worth adding to contrib/prepare-commit-msg?
>
>I like the idea and I would make it conditional based on
>an environment variable? Similarly to GCC_GIT_DIFF_FILE?

With this patch you can use the gcc-config.use-mklog-hook config key
instead of defining GCC_FORCE_MKLOG in the environment.

My new behaviour is enabled when that variable is set to
'smart-amend' (either in the environment, or in your git config).

WDYT?


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2303 bytes --]

commit a58493ff8f3d36645c6d4fad2fca04e3db2d267c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jun 10 12:48:40 2020 +0100

    contrib: Make prepare-commit-msg hook smarter
    
    With this change defining GCC_FORCE_MKLOG=smart-amend in the environment
    will cause the prepare-commit-msg hook to compare the log of a commit
    being amended with the staged changes, and not run mklog.py
    unnecessarily.
    
    This also allows defining gcc-config.use-mklog-hook in Git config
    instead of the GCC_FORCE_MKLOG environment variable, and allows
    GCC_FORCE_MKLOG=no to do nothing even if the config key is set.
    
    contrib/ChangeLog:
    
            * prepare-commit-msg: Do not generate a new ChangeLog template
            for amended commits where the existing log is still OK. Check
            the gcc-config.use-mklog-hook Git config key if the
            GCC_FORCE_MKLOG environment variable is empty.

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 24f0783aae2..cfcf0e7afe5 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -30,7 +30,11 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi
 if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi
 
 # Don't do anything unless requested to.
-if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi
+if [ -z "$GCC_FORCE_MKLOG" ]; then
+    GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0
+    echo xxx $GCC_FORCE_MKLOG
+fi
+if [ -z "$GCC_FORCE_MKLOG" ] || [ $GCC_FORCE_MKLOG = no ]; then exit 0; fi
 
 if [ -z "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
     # No source or "template" means new commit.
@@ -49,6 +53,19 @@ elif [ $COMMIT_SOURCE = commit ]; then
     # otherwise, assume a new commit with -C.
     if [ $SHA1 = HEAD ]; then
 	cmd="diff --cached HEAD^"
+	if [ "$GCC_FORCE_MKLOG" = "smart-amend" ]; then
+	    # Check if the existing message still describes the staged changes.
+	    f=$(mktemp /tmp/git-commit.XXXXXX) || exit 1
+	    git log -1 --pretty=email HEAD > $f
+	    printf '\n---\n\n' >> $f
+	    git $cmd >> $f
+	    if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then
+		# Existing commit message is still OK for amended commit.
+		rm $f
+		exit 0
+	    fi
+	    rm $f
+	fi
     else
 	cmd="diff --cached"
     fi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-10 12:13     ` Jonathan Wakely
@ 2020-06-10 12:15       ` Jonathan Wakely
  2020-06-11  7:45         ` Martin Liška
  2020-06-10 12:20       ` Jonathan Wakely
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-10 12:15 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

On 10/06/20 13:13 +0100, Jonathan Wakely wrote:
>On 10/06/20 09:43 +0200, Martin Liška wrote:
>>On 6/9/20 9:46 PM, Jonathan Wakely wrote:
>>>Is this worth adding to contrib/prepare-commit-msg?
>>
>>I like the idea and I would make it conditional based on
>>an environment variable? Similarly to GCC_GIT_DIFF_FILE?
>
>With this patch you can use the gcc-config.use-mklog-hook config key
>instead of defining GCC_FORCE_MKLOG in the environment.
>
>My new behaviour is enabled when that variable is set to
>'smart-amend' (either in the environment, or in your git config).

And this is another little patch that just avoids running 'git diff'
twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect
of generating the diff to pipe to mklog.py.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 827 bytes --]

commit e147d6164775d004d608501963befeb551732529
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jun 10 13:05:39 2020 +0100

    contrib: Avoid redundant 'git diff' in prepare-commit-msg hook
    
    contrib/ChangeLog:
    
            * prepare-commit-msg: Use 'tee' to save the diff to a file
            instead of running 'git diff' twice.

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index cfcf0e7afe5..d27861f4cd6 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -76,7 +76,9 @@ fi
 
 # Save diff to a file if requested.
 if ! [ -z "$GCC_GIT_DIFF_FILE" ]; then
-  git $cmd > "$GCC_GIT_DIFF_FILE";
+    tee="tee $GCC_GIT_DIFF_FILE"
+else
+    tee="cat"
 fi
 
-git $cmd | git gcc-mklog -c "$COMMIT_MSG_FILE"
+git $cmd | $tee | git gcc-mklog -c "$COMMIT_MSG_FILE"

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-10 12:13     ` Jonathan Wakely
  2020-06-10 12:15       ` Jonathan Wakely
@ 2020-06-10 12:20       ` Jonathan Wakely
  2020-06-11  7:54         ` Martin Liška
  1 sibling, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-10 12:20 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2205 bytes --]

On 10/06/20 13:13 +0100, Jonathan Wakely wrote:
>On 10/06/20 09:43 +0200, Martin Liška wrote:
>>On 6/9/20 9:46 PM, Jonathan Wakely wrote:
>>>Is this worth adding to contrib/prepare-commit-msg?
>>
>>I like the idea and I would make it conditional based on
>>an environment variable? Similarly to GCC_GIT_DIFF_FILE?
>
>With this patch you can use the gcc-config.use-mklog-hook config key
>instead of defining GCC_FORCE_MKLOG in the environment.
>
>My new behaviour is enabled when that variable is set to
>'smart-amend' (either in the environment, or in your git config).
>
>WDYT?
>

>commit a58493ff8f3d36645c6d4fad2fca04e3db2d267c
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Wed Jun 10 12:48:40 2020 +0100
>
>    contrib: Make prepare-commit-msg hook smarter
>    
>    With this change defining GCC_FORCE_MKLOG=smart-amend in the environment
>    will cause the prepare-commit-msg hook to compare the log of a commit
>    being amended with the staged changes, and not run mklog.py
>    unnecessarily.
>    
>    This also allows defining gcc-config.use-mklog-hook in Git config
>    instead of the GCC_FORCE_MKLOG environment variable, and allows
>    GCC_FORCE_MKLOG=no to do nothing even if the config key is set.
>    
>    contrib/ChangeLog:
>    
>            * prepare-commit-msg: Do not generate a new ChangeLog template
>            for amended commits where the existing log is still OK. Check
>            the gcc-config.use-mklog-hook Git config key if the
>            GCC_FORCE_MKLOG environment variable is empty.
>
>diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
>index 24f0783aae2..cfcf0e7afe5 100755
>--- a/contrib/prepare-commit-msg
>+++ b/contrib/prepare-commit-msg
>@@ -30,7 +30,11 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi
> if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi
> 
> # Don't do anything unless requested to.
>-if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi
>+if [ -z "$GCC_FORCE_MKLOG" ]; then
>+    GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0
>+    echo xxx $GCC_FORCE_MKLOG

Oops, this line was left in while I was testing it by amending
existing commits!

Here's an updated patch without that line.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2339 bytes --]

commit cd16c99a5fe281fc60b2023fc302994580078ca1
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jun 10 12:48:40 2020 +0100

    contrib: Make prepare-commit-msg hook smarter
    
    With this change defining GCC_FORCE_MKLOG=smart-amend in the environment
    will cause the prepare-commit-msg hook to compare the log of a commit
    being amended with the staged changes, and not run mklog.py
    unnecessarily.
    
    This also allows defining gcc-config.use-mklog-hook in Git config
    instead of the GCC_FORCE_MKLOG environment variable, and allows
    GCC_FORCE_MKLOG=no to do nothing even if the config key is set.
    
    contrib/ChangeLog:
    
            * prepare-commit-msg: Do not generate a new ChangeLog template
            for amended commits where the existing log is still OK. Check
            the gcc-config.use-mklog-hook Git config key if the
            GCC_FORCE_MKLOG environment variable is empty.
    
    contrib/ChangeLog:
    
            * prepare-commit-msg:

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 24f0783aae2..34f94012eae 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -30,7 +30,10 @@ if ! [ -x contrib/mklog.py ]; then exit 0; fi
 if ! [ -f "$COMMIT_MSG_FILE" ]; then exit 0; fi
 
 # Don't do anything unless requested to.
-if [ -z "$GCC_FORCE_MKLOG" ]; then exit 0; fi
+if [ -z "$GCC_FORCE_MKLOG" ]; then
+    GCC_FORCE_MKLOG=$(git config gcc-config.use-mklog-hook) || exit 0
+fi
+if [ -z "$GCC_FORCE_MKLOG" ] || [ $GCC_FORCE_MKLOG = no ]; then exit 0; fi
 
 if [ -z "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
     # No source or "template" means new commit.
@@ -49,6 +52,19 @@ elif [ $COMMIT_SOURCE = commit ]; then
     # otherwise, assume a new commit with -C.
     if [ $SHA1 = HEAD ]; then
 	cmd="diff --cached HEAD^"
+	if [ "$GCC_FORCE_MKLOG" = "smart-amend" ]; then
+	    # Check if the existing message still describes the staged changes.
+	    f=$(mktemp /tmp/git-commit.XXXXXX) || exit 1
+	    git log -1 --pretty=email HEAD > $f
+	    printf '\n---\n\n' >> $f
+	    git $cmd >> $f
+	    if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then
+		# Existing commit message is still OK for amended commit.
+		rm $f
+		exit 0
+	    fi
+	    rm $f
+	fi
     else
 	cmd="diff --cached"
     fi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-10 12:15       ` Jonathan Wakely
@ 2020-06-11  7:45         ` Martin Liška
  2020-06-11  8:07           ` Martin Liška
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Liška @ 2020-06-11  7:45 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

On 6/10/20 2:15 PM, Jonathan Wakely wrote:
> And this is another little patch that just avoids running 'git diff'
> twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect
> of generating the diff to pipe to mklog.py.

Thanks, please install the improvement.

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-10 12:20       ` Jonathan Wakely
@ 2020-06-11  7:54         ` Martin Liška
  2020-06-11  8:48           ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Liška @ 2020-06-11  7:54 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

On 6/10/20 2:20 PM, Jonathan Wakely wrote:
> Oops, this line was left in while I was testing it by amending
> existing commits!
> 
> Here's an updated patch without that line.

I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG.
Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias.
Wouldn't it be better to only add

git config gcc-config.mklog-hook-type [always|smart-amend]

and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?
Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?

Thanks,
Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-11  7:45         ` Martin Liška
@ 2020-06-11  8:07           ` Martin Liška
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Liška @ 2020-06-11  8:07 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

On 6/11/20 9:45 AM, Martin Liška wrote:
> On 6/10/20 2:15 PM, Jonathan Wakely wrote:
>> And this is another little patch that just avoids running 'git diff'
>> twice, by using tee(1) to write to $GCC_GIT_DIFF_FILE as a side effect
>> of generating the diff to pipe to mklog.py.
> 
> Thanks, please install the improvement.
> 
> Martin

I've pushed the patch as I'm going to replace my GCC_GIT_DIFF_FILE environment
variable with a more feasible 'git config gcc-config.diff-file'.

Pushed to master as well.
Martin

[-- Attachment #2: 0001-prepare-commit-hook-Use-gcc-config.diff-file.patch --]
[-- Type: text/x-patch, Size: 848 bytes --]

From 2dd6d9d4029d99c5ad54a21677506bd25e3f7540 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Thu, 11 Jun 2020 10:02:26 +0200
Subject: [PATCH] prepare-commit-hook: Use gcc-config.diff-file.

contrib/ChangeLog:

	* prepare-commit-msg: Replace ENV variable with a git config
	value.
---
 contrib/prepare-commit-msg | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 57bb91747f6..d4971e65092 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -58,8 +58,9 @@ else
 fi
 
 # Save diff to a file if requested.
-if ! [ -z "$GCC_GIT_DIFF_FILE" ]; then
-    tee="tee $GCC_GIT_DIFF_FILE"
+DIFF_FILE = $(git config gcc-config.diff-file)
+if ! [ -z "$DIFF_FILE" ]; then
+    tee="tee $DIFF_FILE"
 else
     tee="cat"
 fi
-- 
2.26.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-11  7:54         ` Martin Liška
@ 2020-06-11  8:48           ` Jonathan Wakely
  2020-06-11  8:54             ` Martin Liška
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-11  8:48 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches

On 11/06/20 09:54 +0200, Martin Liška wrote:
>On 6/10/20 2:20 PM, Jonathan Wakely wrote:
>>Oops, this line was left in while I was testing it by amending
>>existing commits!
>>
>>Here's an updated patch without that line.
>
>I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG.
>Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias.

Ah yes, I forgot about that alias (I don't use it).

>Wouldn't it be better to only add
>
>git config gcc-config.mklog-hook-type [always|smart-amend]
>
>and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?

Sure, that works for me.

>Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?

No, I was actually using a modified version of the script that didn't
check the env var at all, because I always wanted it to be used.

I'll prepare a patch that replaces the env var completely, and also
modifies the contrib/gcc-git-customization.sh script to optionally
enable the hook.

Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
won't do anything different from 'git commit'?



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-11  8:48           ` Jonathan Wakely
@ 2020-06-11  8:54             ` Martin Liška
  2020-06-11  9:47               ` Jonathan Wakely
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Liška @ 2020-06-11  8:54 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

On 6/11/20 10:48 AM, Jonathan Wakely wrote:
> On 11/06/20 09:54 +0200, Martin Liška wrote:
>> On 6/10/20 2:20 PM, Jonathan Wakely wrote:
>>> Oops, this line was left in while I was testing it by amending
>>> existing commits!
>>>
>>> Here's an updated patch without that line.
>>
>> I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG.
>> Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias.
> 
> Ah yes, I forgot about that alias (I don't use it).

I use it (and I want to distinguish in between it and normal commit command) ;)

> 
>> Wouldn't it be better to only add
>>
>> git config gcc-config.mklog-hook-type [always|smart-amend]
>>
>> and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?
> 
> Sure, that works for me.
> 
>> Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?
> 
> No, I was actually using a modified version of the script that didn't
> check the env var at all, because I always wanted it to be used.

I bet some other people (including me) want to have 2 different commands.

> 
> I'll prepare a patch that replaces the env var completely, and also
> modifies the contrib/gcc-git-customization.sh script to optionally
> enable the hook.

I would preserve it for the alias.

Martin

> 
> Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
> won't do anything different from 'git commit'?
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-11  8:54             ` Martin Liška
@ 2020-06-11  9:47               ` Jonathan Wakely
  2020-06-11  9:50                 ` Martin Liška
  0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Wakely @ 2020-06-11  9:47 UTC (permalink / raw)
  To: Martin Liška; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

On 11/06/20 10:54 +0200, Martin Liška wrote:
>On 6/11/20 10:48 AM, Jonathan Wakely wrote:
>>On 11/06/20 09:54 +0200, Martin Liška wrote:
>>>On 6/10/20 2:20 PM, Jonathan Wakely wrote:
>>>>Oops, this line was left in while I was testing it by amending
>>>>existing commits!
>>>>
>>>>Here's an updated patch without that line.
>>>
>>>I generally like the suggested idea and I have suggestion to usage of the GCC_FORCE_MKLOG.
>>>Right now, we use the env variable only in 'git config alias.gcc-commit-mklog' alias.
>>
>>Ah yes, I forgot about that alias (I don't use it).
>
>I use it (and I want to distinguish in between it and normal commit command) ;)

OK. I'd rather just have one command that always does the right thing,
so want the hook to always run. But I can just change my local copy.

>>>Wouldn't it be better to only add
>>>
>>>git config gcc-config.mklog-hook-type [always|smart-amend]
>>>
>>>and do not support GCC_FORCE_MKLOG=no or GCC_FORCE_MKLOG=smart-amend?

The point of GCC_FORCE_MKLOG=no was as an override to disable the hook
if you have it enabled in your git config options.

If my git config enables the hook, but I want it off for a single
commit, I can use the env var to disable it.

For your scenario, you want the hook off by default, but enabled when
the env var is set (by the gcc-commit-mklog alias).

>>Sure, that works for me.
>>
>>>Or do you have always GCC_FORCE_MKLOG=1 on (I mean for git commit)?
>>
>>No, I was actually using a modified version of the script that didn't
>>check the env var at all, because I always wanted it to be used.
>
>I bet some other people (including me) want to have 2 different commands.
>
>>
>>I'll prepare a patch that replaces the env var completely, and also
>>modifies the contrib/gcc-git-customization.sh script to optionally
>>enable the hook.
>
>I would preserve it for the alias.

OK, I misunderstood what you were suggesting.

Is the attached patch what you're asking for?

>Martin
>
>>
>>Do we want to drop the gcc-config.gcc-commit-mklog alias now, since it
>>won't do anything different from 'git commit'?
>>
>>
>

[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1602 bytes --]

commit c0e9ad58b132d01a3d7bb7eaeb981324eb55074a
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jun 11 10:22:04 2020 +0100

    contrib: Make prepare-commit-msg hook smarter for amends
    
    With this change the prepare-commit-msg hook can compare the log of a
    commit being amended with the staged changes, and not run mklog.py
    unnecessarily. This is controlled by a git config option,
    gcc-config.mklog-hook-type.
    
    contrib/ChangeLog:
    
            * prepare-commit-msg: Use the gcc-config.mklog-hook-type Git
            config key instead of the GCC_FORCE_MKLOG environment variable.
            Optionally disable generating a new ChangeLog template for
            amended commits when the existing log is still OK.

diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index cc9ba2e6ba1..969847df6f4 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -49,6 +49,19 @@ elif [ $COMMIT_SOURCE = commit ]; then
     # otherwise, assume a new commit with -C.
     if [ $SHA1 = HEAD ]; then
 	cmd="diff --cached HEAD^"
+	if [ "$(git config gcc-config.mklog-hook-type)" = "smart-amend" ]; then
+	    # Check if the existing message still describes the staged changes.
+	    f=$(mktemp /tmp/git-commit.XXXXXX) || exit 1
+	    git log -1 --pretty=email HEAD > $f
+	    printf '\n---\n\n' >> $f
+	    git $cmd >> $f
+	    if contrib/gcc-changelog/git_email.py "$f" >/dev/null 2>&1; then
+		# Existing commit message is still OK for amended commit.
+		rm $f
+		exit 0
+	    fi
+	    rm $f
+	fi
     else
 	cmd="diff --cached"
     fi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [committed] gcc-changelog: Use non-zero exit status on error
  2020-06-11  9:47               ` Jonathan Wakely
@ 2020-06-11  9:50                 ` Martin Liška
  0 siblings, 0 replies; 13+ messages in thread
From: Martin Liška @ 2020-06-11  9:50 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches

On 6/11/20 11:47 AM, Jonathan Wakely wrote:
> Is the attached patch what you're asking for?

Yes ;)

Martin

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-06-11  9:50 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 19:42 [committed] gcc-changelog: Use non-zero exit status on error Jonathan Wakely
2020-06-09 19:46 ` Jonathan Wakely
2020-06-10  7:43   ` Martin Liška
2020-06-10 12:13     ` Jonathan Wakely
2020-06-10 12:15       ` Jonathan Wakely
2020-06-11  7:45         ` Martin Liška
2020-06-11  8:07           ` Martin Liška
2020-06-10 12:20       ` Jonathan Wakely
2020-06-11  7:54         ` Martin Liška
2020-06-11  8:48           ` Jonathan Wakely
2020-06-11  8:54             ` Martin Liška
2020-06-11  9:47               ` Jonathan Wakely
2020-06-11  9:50                 ` Martin Liška

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 read-only IMAP folder(s) and NNTP newsgroup(s).