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 <m...@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 <m...@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"
