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 <tob...@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
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():
