From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id EB5933858CDB for ; Tue, 22 Aug 2023 06:56:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EB5933858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6EC5C2F4; Mon, 21 Aug 2023 23:57:37 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1BB453F64C; Mon, 21 Aug 2023 23:56:55 -0700 (PDT) From: Richard Sandiford To: Marc =?utf-8?Q?Poulhi=C3=A8s?= via Gcc-patches Mail-Followup-To: Marc =?utf-8?Q?Poulhi=C3=A8s?= via Gcc-patches ,Marc =?utf-8?Q?Poulhi=C3=A8s?= , richard.sandiford@arm.com Cc: Marc =?utf-8?Q?Poulhi=C3=A8s?= Subject: Re: [PATCH v2] mklog: handle Signed-Off-By, minor cleanup References: <20230721110813.3136547-1-dkm@kataplop.net> Date: Tue, 22 Aug 2023 07:56:54 +0100 In-Reply-To: <20230721110813.3136547-1-dkm@kataplop.net> ("Marc =?utf-8?Q?Poulhi=C3=A8s?= via Gcc-patches"'s message of "Fri, 21 Jul 2023 13:08:13 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-25.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_ASCII_DIVIDERS,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Marc Poulhi=C3=A8s via Gcc-patches 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=C3=A8s > --- > 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 >=20=20 > LINE_LIMIT =3D 100 > TAB_WIDTH =3D 8 > -CO_AUTHORED_BY_PREFIX =3D 'co-authored-by: ' > + > +# Initial commit: > +# +--------------------------------------------------+ > +# | gccrs: Some title | > +# | | This is the "st= art" > +# | This is some text explaining the commit. | > +# | There can be several lines. | > +# | |<---------------= ----> > +# | Signed-off-by: My Name | This is the "en= d" > +# +--------------------------------------------------+ > +# > +# Results in: > +# +--------------------------------------------------+ > +# | gccrs: Some title | > +# | | > +# | This is some text explaining the commit. | This is the "st= art" > +# | There can be several lines. | > +# | |<---------------= ----> > +# | gcc/rust/ChangeLog: | > +# | | This is the gen= erated > +# | * some_file (bla): | ChangeLog part > +# | (foo): | > +# | |<---------------= ----> > +# | Signed-off-by: My Name | This is the "en= d" > +# +--------------------------------------------------+ > + > +# this regex matches the first line of the "end" in the initial commit m= essage > +FIRST_LINE_OF_END_RE =3D 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 =3D re.compile(r'(\/(\/|\*)|[Cc*!])\s+(?PPR [a-z+-]+\/[0-9]= +)') > prnum_regex =3D re.compile(r'PR (?P[a-z+-]+)/(?P[0-9]+)') > @@ -330,10 +357,7 @@ def update_copyright(data): >=20=20 >=20=20 > 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) =3D=3D None >=20=20 > if __name__ =3D=3D '__main__': > extra_args =3D 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 >=20=20 > -if [ -z "$COMMIT_SOURCE" ] || [ $COMMIT_SOURCE =3D template ]; then > +if [ -z "$COMMIT_SOURCE" ] || [ "$COMMIT_SOURCE" =3D template ]; then > # No source or "template" means new commit. > cmd=3D"diff --cached" >=20=20 > -elif [ $COMMIT_SOURCE =3D message ]; then > +elif [ "$COMMIT_SOURCE" =3D message ]; then > # "message" means -m; assume a new commit if there are any changes s= taged. > if ! git diff --cached --quiet; then > cmd=3D"diff --cached" > @@ -44,23 +44,23 @@ elif [ $COMMIT_SOURCE =3D message ]; then > cmd=3D"diff --cached HEAD^" > fi >=20=20 > -elif [ $COMMIT_SOURCE =3D commit ]; then > +elif [ "$COMMIT_SOURCE" =3D commit ]; then > # The message of an existing commit. If it's HEAD, assume --amend; > # otherwise, assume a new commit with -C. > - if [ $SHA1 =3D HEAD ]; then > + if [ "$SHA1" =3D HEAD ]; then > cmd=3D"diff --cached HEAD^" > if [ "$(git config gcc-config.mklog-hook-type)" =3D "smart-amend" ]; th= en > # Check if the existing message still describes the staged changes. > f=3D$(mktemp /tmp/git-commit.XXXXXX) || exit 1 > - git log -1 --pretty=3Demail HEAD > $f > - printf '\n---\n\n' >> $f > - git $cmd >> $f > + git log -1 --pretty=3Demail 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=3D"diff --cached" > @@ -72,7 +72,7 @@ fi >=20=20 > # Save diff to a file if requested. > DIFF_FILE=3D$(git config gcc-config.diff-file) > -if ! [ -z "$DIFF_FILE" ]; then > +if [ -n "$DIFF_FILE" ]; then > tee=3D"tee $DIFF_FILE" > else > tee=3D"cat"