[Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Tobias Burnus]

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

That's based on the fail https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
and on the discussion on IRC.

The problem in for the cron job was that r14-3661-g084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
referenced a commit that did not exist.

This was temporarily fixed by Jakub, but it makes sense to detect this
in the pre-commit hook.


With the patch,
   contrib/gcc-changelog/git_email.py 0001-Revert-libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch
now prints:
OK
Warnings:
Cannot obtain info about reverted commit '46c2e94ca66ed9991c45a6ba6204ed02869efc39'

while
   contrib/gcc-changelog/git_check_commit.py 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
now fails with:
   Checking 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002: FAILED
   ERR: Cannot find to-be-reverted commit: "46c2e94ca66ed9991c45a6ba6204ed02869efc39"

(check_email.py always shows the warning, git_check_commit.py only with '-v')

Notes:
- I am not sure whether a sensible testcase can be added - and, hence, I have not added one.
- I have run "pytest-3' but my python is too old and thus might miss some checks which
   newer pytest/flake8 will find. But at least it did pass here.
- I have not tested the cherry-pick + commit does not exist case,
   which now creates a warning as I did not quickly find a testcase.

Comments, remarks, suggestions, approval?

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: check-revert-commit.diff --]
[-- Type: text/x-patch, Size: 2921 bytes --]

contrib/gcc-changelog: Check whether revert-commit exists

contrib/ChangeLog:

	* gcc-changelog/git_commit.py (GitCommit.__init__):
	Handle commit_to_info_hook = None; otherwise, if None,
	regard it as error.
	(to_changelog_entries): Handle commit_to_info_hook = None;
	if info is None, create a warning for it.
	* gcc-changelog/git_email.py (GitEmail.__init__):
	call super() with commit_to_info_hook=None instead
	of a lamda function.

 contrib/gcc-changelog/git_commit.py | 14 +++++++++-----
 contrib/gcc-changelog/git_email.py  |  3 +--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
index 4f3131021f2..4d024026f2b 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -329,11 +329,13 @@ class GitCommit:
                 self.revert_commit = m.group('hash')
                 break
         if self.revert_commit:
+            # The following happens for get_email.py:
+            if not self.commit_to_info_hook:
+                return
             self.info = self.commit_to_info_hook(self.revert_commit)
-
-        # The following happens for get_email.py:
-        if not self.info:
-            return
+            if not self.info:
+                self.errors.append(Error('Cannot find to-be-reverted commit', self.revert_commit))
+                return
 
         self.check_commit_email()
 
@@ -796,12 +798,14 @@ class GitCommit:
                 orig_date = self.original_info.date
                 current_timestamp = orig_date.strftime(DATE_FORMAT)
             elif self.cherry_pick_commit:
-                info = self.commit_to_info_hook(self.cherry_pick_commit)
+                info = (self.commit_to_info_hook
+                        and self.commit_to_info_hook(self.cherry_pick_commit))
                 # it can happen that it is a cherry-pick for a different
                 # repository
                 if info:
                     timestamp = info.date.strftime(DATE_FORMAT)
                 else:
+                    self.warnings.append(f"Cherry-picked commit not found: '{self.cherry_pick_commit}'")
                     timestamp = current_timestamp
             elif not timestamp or use_commit_ts:
                 timestamp = current_timestamp
diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
index 49f41f2ec99..93808dfabb6 100755
--- a/contrib/gcc-changelog/git_email.py
+++ b/contrib/gcc-changelog/git_email.py
@@ -89,8 +89,7 @@ class GitEmail(GitCommit):
                 t = 'M'
             modified_files.append((target if t != 'D' else source, t))
         git_info = GitInfo(None, date, author, message, modified_files)
-        super().__init__(git_info,
-                         commit_to_info_hook=lambda x: None)
+        super().__init__(git_info, commit_to_info_hook=None)
 
 
 def show_help():

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Tobias Burnus]

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

Attached an old patch. See attached patch for the current one.

Difference is one line: the warning that is shown in the example output
below.

On 05.09.23 16:37, Tobias Burnus wrote:
> That's based on the fail
> https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
> and on the discussion on IRC.
>
> The problem in for the cron job was that
> r14-3661-g084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
> referenced a commit that did not exist.
>
> This was temporarily fixed by Jakub, but it makes sense to detect this
> in the pre-commit hook.
>
>
> With the patch,
>   contrib/gcc-changelog/git_email.py
> 0001-Revert-libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch
> now prints:
> OK
> Warnings:
> Cannot obtain info about reverted commit
> '46c2e94ca66ed9991c45a6ba6204ed02869efc39'
>
> while
>   contrib/gcc-changelog/git_check_commit.py
> 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
> now fails with:
>   Checking 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002: FAILED
>   ERR: Cannot find to-be-reverted commit:
> "46c2e94ca66ed9991c45a6ba6204ed02869efc39"
>
> (check_email.py always shows the warning, git_check_commit.py only
> with '-v')
>
> Notes:
> - I am not sure whether a sensible testcase can be added - and, hence,
> I have not added one.
> - I have run "pytest-3' but my python is too old and thus might miss
> some checks which
>   newer pytest/flake8 will find. But at least it did pass here.
> - I have not tested the cherry-pick + commit does not exist case,
>   which now creates a warning as I did not quickly find a testcase.
>
> Comments, remarks, suggestions, approval?
Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: check-revert-commit.diff --]
[-- Type: text/x-patch, Size: 3027 bytes --]

contrib/gcc-changelog: Check whether revert-commit exists

contrib/ChangeLog:

	* gcc-changelog/git_commit.py (GitCommit.__init__):
	Handle commit_to_info_hook = None; otherwise, if None,
	regard it as error.
	(to_changelog_entries): Handle commit_to_info_hook = None;
	if info is None, create a warning for it.
	* gcc-changelog/git_email.py (GitEmail.__init__):
	call super() with commit_to_info_hook=None instead
	of a lamda function.

 contrib/gcc-changelog/git_commit.py | 14 +++++++++-----
 contrib/gcc-changelog/git_email.py  |  3 +--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
index 4f3131021f2..99e40c3c0d4 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -329,11 +329,14 @@ class GitCommit:
                 self.revert_commit = m.group('hash')
                 break
         if self.revert_commit:
+            # The following happens for get_email.py:
+            if not self.commit_to_info_hook:
+                self.warnings.append(f"Cannot obtain info about reverted commit '{self.revert_commit}'")
+                return
             self.info = self.commit_to_info_hook(self.revert_commit)
-
-        # The following happens for get_email.py:
-        if not self.info:
-            return
+            if not self.info:
+                self.errors.append(Error('Cannot find to-be-reverted commit', self.revert_commit))
+                return
 
         self.check_commit_email()
 
@@ -796,12 +799,14 @@ class GitCommit:
                 orig_date = self.original_info.date
                 current_timestamp = orig_date.strftime(DATE_FORMAT)
             elif self.cherry_pick_commit:
-                info = self.commit_to_info_hook(self.cherry_pick_commit)
+                info = (self.commit_to_info_hook
+                        and self.commit_to_info_hook(self.cherry_pick_commit))
                 # it can happen that it is a cherry-pick for a different
                 # repository
                 if info:
                     timestamp = info.date.strftime(DATE_FORMAT)
                 else:
+                    self.warnings.append(f"Cherry-picked commit not found: '{self.cherry_pick_commit}'")
                     timestamp = current_timestamp
             elif not timestamp or use_commit_ts:
                 timestamp = current_timestamp
diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
index 49f41f2ec99..93808dfabb6 100755
--- a/contrib/gcc-changelog/git_email.py
+++ b/contrib/gcc-changelog/git_email.py
@@ -89,8 +89,7 @@ class GitEmail(GitCommit):
                 t = 'M'
             modified_files.append((target if t != 'D' else source, t))
         git_info = GitInfo(None, date, author, message, modified_files)
-        super().__init__(git_info,
-                         commit_to_info_hook=lambda x: None)
+        super().__init__(git_info, commit_to_info_hook=None)
 
 
 def show_help():

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Arsen Arsenović]

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


Tobias Burnus <tobias@codesourcery.com> writes:

> Attached an old patch. See attached patch for the current one.
>
> Difference is one line: the warning that is shown in the example output
> below.

Python-wise, the changes seem fine.  Unsure if it does the right thing,
though, since I'm not familiar with the full script.
-- 
Arsen Arsenović

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 381 bytes --]

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Christophe Lyon]

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

Hi!

On Tue, 5 Sept 2023 at 16:38, Tobias Burnus <tobias@codesourcery.com> wrote:

> That's based on the fail
> https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
> and on the discussion on IRC.
>

Sorry I didn't notice the problem, nor the discussion on IRC, but I can see
that my commits created the problem, sorry for that.

I'm not sure how your patch would have prevented me from doing this?
What happened is that I had 3 patches on top of master
- HEAD: the one I wanted to push
- HEAD-1: revert of HEAD-2
- HEAD-2:  libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch

I had actually forgotten about HEAD-1 and HEAD-2, HEAD was unrelated to
those, so when I pushed, I pushed 3 commits while I thought there was only
one.
I did run contrib/gcc-changelog/git_check_commit.py (I realize I'm not sure
whether I used git_check_commit.py or git_commit.py), but only on HEAD
since I had forgotten about the other two.

Are these scripts executed by the commit hooks too? (and thus they would
now warn?)

Thanks,

Christophe


> The problem in for the cron job was that
> r14-3661-g084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
> referenced a commit that did not exist.
>
> This was temporarily fixed by Jakub, but it makes sense to detect this
> in the pre-commit hook.
>
>
> With the patch,
>    contrib/gcc-changelog/git_email.py
> 0001-Revert-libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch
> now prints:
> OK
> Warnings:
> Cannot obtain info about reverted commit
> '46c2e94ca66ed9991c45a6ba6204ed02869efc39'
>
> while
>    contrib/gcc-changelog/git_check_commit.py
> 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002
> now fails with:
>    Checking 084a7cf9fb2d9cb98dfbe7d91602c840ec50b002: FAILED
>    ERR: Cannot find to-be-reverted commit:
> "46c2e94ca66ed9991c45a6ba6204ed02869efc39"
>
> (check_email.py always shows the warning, git_check_commit.py only with
> '-v')
>
> Notes:
> - I am not sure whether a sensible testcase can be added - and, hence, I
> have not added one.
> - I have run "pytest-3' but my python is too old and thus might miss some
> checks which
>    newer pytest/flake8 will find. But at least it did pass here.
> - I have not tested the cherry-pick + commit does not exist case,
>    which now creates a warning as I did not quickly find a testcase.
>
> Comments, remarks, suggestions, approval?
>
> Tobias
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201,
> 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer:
> Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München;
> Registergericht München, HRB 106955
>

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Tobias Burnus]

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

First: Hi all,

attached is an updated patch (v3) where I changed the wording
of the warning to distinguish technical reasons for not using
the commit data (→ git_email.py) from not-found lookups (git_check_commit.py)
to avoid confusion. (See also below.)

Any remarks/suggestions - related to this (e.g. wording improvements or ...)
or to the patch in general?

Hi Christoph,

On 07.09.23 10:20, Christophe Lyon wrote:

> On Tue, 5 Sept 2023 at 16:38, Tobias Burnus <tobias@codesourcery.com>
> wrote:
>
>     That's based on the fail
>     https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
>     and on the discussion on IRC.
>
>
> Sorry I didn't notice the problem, nor the discussion on IRC, but I
> can see that my commits created the problem, sorry for that.

Well, as the saying goes: you can't make an omelette without breaking eggs.

And there were three issues: The invalid reference, the missing check
for the former, and that it caused the nightly script to break.

While we cannot prevent human errors (nor KI ones) and have fixed/worked
around the latter, we can improve on the checking part :-)

> I'm not sure how your patch would have prevented me from doing this?

There are two checking scripts:

One for manual running on a file produced by 'git format-patch', which is
   ./contrib/gcc-changelog/git_email.py

Before it crashed for the revert, now it prints OK with the warning:
   Cannot obtain info about reverted commit '46c2e94ca66ed9991c45a6ba6204ed02869efc39'
re-reading it, I think the wording is wrong. It should be:
   Invoked script cannot obtain info about reverted commits such as '46c2e94ca66ed9991c45a6ba6204ed02869efc39'
as that's a generic limitation - we simply do not know whether
that commit exists or not. (Changed in the attached patch.)


And the other operates on the git repository:
   ./contrib/gcc-changelog/git_check_commit.py
the latter now fails with:
   ERR: Cannot find to-be-reverted commit: "46c2e94ca66ed9991c45a6ba6204ed02869efc39"

That script can be run manually on any commit (e.g. before the commit;
consider using the '-v' and/or '-p' options; see '--help').

But that script is also run on the GCC server as pre-commit hook.

Therefore, the latter would have rejected your commit with the error message
during "git push", permitting to fix the commit (git commit --amend) before
trying again, similar to missing '\t' in the changelog or ...

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: check-revert-commit-v3.diff --]
[-- Type: text/x-patch, Size: 3399 bytes --]

contrib/gcc-changelog: Check whether revert-commit exists

contrib/ChangeLog:

	* gcc-changelog/git_commit.py (GitCommit.__init__):
	Handle commit_to_info_hook = None; otherwise, if None,
	regard it as error.
	(to_changelog_entries): Handle commit_to_info_hook = None;
	if info is None, create a warning for it.
	* gcc-changelog/git_email.py (GitEmail.__init__):
	call super() with commit_to_info_hook=None instead
	of a lamda function.

 contrib/gcc-changelog/git_commit.py | 20 +++++++++++++++-----
 contrib/gcc-changelog/git_email.py  |  3 +--
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
index 4f3131021f2..4f1bd4d7293 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -329,11 +329,15 @@ class GitCommit:
                 self.revert_commit = m.group('hash')
                 break
         if self.revert_commit:
+            # The following happens for get_email.py:
+            if not self.commit_to_info_hook:
+                self.warnings.append(f"Invoked script can technically not obtain info about "
+                                     f"reverted commits such as '{self.revert_commit}'")
+                return
             self.info = self.commit_to_info_hook(self.revert_commit)
-
-        # The following happens for get_email.py:
-        if not self.info:
-            return
+            if not self.info:
+                self.errors.append(Error('Cannot find to-be-reverted commit', self.revert_commit))
+                return
 
         self.check_commit_email()
 
@@ -796,12 +800,18 @@ class GitCommit:
                 orig_date = self.original_info.date
                 current_timestamp = orig_date.strftime(DATE_FORMAT)
             elif self.cherry_pick_commit:
-                info = self.commit_to_info_hook(self.cherry_pick_commit)
+                info = (self.commit_to_info_hook
+                        and self.commit_to_info_hook(self.cherry_pick_commit))
                 # it can happen that it is a cherry-pick for a different
                 # repository
                 if info:
                     timestamp = info.date.strftime(DATE_FORMAT)
                 else:
+                    if self.commit_to_info_hook:
+                        self.warnings.append(f"Cherry-picked commit not found: '{self.cherry_pick_commit}'")
+                    else:
+                        self.warnings.append(f"Invoked script can technically not obtain info about "
+                                             f"cherry-picked commits such as '{self.revert_commit}'")
                     timestamp = current_timestamp
             elif not timestamp or use_commit_ts:
                 timestamp = current_timestamp
diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
index 49f41f2ec99..93808dfabb6 100755
--- a/contrib/gcc-changelog/git_email.py
+++ b/contrib/gcc-changelog/git_email.py
@@ -89,8 +89,7 @@ class GitEmail(GitCommit):
                 t = 'M'
             modified_files.append((target if t != 'D' else source, t))
         git_info = GitInfo(None, date, author, message, modified_files)
-        super().__init__(git_info,
-                         commit_to_info_hook=lambda x: None)
+        super().__init__(git_info, commit_to_info_hook=None)
 
 
 def show_help():

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Jakub Jelinek]

On Thu, Sep 07, 2023 at 11:30:53AM +0200, Tobias Burnus wrote:
> contrib/gcc-changelog: Check whether revert-commit exists
> 
> contrib/ChangeLog:
> 
> 	* gcc-changelog/git_commit.py (GitCommit.__init__):
> 	Handle commit_to_info_hook = None; otherwise, if None,
> 	regard it as error.
> 	(to_changelog_entries): Handle commit_to_info_hook = None;
> 	if info is None, create a warning for it.
> 	* gcc-changelog/git_email.py (GitEmail.__init__):
> 	call super() with commit_to_info_hook=None instead
> 	of a lamda function.
> 
>  contrib/gcc-changelog/git_commit.py | 20 +++++++++++++++-----
>  contrib/gcc-changelog/git_email.py  |  3 +--
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
> index 4f3131021f2..4f1bd4d7293 100755
> --- a/contrib/gcc-changelog/git_commit.py
> +++ b/contrib/gcc-changelog/git_commit.py
> @@ -329,11 +329,15 @@ class GitCommit:
>                  self.revert_commit = m.group('hash')
>                  break
>          if self.revert_commit:
> +            # The following happens for get_email.py:
> +            if not self.commit_to_info_hook:
> +                self.warnings.append(f"Invoked script can technically not obtain info about "
> +                                     f"reverted commits such as '{self.revert_commit}'")

I think not should precede technically (or should we just drop technically)?

> +                        self.warnings.append(f"Invoked script can technically not obtain info about "
> +                                             f"cherry-picked commits such as '{self.revert_commit}'")

Likewise.

>                      timestamp = current_timestamp
>              elif not timestamp or use_commit_ts:
>                  timestamp = current_timestamp
> diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
> index 49f41f2ec99..93808dfabb6 100755
> --- a/contrib/gcc-changelog/git_email.py
> +++ b/contrib/gcc-changelog/git_email.py
> @@ -89,8 +89,7 @@ class GitEmail(GitCommit):
>                  t = 'M'
>              modified_files.append((target if t != 'D' else source, t))
>          git_info = GitInfo(None, date, author, message, modified_files)
> -        super().__init__(git_info,
> -                         commit_to_info_hook=lambda x: None)
> +        super().__init__(git_info, commit_to_info_hook=None)
>  
>  
>  def show_help():

Otherwise LGTM, but it would be good after committing it try to commit
reversion commit of some non-existent hash (willing to handle ChangeLog
manually again if it makes it through).

	Jakub

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Jakub Jelinek]

On Thu, Sep 07, 2023 at 10:20:19AM +0200, Christophe Lyon via Gcc-patches wrote:
> On Tue, 5 Sept 2023 at 16:38, Tobias Burnus <tobias@codesourcery.com> wrote:
> 
> > That's based on the fail
> > https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
> > and on the discussion on IRC.
> >
> 
> Sorry I didn't notice the problem, nor the discussion on IRC, but I can see
> that my commits created the problem, sorry for that.
> 
> I'm not sure how your patch would have prevented me from doing this?
> What happened is that I had 3 patches on top of master
> - HEAD: the one I wanted to push
> - HEAD-1: revert of HEAD-2

git reset HEAD^
instead of committing a revert would be better I think.

> - HEAD-2:  libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch
> 
> I had actually forgotten about HEAD-1 and HEAD-2, HEAD was unrelated to
> those, so when I pushed, I pushed 3 commits while I thought there was only
> one.
> I did run contrib/gcc-changelog/git_check_commit.py (I realize I'm not sure
> whether I used git_check_commit.py or git_commit.py), but only on HEAD
> since I had forgotten about the other two.

Could you please remove your
2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>

	PR libstdc++/111238
	* configure: Regenerate.
	* configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
	non-Canadian builds.
libstdc++-v3/ChangeLog entry if that commit is indeed not in (or add
a Revert: entry for it right after it if you think it needs to be in)?
That is a part I haven't done, my/Arsen's hacks to make the version update
get through basically ignored that revert commit.

ChangeLog files can be changed by commits which only touch ChangeLog files
and nothing else (ok, date stamp too, but please don't update that), no
ChangeLog in the message needs to be provided for such changes.

	Jakub

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Christophe Lyon]

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

On Thu, 7 Sept 2023 at 11:45, Jakub Jelinek <jakub@redhat.com> wrote:

> On Thu, Sep 07, 2023 at 10:20:19AM +0200, Christophe Lyon via Gcc-patches
> wrote:
> > On Tue, 5 Sept 2023 at 16:38, Tobias Burnus <tobias@codesourcery.com>
> wrote:
> >
> > > That's based on the fail
> > > https://gcc.gnu.org/pipermail/gccadmin/2023q3/020349.html
> > > and on the discussion on IRC.
> > >
> >
> > Sorry I didn't notice the problem, nor the discussion on IRC, but I can
> see
> > that my commits created the problem, sorry for that.
> >
> > I'm not sure how your patch would have prevented me from doing this?
> > What happened is that I had 3 patches on top of master
> > - HEAD: the one I wanted to push
> > - HEAD-1: revert of HEAD-2
>
> git reset HEAD^
> instead of committing a revert would be better I think.
>

The patch I reverted locally was still under discussion, so I wanted to
keep it around until we made a decision.
But yeah.....


> > - HEAD-2:  libstdc-Use-GLIBCXX_CHECK_LINKER_FEATURES-for.patch
> >
> > I had actually forgotten about HEAD-1 and HEAD-2, HEAD was unrelated to
> > those, so when I pushed, I pushed 3 commits while I thought there was
> only
> > one.
> > I did run contrib/gcc-changelog/git_check_commit.py (I realize I'm not
> sure
> > whether I used git_check_commit.py or git_commit.py), but only on HEAD
> > since I had forgotten about the other two.
>
> Could you please remove your
> 2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
>
>         PR libstdc++/111238
>         * configure: Regenerate.
>         * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
>         non-Canadian builds.
> libstdc++-v3/ChangeLog entry if that commit is indeed not in (or add
> a Revert: entry for it right after it if you think it needs to be in)?
> That is a part I haven't done, my/Arsen's hacks to make the version update
> get through basically ignored that revert commit.
>
> ChangeLog files can be changed by commits which only touch ChangeLog files
> and nothing else (ok, date stamp too, but please don't update that), no
> ChangeLog in the message needs to be provided for such changes.
>

Like so:
 commit d2bb261dbf282bbb258e1e5f17c1b6230327e076 (HEAD -> master)
Author: Christophe Lyon <christophe.lyon@linaro.org>
Date:   Fri Sep 8 08:13:32 2023 +0000

    Revert "libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds
(PR111238)"

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 8f7b01e0563..0c60149d7f6 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -4,6 +4,16 @@
        for freestanding.
        * configure: Regenerate.

+2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
+
+       Revert
+       2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
+
+       PR libstdc++/111238
+       * configure: Regenerate.
+       * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
+       non-Canadian builds.
+
 2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>

        PR libstdc++/111238

I inserted my "Revert"  ChangeLog entry between the entry I want to declare
reverted and Jonathan's more recent patch about GLIBCXX_ENABLE_BACKTRACE.
Is that OK for the commit hooks?


        Jakub
>
>

Re: [Patch] contrib/gcc-changelog: Check whether revert-commit exists

[Jakub Jelinek]

On Fri, Sep 08, 2023 at 10:25:42AM +0200, Christophe Lyon wrote:
>     Revert "libstdc++: Use GLIBCXX_CHECK_LINKER_FEATURES for cross-builds
> (PR111238)"
> 
> diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
> index 8f7b01e0563..0c60149d7f6 100644
> --- a/libstdc++-v3/ChangeLog
> +++ b/libstdc++-v3/ChangeLog
> @@ -4,6 +4,16 @@
>         for freestanding.
>         * configure: Regenerate.
> 
> +2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
> +
> +       Revert
> +       2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
> +
> +       PR libstdc++/111238
> +       * configure: Regenerate.
> +       * configure.ac: Call GLIBCXX_CHECK_LINKER_FEATURES in cross,
> +       non-Canadian builds.
> +
>  2023-09-04  Christophe Lyon  <christophe.lyon@linaro.org>
> 
>         PR libstdc++/111238
> 
> I inserted my "Revert"  ChangeLog entry between the entry I want to declare
> reverted and Jonathan's more recent patch about GLIBCXX_ENABLE_BACKTRACE.
> Is that OK for the commit hooks?

Yes, thanks.

	Jakub

[committed] Update contrib + libgomp ChangeLogs for failed reject-commit testing (was: [Patch] contrib/gcc-changelog: Check whether revert-commit exists)

[Tobias Burnus]

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

On 07.09.23 11:38, Jakub Jelinek wrote:
> On Thu, Sep 07, 2023 at 11:30:53AM +0200, Tobias Burnus wrote:
>> contrib/gcc-changelog: Check whether revert-commit exists
>> ...
> I think not should precede technically (or should we just drop technically)?

'technically' has been dropped (twice), based on the IRC discussion (in
the original commit and in a follow-up fixing commit as the re-commit
patch missed it).

Otherwise, the patch has been applied - but we did struggle with getting
it copied over to the right location on the server + getting it to work.

Last status: The check does work locally but did not for "git push"; in
principle, it should also work on the server but it failed when last
tested. While several directories on the server have now the new script,
it is unclear whether the (or all the) relevant one(s) have it and
whether the script itself is fine or not.

In any case, attached is the commit that rectifies the ChangeLog files
and also lists in the log-message part what the related commits did and
didn't do.

Committed as r14-3806-g478c37e7234530 - after the "Daily Bump", which
also required the same temporary work around as for the original issue.

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: committed.diff --]
[-- Type: text/x-patch, Size: 3033 bytes --]

commit 478c37e72345307fad0aa06e0dae133eae206b0c
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Fri Sep 8 12:02:02 2023 +0200

    Update contrib + libgomp ChangeLogs for failed reject-commit testing
    
    The following commit should have enabled checking for invalid revert hashes;
    it worked locally - but did work as pre-commit hook on sourceware
    as it wasn't copied to the hook directory:
    r14-3777-gff20bce9f58 contrib/gcc-changelog: Check whether revert-commit exists
    
    Hence, the following revert commit was wrongly applied:
    r14-3778-gfbbd9001e9b Revert "contrib/gcc-changelog: Check whether revert-commit exists"
    (In this commit: contrib/ChangeLog update for the revert.)
    
    r14-3779-g69e83181ebc contrib/gcc-changelog: Check whether revert-commit exists
    Re-applied the commit with a commit-log typo fixed but missing a late commit.
    
    r14-3780-g1b0934b7276 Revert "contrib/gcc-changelog: Check whether revert-commit exists"
    This commit still came through but re-instated the late wording fix in
    contrib/gcc-changelog/git_commit.py.
    (In this commit: contrib/ChangeLog update for the wording change.)
    
    r14-3781-gd22cd7745ff Revert: "Another revert test with a bogus hash"
    Another attempt to get a reject, but it still came through.
    It removed tailing whitespace in libgomp/target.c
    (In this commit: libgomp/ChangeLog was for the whitespace removal.)

diff --git a/contrib/ChangeLog b/contrib/ChangeLog
index e49bbe30446..403a095512c 100644
--- a/contrib/ChangeLog
+++ b/contrib/ChangeLog
@@ -1,3 +1,8 @@
+2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
+
+	* gcc-changelog/git_commit.py (GitCommit.__init__,
+	to_changelog_entries): Fix lost wording fix.
+
 2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
 
 	* gcc-changelog/git_commit.py (GitCommit.__init__):
@@ -9,6 +14,20 @@
 	call super() with commit_to_info_hook=None instead
 	of a lambda function.
 
+2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
+
+	Revert:
+	2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
+
+	* gcc-changelog/git_commit.py (GitCommit.__init__):
+	Handle commit_to_info_hook = None; otherwise, if None,
+	regard it as error.
+	(to_changelog_entries): Handle commit_to_info_hook = None;
+	if info is None, create a warning for it.
+	* gcc-changelog/git_email.py (GitEmail.__init__):
+	call super() with commit_to_info_hook=None instead
+	of a lamda function.
+
 2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
 
 	* gcc-changelog/git_commit.py (GitCommit.__init__):
diff --git a/libgomp/ChangeLog b/libgomp/ChangeLog
index dbd5e4fc4ee..fb96155394e 100644
--- a/libgomp/ChangeLog
+++ b/libgomp/ChangeLog
@@ -5,6 +5,10 @@
 	omp_get_default_allocator.
 	(OMP_ALLOCATOR): Fix ICV var name; add see-also references.
 
+2023-09-07  Tobias Burnus  <tobias@codesourcery.com>
+
+	* target.c (gomp_unload_device): Remove tailing whitespace.
+
 2023-09-04  Tobias Burnus  <tobias@codesourcery.com>
 	    Thomas Schwinge  <thomas@codesourcery.com>