public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] mklog: handle Signed-Off-By, minor cleanup
@ 2023-07-21 11:08 Marc Poulhiès
  2023-08-22  6:56 ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Poulhiès @ 2023-07-21 11:08 UTC (permalink / raw)
  To: gcc-patches; +Cc: Marc Poulhiès

Consider Signed-Off-By lines as part of the ending of the initial
commit to avoid having these in the middle of the log when the
changelog part is injected after.

This is particularly usefull with:

 $ git gcc-commit-mklog --amend -s

that can be used to create the changelog and add the Signed-Off-By line.

Also applies most of the shellcheck suggestions on the
prepare-commit-msg hook.

contrib/ChangeLog:

	* mklog.py: Leave SOB lines after changelog.
	* prepare-commit-msg: Apply most shellcheck suggestions.

Signed-off-by: Marc Poulhiès <dkm@kataplop.net>
---
Previous version was missing the ChangeLog.

This command is used in particular during the dev of the frontend
for the Rust language (see r13-7099-g4b25fc15b925f8 as an example
of a SoB ending in the middle of the commit message).

Ok for master?

 contrib/mklog.py           | 34 +++++++++++++++++++++++++++++-----
 contrib/prepare-commit-msg | 20 ++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/contrib/mklog.py b/contrib/mklog.py
index 777212c98d7..e5cc69e0d0a 100755
--- a/contrib/mklog.py
+++ b/contrib/mklog.py
@@ -41,7 +41,34 @@ from unidiff import PatchSet
 
 LINE_LIMIT = 100
 TAB_WIDTH = 8
-CO_AUTHORED_BY_PREFIX = 'co-authored-by: '
+
+# Initial commit:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  | This is the "start"
+#   | This is some text explaining the commit.         |
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+#
+# Results in:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  |
+#   | This is some text explaining the commit.         | This is the "start"
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | gcc/rust/ChangeLog:                              |
+#   |                                                  | This is the generated
+#   |         * some_file (bla):                       | ChangeLog part
+#   |         (foo):                                   |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+
+# this regex matches the first line of the "end" in the initial commit message
+FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by|co-authored-by|#): ')
 
 pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)')
 prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)')
@@ -330,10 +357,7 @@ def update_copyright(data):
 
 
 def skip_line_in_changelog(line):
-    if line.lower().startswith(CO_AUTHORED_BY_PREFIX) or line.startswith('#'):
-        return False
-    return True
-
+    return FIRST_LINE_OF_END_RE.match(line) == None
 
 if __name__ == '__main__':
     extra_args = os.getenv('GCC_MKLOG_ARGS')
diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 48c9dad3c6f..1e94706ba40 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -32,11 +32,11 @@ 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 "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
+if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" = template ]; then
     # No source or "template" means new commit.
     cmd="diff --cached"
 
-elif [ $COMMIT_SOURCE = message ]; then
+elif [ "$COMMIT_SOURCE" = message ]; then
     # "message" means -m; assume a new commit if there are any changes staged.
     if ! git diff --cached --quiet; then
 	cmd="diff --cached"
@@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE = message ]; then
 	cmd="diff --cached HEAD^"
     fi
 
-elif [ $COMMIT_SOURCE = commit ]; then
+elif [ "$COMMIT_SOURCE" = commit ]; then
     # The message of an existing commit.  If it's HEAD, assume --amend;
     # otherwise, assume a new commit with -C.
-    if [ $SHA1 = HEAD ]; then
+    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
+	    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
+		rm "$f"
 		exit 0
 	    fi
-	    rm $f
+	    rm "$f"
 	fi
     else
 	cmd="diff --cached"
@@ -72,7 +72,7 @@ fi
 
 # Save diff to a file if requested.
 DIFF_FILE=$(git config gcc-config.diff-file)
-if ! [ -z "$DIFF_FILE" ]; then
+if [ -n "$DIFF_FILE" ]; then
     tee="tee $DIFF_FILE"
 else
     tee="cat"
-- 
2.40.1


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

* Re: [PATCH v2] mklog: handle Signed-Off-By, minor cleanup
  2023-07-21 11:08 [PATCH v2] mklog: handle Signed-Off-By, minor cleanup Marc Poulhiès
@ 2023-08-22  6:56 ` Richard Sandiford
  2023-09-03 18:47   ` [PATCH v3] mklog: handle Signed-off-by, " Marc Poulhiès
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2023-08-22  6:56 UTC (permalink / raw)
  To: Marc Poulhiès via Gcc-patches; +Cc: Marc Poulhiès

Marc Poulhiès via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Consider Signed-Off-By lines as part of the ending of the initial
> commit to avoid having these in the middle of the log when the
> changelog part is injected after.
>
> This is particularly usefull with:
>
>  $ git gcc-commit-mklog --amend -s
>
> that can be used to create the changelog and add the Signed-Off-By line.
>
> Also applies most of the shellcheck suggestions on the
> prepare-commit-msg hook.
>
> contrib/ChangeLog:
>
> 	* mklog.py: Leave SOB lines after changelog.
> 	* prepare-commit-msg: Apply most shellcheck suggestions.
>
> Signed-off-by: Marc Poulhiès <dkm@kataplop.net>
> ---
> Previous version was missing the ChangeLog.
>
> This command is used in particular during the dev of the frontend
> for the Rust language (see r13-7099-g4b25fc15b925f8 as an example
> of a SoB ending in the middle of the commit message).
>
> Ok for master?
>
>  contrib/mklog.py           | 34 +++++++++++++++++++++++++++++-----
>  contrib/prepare-commit-msg | 20 ++++++++++----------
>  2 files changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/contrib/mklog.py b/contrib/mklog.py
> index 777212c98d7..e5cc69e0d0a 100755
> --- a/contrib/mklog.py
> +++ b/contrib/mklog.py
> @@ -41,7 +41,34 @@ from unidiff import PatchSet
>  
>  LINE_LIMIT = 100
>  TAB_WIDTH = 8
> -CO_AUTHORED_BY_PREFIX = 'co-authored-by: '
> +
> +# Initial commit:
> +#   +--------------------------------------------------+
> +#   | gccrs: Some title                                |
> +#   |                                                  | This is the "start"
> +#   | This is some text explaining the commit.         |
> +#   | There can be several lines.                      |
> +#   |                                                  |<------------------->
> +#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
> +#   +--------------------------------------------------+
> +#
> +# Results in:
> +#   +--------------------------------------------------+
> +#   | gccrs: Some title                                |
> +#   |                                                  |
> +#   | This is some text explaining the commit.         | This is the "start"
> +#   | There can be several lines.                      |
> +#   |                                                  |<------------------->
> +#   | gcc/rust/ChangeLog:                              |
> +#   |                                                  | This is the generated
> +#   |         * some_file (bla):                       | ChangeLog part
> +#   |         (foo):                                   |
> +#   |                                                  |<------------------->
> +#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
> +#   +--------------------------------------------------+
> +
> +# this regex matches the first line of the "end" in the initial commit message
> +FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by|co-authored-by|#): ')

The current code only requires an initial "#", rather than an initial "#: ".
Is that a deliberate change?

The patch LGTM apart from that.

Thanks,
Richard

>  pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)')
>  prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)')
> @@ -330,10 +357,7 @@ def update_copyright(data):
>  
>  
>  def skip_line_in_changelog(line):
> -    if line.lower().startswith(CO_AUTHORED_BY_PREFIX) or line.startswith('#'):
> -        return False
> -    return True
> -
> +    return FIRST_LINE_OF_END_RE.match(line) == None
>  
>  if __name__ == '__main__':
>      extra_args = os.getenv('GCC_MKLOG_ARGS')
> diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
> index 48c9dad3c6f..1e94706ba40 100755
> --- a/contrib/prepare-commit-msg
> +++ b/contrib/prepare-commit-msg
> @@ -32,11 +32,11 @@ 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 "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
> +if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" = template ]; then
>      # No source or "template" means new commit.
>      cmd="diff --cached"
>  
> -elif [ $COMMIT_SOURCE = message ]; then
> +elif [ "$COMMIT_SOURCE" = message ]; then
>      # "message" means -m; assume a new commit if there are any changes staged.
>      if ! git diff --cached --quiet; then
>  	cmd="diff --cached"
> @@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE = message ]; then
>  	cmd="diff --cached HEAD^"
>      fi
>  
> -elif [ $COMMIT_SOURCE = commit ]; then
> +elif [ "$COMMIT_SOURCE" = commit ]; then
>      # The message of an existing commit.  If it's HEAD, assume --amend;
>      # otherwise, assume a new commit with -C.
> -    if [ $SHA1 = HEAD ]; then
> +    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
> +	    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
> +		rm "$f"
>  		exit 0
>  	    fi
> -	    rm $f
> +	    rm "$f"
>  	fi
>      else
>  	cmd="diff --cached"
> @@ -72,7 +72,7 @@ fi
>  
>  # Save diff to a file if requested.
>  DIFF_FILE=$(git config gcc-config.diff-file)
> -if ! [ -z "$DIFF_FILE" ]; then
> +if [ -n "$DIFF_FILE" ]; then
>      tee="tee $DIFF_FILE"
>  else
>      tee="cat"

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

* [PATCH v3] mklog: handle Signed-off-by, minor cleanup
  2023-08-22  6:56 ` Richard Sandiford
@ 2023-09-03 18:47   ` Marc Poulhiès
  2023-09-04 18:38     ` Richard Sandiford
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Poulhiès @ 2023-09-03 18:47 UTC (permalink / raw)
  To: gcc-patches, richard.sandiford; +Cc: Marc Poulhiès

Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> +# this regex matches the first line of the "end" in the initial commit message
>> +FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by|co-authored-by|#): ')
>
> The current code only requires an initial "#", rather than an initial "#: ".
> Is that a deliberate change?
>
> The patch LGTM apart from that.

Hello Richard,

Thanks for the review and sorry for the delayed answer as I was away the
past weeks. This issue was catched early this month
(https://github.com/Rust-GCC/gccrs/pull/2504), but I didn't want to send
something here before leaving. Here's a fixed patched.

Ok for master?

Thanks,
Marc

---
 contrib/mklog.py           | 34 +++++++++++++++++++++++++++++-----
 contrib/prepare-commit-msg | 20 ++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/contrib/mklog.py b/contrib/mklog.py
index 26230b9b4f2..496780883fb 100755
--- a/contrib/mklog.py
+++ b/contrib/mklog.py
@@ -41,7 +41,34 @@ from unidiff import PatchSet
 
 LINE_LIMIT = 100
 TAB_WIDTH = 8
-CO_AUTHORED_BY_PREFIX = 'co-authored-by: '
+
+# Initial commit:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  | This is the "start"
+#   | This is some text explaining the commit.         |
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+#
+# Results in:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  |
+#   | This is some text explaining the commit.         | This is the "start"
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | gcc/rust/ChangeLog:                              |
+#   |                                                  | This is the generated
+#   |         * some_file (bla):                       | ChangeLog part
+#   |         (foo):                                   |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+
+# this regex matches the first line of the "end" in the initial commit message
+FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by:|co-authored-by:|#) ')
 
 pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)')
 prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)')
@@ -330,10 +357,7 @@ def update_copyright(data):
 
 
 def skip_line_in_changelog(line):
-    if line.lower().startswith(CO_AUTHORED_BY_PREFIX) or line.startswith('#'):
-        return False
-    return True
-
+    return FIRST_LINE_OF_END_RE.match(line) == None
 
 if __name__ == '__main__':
     extra_args = os.getenv('GCC_MKLOG_ARGS')
diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 48c9dad3c6f..1e94706ba40 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -32,11 +32,11 @@ 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 "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
+if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" = template ]; then
     # No source or "template" means new commit.
     cmd="diff --cached"
 
-elif [ $COMMIT_SOURCE = message ]; then
+elif [ "$COMMIT_SOURCE" = message ]; then
     # "message" means -m; assume a new commit if there are any changes staged.
     if ! git diff --cached --quiet; then
 	cmd="diff --cached"
@@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE = message ]; then
 	cmd="diff --cached HEAD^"
     fi
 
-elif [ $COMMIT_SOURCE = commit ]; then
+elif [ "$COMMIT_SOURCE" = commit ]; then
     # The message of an existing commit.  If it's HEAD, assume --amend;
     # otherwise, assume a new commit with -C.
-    if [ $SHA1 = HEAD ]; then
+    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
+	    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
+		rm "$f"
 		exit 0
 	    fi
-	    rm $f
+	    rm "$f"
 	fi
     else
 	cmd="diff --cached"
@@ -72,7 +72,7 @@ fi
 
 # Save diff to a file if requested.
 DIFF_FILE=$(git config gcc-config.diff-file)
-if ! [ -z "$DIFF_FILE" ]; then
+if [ -n "$DIFF_FILE" ]; then
     tee="tee $DIFF_FILE"
 else
     tee="cat"
-- 
2.40.1


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

* Re: [PATCH v3] mklog: handle Signed-off-by, minor cleanup
  2023-09-03 18:47   ` [PATCH v3] mklog: handle Signed-off-by, " Marc Poulhiès
@ 2023-09-04 18:38     ` Richard Sandiford
  2023-09-04 19:45       ` Marc
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Sandiford @ 2023-09-04 18:38 UTC (permalink / raw)
  To: Marc Poulhiès via Gcc-patches; +Cc: Marc Poulhiès

Marc Poulhiès via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>>> +# this regex matches the first line of the "end" in the initial commit message
>>> +FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by|co-authored-by|#): ')
>>
>> The current code only requires an initial "#", rather than an initial "#: ".
>> Is that a deliberate change?
>>
>> The patch LGTM apart from that.
>
> Hello Richard,
>
> Thanks for the review and sorry for the delayed answer as I was away the
> past weeks. This issue was catched early this month
> (https://github.com/Rust-GCC/gccrs/pull/2504), but I didn't want to send
> something here before leaving. Here's a fixed patched.
>
> Ok for master?
>
> Thanks,
> Marc
>
> ---
>  contrib/mklog.py           | 34 +++++++++++++++++++++++++++++-----
>  contrib/prepare-commit-msg | 20 ++++++++++----------
>  2 files changed, 39 insertions(+), 15 deletions(-)
>
> diff --git a/contrib/mklog.py b/contrib/mklog.py
> index 26230b9b4f2..496780883fb 100755
> --- a/contrib/mklog.py
> +++ b/contrib/mklog.py
> @@ -41,7 +41,34 @@ from unidiff import PatchSet
>  
>  LINE_LIMIT = 100
>  TAB_WIDTH = 8
> -CO_AUTHORED_BY_PREFIX = 'co-authored-by: '
> +
> +# Initial commit:
> +#   +--------------------------------------------------+
> +#   | gccrs: Some title                                |
> +#   |                                                  | This is the "start"
> +#   | This is some text explaining the commit.         |
> +#   | There can be several lines.                      |
> +#   |                                                  |<------------------->
> +#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
> +#   +--------------------------------------------------+
> +#
> +# Results in:
> +#   +--------------------------------------------------+
> +#   | gccrs: Some title                                |
> +#   |                                                  |
> +#   | This is some text explaining the commit.         | This is the "start"
> +#   | There can be several lines.                      |
> +#   |                                                  |<------------------->
> +#   | gcc/rust/ChangeLog:                              |
> +#   |                                                  | This is the generated
> +#   |         * some_file (bla):                       | ChangeLog part
> +#   |         (foo):                                   |
> +#   |                                                  |<------------------->
> +#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
> +#   +--------------------------------------------------+
> +
> +# this regex matches the first line of the "end" in the initial commit message
> +FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by:|co-authored-by:|#) ')

Personally I think it would be safer to drop the final space in the regexp.

OK with that change if you agree.

Thanks,
Richard

>  
>  pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)')
>  prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)')
> @@ -330,10 +357,7 @@ def update_copyright(data):
>  
>  
>  def skip_line_in_changelog(line):
> -    if line.lower().startswith(CO_AUTHORED_BY_PREFIX) or line.startswith('#'):
> -        return False
> -    return True
> -
> +    return FIRST_LINE_OF_END_RE.match(line) == None
>  
>  if __name__ == '__main__':
>      extra_args = os.getenv('GCC_MKLOG_ARGS')
> diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
> index 48c9dad3c6f..1e94706ba40 100755
> --- a/contrib/prepare-commit-msg
> +++ b/contrib/prepare-commit-msg
> @@ -32,11 +32,11 @@ 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 "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
> +if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" = template ]; then
>      # No source or "template" means new commit.
>      cmd="diff --cached"
>  
> -elif [ $COMMIT_SOURCE = message ]; then
> +elif [ "$COMMIT_SOURCE" = message ]; then
>      # "message" means -m; assume a new commit if there are any changes staged.
>      if ! git diff --cached --quiet; then
>  	cmd="diff --cached"
> @@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE = message ]; then
>  	cmd="diff --cached HEAD^"
>      fi
>  
> -elif [ $COMMIT_SOURCE = commit ]; then
> +elif [ "$COMMIT_SOURCE" = commit ]; then
>      # The message of an existing commit.  If it's HEAD, assume --amend;
>      # otherwise, assume a new commit with -C.
> -    if [ $SHA1 = HEAD ]; then
> +    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
> +	    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
> +		rm "$f"
>  		exit 0
>  	    fi
> -	    rm $f
> +	    rm "$f"
>  	fi
>      else
>  	cmd="diff --cached"
> @@ -72,7 +72,7 @@ fi
>  
>  # Save diff to a file if requested.
>  DIFF_FILE=$(git config gcc-config.diff-file)
> -if ! [ -z "$DIFF_FILE" ]; then
> +if [ -n "$DIFF_FILE" ]; then
>      tee="tee $DIFF_FILE"
>  else
>      tee="cat"

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

* Re: [PATCH v3] mklog: handle Signed-off-by, minor cleanup
  2023-09-04 18:38     ` Richard Sandiford
@ 2023-09-04 19:45       ` Marc
  0 siblings, 0 replies; 6+ messages in thread
From: Marc @ 2023-09-04 19:45 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches


Richard Sandiford <richard.sandiford@arm.com> writes:

>> +# this regex matches the first line of the "end" in the initial commit message
>> +FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by:|co-authored-by:|#) ')
>
> Personally I think it would be safer to drop the final space in the regexp.
>
> OK with that change if you agree.

Hello,

You're correct. I'll commit the adjusted change.

Thanks,
Marc

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

* [PATCH v3] mklog: handle Signed-off-by, minor cleanup
@ 2023-08-02  9:06 Marc Poulhiès
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Poulhiès @ 2023-08-02  9:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: Marc Poulhiès

Consider Signed-off-by lines as part of the ending of the initial
commit to avoid having these in the middle of the log when the
changelog part is injected after.

This is particularly usefull with:

 $ git gcc-commit-mklog --amend -s

that can be used to create the changelog and add the Signed-off-by line.

Also applies most of the shellcheck suggestions on the
prepare-commit-msg hook.

contrib/ChangeLog:

	* mklog.py: Leave SOB lines after changelog.
	* prepare-commit-msg: Apply most shellcheck suggestions.

Signed-off-by: Marc Poulhiès <dkm@kataplop.net>
---
Found a small bug in the regex for comments, now fixed.

This command is used in particular during the dev of the frontend
for the Rust language (see r13-7099-g4b25fc15b925f8 as an example
of a SoB ending in the middle of the commit message).

Ok for master?

 contrib/mklog.py           | 34 +++++++++++++++++++++++++++++-----
 contrib/prepare-commit-msg | 20 ++++++++++----------
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/contrib/mklog.py b/contrib/mklog.py
index 26230b9b4f2..496780883fb 100755
--- a/contrib/mklog.py
+++ b/contrib/mklog.py
@@ -41,7 +41,34 @@ from unidiff import PatchSet
 
 LINE_LIMIT = 100
 TAB_WIDTH = 8
-CO_AUTHORED_BY_PREFIX = 'co-authored-by: '
+
+# Initial commit:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  | This is the "start"
+#   | This is some text explaining the commit.         |
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+#
+# Results in:
+#   +--------------------------------------------------+
+#   | gccrs: Some title                                |
+#   |                                                  |
+#   | This is some text explaining the commit.         | This is the "start"
+#   | There can be several lines.                      |
+#   |                                                  |<------------------->
+#   | gcc/rust/ChangeLog:                              |
+#   |                                                  | This is the generated
+#   |         * some_file (bla):                       | ChangeLog part
+#   |         (foo):                                   |
+#   |                                                  |<------------------->
+#   | Signed-off-by: My Name <my@mail.com>             | This is the "end"
+#   +--------------------------------------------------+
+
+# this regex matches the first line of the "end" in the initial commit message
+FIRST_LINE_OF_END_RE = re.compile('(?i)^(signed-off-by:|co-authored-by:|#) ')
 
 pr_regex = re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?P<pr>PR [a-z+-]+\/[0-9]+)')
 prnum_regex = re.compile(r'PR (?P<comp>[a-z+-]+)/(?P<num>[0-9]+)')
@@ -330,10 +357,7 @@ def update_copyright(data):
 
 
 def skip_line_in_changelog(line):
-    if line.lower().startswith(CO_AUTHORED_BY_PREFIX) or line.startswith('#'):
-        return False
-    return True
-
+    return FIRST_LINE_OF_END_RE.match(line) == None
 
 if __name__ == '__main__':
     extra_args = os.getenv('GCC_MKLOG_ARGS')
diff --git a/contrib/prepare-commit-msg b/contrib/prepare-commit-msg
index 48c9dad3c6f..1e94706ba40 100755
--- a/contrib/prepare-commit-msg
+++ b/contrib/prepare-commit-msg
@@ -32,11 +32,11 @@ 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 "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE = template ]; then
+if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" = template ]; then
     # No source or "template" means new commit.
     cmd="diff --cached"
 
-elif [ $COMMIT_SOURCE = message ]; then
+elif [ "$COMMIT_SOURCE" = message ]; then
     # "message" means -m; assume a new commit if there are any changes staged.
     if ! git diff --cached --quiet; then
 	cmd="diff --cached"
@@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE = message ]; then
 	cmd="diff --cached HEAD^"
     fi
 
-elif [ $COMMIT_SOURCE = commit ]; then
+elif [ "$COMMIT_SOURCE" = commit ]; then
     # The message of an existing commit.  If it's HEAD, assume --amend;
     # otherwise, assume a new commit with -C.
-    if [ $SHA1 = HEAD ]; then
+    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
+	    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
+		rm "$f"
 		exit 0
 	    fi
-	    rm $f
+	    rm "$f"
 	fi
     else
 	cmd="diff --cached"
@@ -72,7 +72,7 @@ fi
 
 # Save diff to a file if requested.
 DIFF_FILE=$(git config gcc-config.diff-file)
-if ! [ -z "$DIFF_FILE" ]; then
+if [ -n "$DIFF_FILE" ]; then
     tee="tee $DIFF_FILE"
 else
     tee="cat"
-- 
2.40.1


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

end of thread, other threads:[~2023-09-04 19:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-21 11:08 [PATCH v2] mklog: handle Signed-Off-By, minor cleanup Marc Poulhiès
2023-08-22  6:56 ` Richard Sandiford
2023-09-03 18:47   ` [PATCH v3] mklog: handle Signed-off-by, " Marc Poulhiès
2023-09-04 18:38     ` Richard Sandiford
2023-09-04 19:45       ` Marc
2023-08-02  9:06 Marc Poulhiès

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).