* [PATCH 1/4] BaseTools/PatchCheck.py: Extract email check code to EmailAddressCheck
2020-01-02 15:25 [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Philippe Mathieu-Daudé
@ 2020-01-02 15:25 ` Philippe Mathieu-Daudé
2020-01-02 15:25 ` [PATCH 2/4] BaseTools/PatchCheck.py: Let EmailAddressCheck describe email checked Philippe Mathieu-Daudé
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-02 15:25 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Michael D Kinney, Philippe Mathieu-Daude,
Liming Gao, Jordan Justen
As we are going to reuse this code out of the CommitMessageCheck
class, extract it in a new class: EmailAddressCheck.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
BaseTools/Scripts/PatchCheck.py | 82 +++++++++++++++++++++------------
1 file changed, 53 insertions(+), 29 deletions(-)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 2a4e6f603e79..0e654345fcc8 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -22,6 +22,58 @@ class Verbose:
SILENT, ONELINE, NORMAL = range(3)
level = NORMAL
+class EmailAddressCheck:
+ """Checks an email address."""
+
+ def __init__(self, email):
+ self.ok = True
+
+ if email is None:
+ self.error('Email address is missing!')
+ return
+
+ self.check_email_address(email)
+
+ def error(self, *err):
+ if self.ok and Verbose.level > Verbose.ONELINE:
+ print('The email address is not valid:')
+ self.ok = False
+ if Verbose.level < Verbose.NORMAL:
+ return
+ count = 0
+ for line in err:
+ prefix = (' *', ' ')[count > 0]
+ print(prefix, line)
+ count += 1
+
+ email_re1 = re.compile(r'(?:\s*)(.*?)(\s*)<(.+)>\s*$',
+ re.MULTILINE|re.IGNORECASE)
+
+ def check_email_address(self, email):
+ email = email.strip()
+ mo = self.email_re1.match(email)
+ if mo is None:
+ self.error("Email format is invalid: " + email.strip())
+ return
+
+ name = mo.group(1).strip()
+ if name == '':
+ self.error("Name is not provided with email address: " +
+ email)
+ else:
+ quoted = len(name) > 2 and name[0] == '"' and name[-1] == '"'
+ if name.find(',') >= 0 and not quoted:
+ self.error('Add quotes (") around name with a comma: ' +
+ name)
+
+ if mo.group(2) == '':
+ self.error("There should be a space between the name and " +
+ "email address: " + email)
+
+ if mo.group(3).find(' ') >= 0:
+ self.error("The email address cannot contain a space: " +
+ mo.group(3))
+
class CommitMessageCheck:
"""Checks the contents of a git commit message."""
@@ -121,38 +173,10 @@ class CommitMessageCheck:
if s[2] != ' ':
self.error("There should be a space after '" + sig + ":'")
- self.check_email_address(s[3])
+ EmailAddressCheck(s[3])
return sigs
- email_re1 = re.compile(r'(?:\s*)(.*?)(\s*)<(.+)>\s*$',
- re.MULTILINE|re.IGNORECASE)
-
- def check_email_address(self, email):
- email = email.strip()
- mo = self.email_re1.match(email)
- if mo is None:
- self.error("Email format is invalid: " + email.strip())
- return
-
- name = mo.group(1).strip()
- if name == '':
- self.error("Name is not provided with email address: " +
- email)
- else:
- quoted = len(name) > 2 and name[0] == '"' and name[-1] == '"'
- if name.find(',') >= 0 and not quoted:
- self.error('Add quotes (") around name with a comma: ' +
- name)
-
- if mo.group(2) == '':
- self.error("There should be a space between the name and " +
- "email address: " + email)
-
- if mo.group(3).find(' ') >= 0:
- self.error("The email address cannot contain a space: " +
- mo.group(3))
-
def check_signed_off_by(self):
sob='Signed-off-by'
if self.msg.find(sob) < 0:
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] BaseTools/PatchCheck.py: Let EmailAddressCheck describe email checked
2020-01-02 15:25 [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Philippe Mathieu-Daudé
2020-01-02 15:25 ` [PATCH 1/4] BaseTools/PatchCheck.py: Extract email check code to EmailAddressCheck Philippe Mathieu-Daudé
@ 2020-01-02 15:25 ` Philippe Mathieu-Daudé
2020-01-02 15:25 ` [PATCH 3/4] BaseTools/PatchCheck.py: Check the patch author email address Philippe Mathieu-Daudé
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-02 15:25 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Michael D Kinney, Philippe Mathieu-Daude,
Liming Gao, Jordan Justen
We are checking different emails from the signature list. We are
going to check more. To be able to differency, add a description
field, so the error reported is clearer.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
BaseTools/Scripts/PatchCheck.py | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 0e654345fcc8..a0ff5ec0038a 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -25,18 +25,22 @@ class Verbose:
class EmailAddressCheck:
"""Checks an email address."""
- def __init__(self, email):
+ def __init__(self, email, description):
self.ok = True
if email is None:
self.error('Email address is missing!')
return
+ if description is None:
+ self.error('Email description is missing!')
+ return
+ self.description = "'" + description + "'"
self.check_email_address(email)
def error(self, *err):
if self.ok and Verbose.level > Verbose.ONELINE:
- print('The email address is not valid:')
+ print('The ' + self.description + ' email address is not valid:')
self.ok = False
if Verbose.level < Verbose.NORMAL:
return
@@ -173,7 +177,7 @@ class CommitMessageCheck:
if s[2] != ' ':
self.error("There should be a space after '" + sig + ":'")
- EmailAddressCheck(s[3])
+ EmailAddressCheck(s[3], sig)
return sigs
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] BaseTools/PatchCheck.py: Check the patch author email address
2020-01-02 15:25 [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Philippe Mathieu-Daudé
2020-01-02 15:25 ` [PATCH 1/4] BaseTools/PatchCheck.py: Extract email check code to EmailAddressCheck Philippe Mathieu-Daudé
2020-01-02 15:25 ` [PATCH 2/4] BaseTools/PatchCheck.py: Let EmailAddressCheck describe email checked Philippe Mathieu-Daudé
@ 2020-01-02 15:25 ` Philippe Mathieu-Daudé
2020-01-02 15:25 ` [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer " Philippe Mathieu-Daudé
2020-01-03 13:11 ` [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Laszlo Ersek
4 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-02 15:25 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Michael D Kinney, Philippe Mathieu-Daude,
Liming Gao, Jordan Justen
To avoid patches committed with incorrect email address,
use the EmailAddressCheck class on the author email too.
Example:
$ python BaseTools/Scripts/PatchCheck.py 1a04951309f
Checking git commit: 1a04951309f
The 'Author' email address is not valid:
* The email address cannot contain a space: /o=Intel/ou=External \
(FYDIBOHF25SPDLT)/cn=Recipients/cn=fe425ca7e5f4401abed22b904fe5d964
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
BaseTools/Scripts/PatchCheck.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index a0ff5ec0038a..f0e661bfd6e3 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -450,6 +450,9 @@ class CheckOnePatch:
self.patch = patch
self.find_patch_pieces()
+ email_check = EmailAddressCheck(self.author_email, 'Author')
+ email_ok = email_check.ok
+
msg_check = CommitMessageCheck(self.commit_subject, self.commit_msg)
msg_ok = msg_check.ok
@@ -458,7 +461,7 @@ class CheckOnePatch:
diff_check = GitDiffCheck(self.diff)
diff_ok = diff_check.ok
- self.ok = msg_ok and diff_ok
+ self.ok = email_ok and msg_ok and diff_ok
if Verbose.level == Verbose.ONELINE:
if self.ok:
@@ -536,6 +539,8 @@ class CheckOnePatch:
self.commit_subject = self.commit_subject.replace('\n', '')
self.commit_subject = self.subject_prefix_re.sub('', self.commit_subject, 1)
+ self.author_email = pmail['from']
+
class CheckGitCommits:
"""Reads patches from git based on the specified git revision range.
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer email address
2020-01-02 15:25 [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-01-02 15:25 ` [PATCH 3/4] BaseTools/PatchCheck.py: Check the patch author email address Philippe Mathieu-Daudé
@ 2020-01-02 15:25 ` Philippe Mathieu-Daudé
2020-01-03 22:59 ` Jordan Justen
2020-01-03 13:11 ` [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Laszlo Ersek
4 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-02 15:25 UTC (permalink / raw)
To: devel
Cc: Laszlo Ersek, Michael D Kinney, Philippe Mathieu-Daude,
Liming Gao, Jordan Justen
To avoid patches committed with incorrect email address,
use the EmailAddressCheck class on the committer email too.
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
---
RFC because I haven't checked --pretty="%cn <%ce>" works on Windows shell.
BaseTools/Scripts/PatchCheck.py | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index f0e661bfd6e3..3baeb3de7ba2 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -560,6 +560,8 @@ class CheckGitCommits:
else:
blank_line = True
print('Checking git commit:', commit)
+ email = self.read_committer_email_address_from_git(commit)
+ self.ok &= EmailAddressCheck(email, 'Committer').ok
patch = self.read_patch_from_git(commit)
self.ok &= CheckOnePatch(commit, patch).ok
if not commits:
@@ -578,6 +580,10 @@ class CheckGitCommits:
# Run git to get the commit patch
return self.run_git('show', '--pretty=email', '--no-textconv', commit)
+ def read_committer_email_address_from_git(self, commit):
+ # Run git to get the committer email
+ return self.run_git('show', '--pretty="%cn <%ce>"', '--no-patch', commit)
+
def run_git(self, *args):
cmd = [ 'git' ]
cmd += args
--
2.21.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer email address
2020-01-02 15:25 ` [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer " Philippe Mathieu-Daudé
@ 2020-01-03 22:59 ` Jordan Justen
2020-01-04 11:38 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 10+ messages in thread
From: Jordan Justen @ 2020-01-03 22:59 UTC (permalink / raw)
To: Philippe Mathieu-Daude, devel
Cc: Laszlo Ersek, Michael D Kinney, Philippe Mathieu-Daude,
Liming Gao
On 2020-01-02 07:25:53, Philippe Mathieu-Daude wrote:
> To avoid patches committed with incorrect email address,
> use the EmailAddressCheck class on the committer email too.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
> ---
> RFC because I haven't checked --pretty="%cn <%ce>" works on Windows shell.
>
> BaseTools/Scripts/PatchCheck.py | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index f0e661bfd6e3..3baeb3de7ba2 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -560,6 +560,8 @@ class CheckGitCommits:
> else:
> blank_line = True
> print('Checking git commit:', commit)
> + email = self.read_committer_email_address_from_git(commit)
> + self.ok &= EmailAddressCheck(email, 'Committer').ok
> patch = self.read_patch_from_git(commit)
> self.ok &= CheckOnePatch(commit, patch).ok
> if not commits:
> @@ -578,6 +580,10 @@ class CheckGitCommits:
> # Run git to get the commit patch
> return self.run_git('show', '--pretty=email', '--no-textconv', commit)
>
> + def read_committer_email_address_from_git(self, commit):
> + # Run git to get the committer email
> + return self.run_git('show', '--pretty="%cn <%ce>"', '--no-patch', commit)
I think '--pretty=%cn <%ce>' ought to work without double-quotes
because the argument is separately sent via the subprocess.Popen call.
I'm not certain it will work, but it ought to. :)
-Jordan
> +
> def run_git(self, *args):
> cmd = [ 'git' ]
> cmd += args
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer email address
2020-01-03 22:59 ` Jordan Justen
@ 2020-01-04 11:38 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-04 11:38 UTC (permalink / raw)
To: Jordan Justen, devel; +Cc: Laszlo Ersek, Michael D Kinney, Liming Gao
On 1/3/20 11:59 PM, Jordan Justen wrote:
> On 2020-01-02 07:25:53, Philippe Mathieu-Daude wrote:
>> To avoid patches committed with incorrect email address,
>> use the EmailAddressCheck class on the committer email too.
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Signed-off-by: Philippe Mathieu-Daude <philmd@redhat.com>
>> ---
>> RFC because I haven't checked --pretty="%cn <%ce>" works on Windows shell.
>>
>> BaseTools/Scripts/PatchCheck.py | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
>> index f0e661bfd6e3..3baeb3de7ba2 100755
>> --- a/BaseTools/Scripts/PatchCheck.py
>> +++ b/BaseTools/Scripts/PatchCheck.py
>> @@ -560,6 +560,8 @@ class CheckGitCommits:
>> else:
>> blank_line = True
>> print('Checking git commit:', commit)
>> + email = self.read_committer_email_address_from_git(commit)
>> + self.ok &= EmailAddressCheck(email, 'Committer').ok
>> patch = self.read_patch_from_git(commit)
>> self.ok &= CheckOnePatch(commit, patch).ok
>> if not commits:
>> @@ -578,6 +580,10 @@ class CheckGitCommits:
>> # Run git to get the commit patch
>> return self.run_git('show', '--pretty=email', '--no-textconv', commit)
>>
>> + def read_committer_email_address_from_git(self, commit):
>> + # Run git to get the committer email
>> + return self.run_git('show', '--pretty="%cn <%ce>"', '--no-patch', commit)
>
> I think '--pretty=%cn <%ce>' ought to work without double-quotes
> because the argument is separately sent via the subprocess.Popen call.
> I'm not certain it will work, but it ought to. :)
Oh, this works like charm on Linux :)
Thanks for the tip (and review)!
Phil.
>> +
>> def run_git(self, *args):
>> cmd = [ 'git' ]
>> cmd += args
>> --
>> 2.21.0
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses
2020-01-02 15:25 [PATCH 0/4] BaseTools/PatchCheck: Check committer/author email addresses Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-01-02 15:25 ` [RFC PATCH 4/4] BaseTools/PatchCheck.py: Check the committer " Philippe Mathieu-Daudé
@ 2020-01-03 13:11 ` Laszlo Ersek
4 siblings, 0 replies; 10+ messages in thread
From: Laszlo Ersek @ 2020-01-03 13:11 UTC (permalink / raw)
To: Philippe Mathieu-Daude, devel; +Cc: Michael D Kinney, Liming Gao, Jordan Justen
Hi,
On 01/02/20 16:25, Philippe Mathieu-Daude wrote:
> Last month I cleaned the git-history incorrect email
> addresses: https://edk2.groups.io/g/devel/message/51834
> but today I noticed more incorrect addresses got committed
> (see caa917491a4..33a3293651).
> To avoid having the same cleanup in the future, fix the
> problem once for all by having PatchCheck doing this
> automatically (on merge).
>
> Since we already have code to check email address in
> PatchCheck, factor the code out to reuse it, and
> add checks for committer/author addresses.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
>
> Philippe Mathieu-Daude (4):
> BaseTools/PatchCheck.py: Extract email check code to EmailAddressCheck
> BaseTools/PatchCheck.py: Let EmailAddressCheck describe email checked
> BaseTools/PatchCheck.py: Check the patch author email address
> BaseTools/PatchCheck.py: Check the committer email address
>
> BaseTools/Scripts/PatchCheck.py | 99 +++++++++++++++++++++++----------
> 1 file changed, 69 insertions(+), 30 deletions(-)
>
still struggling with my email backlog, so I'm going to skip this for now.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread