[MediaWiki-commits] [Gerrit] integration/commit-message-validator[master]: Validate Change-Id and Depends-On values
jenkins-bot has submitted this change and it was merged.
Change subject: Validate Change-Id and Depends-On values
..
Validate Change-Id and Depends-On values
A valid Gerrit change-id is a SHA-1 hash represented as a 40 digit hex
string prefixed with I to distinguish it from a commit hash.
Bug: T142801
Change-Id: I98733c7e87f02ca835b0968415ce1e3253996cf6
---
M commit_message_validator/__init__.py
M commit_message_validator/tests/data/check_message_errors.msg
M commit_message_validator/tests/data/check_message_errors.out
M commit_message_validator/tests/data/check_message_ok.msg
A commit_message_validator/tests/data/invalid_change_id.msg
A commit_message_validator/tests/data/invalid_change_id.out
A commit_message_validator/tests/data/invalid_depends_on.msg
A commit_message_validator/tests/data/invalid_depends_on.out
A commit_message_validator/tests/data/multiple_change_id.msg
A commit_message_validator/tests/data/multiple_change_id.out
10 files changed, 48 insertions(+), 16 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/commit_message_validator/__init__.py
b/commit_message_validator/__init__.py
index ee64e30..59964d4 100644
--- a/commit_message_validator/__init__.py
+++ b/commit_message_validator/__init__.py
@@ -98,17 +98,10 @@
if m.group(2) != ' ':
yield "Expected one space after 'Depends-On:'"
-change_id = m.group(3).strip()
-change_id_is_hex = True
-try:
-int(change_id[1:], 16)
-except ValueError:
-change_id_is_hex = False
-if change_id.upper().startswith('I') and change_id_is_hex:
-if change_id[0] != 'I':
-yield "The Depends-On value must use uppercase I"
-else:
-yield "The Depends-On value is not a Gerrit Change-Id"
+
+def is_valid_change_id(s):
+"""A Gerrit change id is a 40 character hex string prefixed with 'I'."""
+return re.match('^I[a-f0-9]{40}$', s)
def check_message(lines):
@@ -120,8 +113,10 @@
- For any "^Bug: " line, prior line is another Bug: line or empty
- For any "^Depends-On: " line, next line is not blank
- For any "^Depends-On: " line, prior line is Bug|Depends-On or empty
+- For any "^Depends-On: " line, the RHS is a valid change id
- Exactly one "Change-Id: " line per commit
- For "Change-Id: " line, prior line is empty or "^(Bug|Depends-On): "
+- For "Change-Id: " line, the RHS is a valid change id
- No blank lines between any "(Bug|Depends-On): " lines and "Change-Id: "
- Only "(cherry picked from commit" can follow "Change-Id: "
- Message has at least 3 lines (subject, blank, Change-Id)
@@ -166,7 +161,13 @@
errors.append(
"Line %d: Expected blank line before Depends-On:" % rline)
-if line.startswith('Change-Id: I'):
+(label, rhs) = line.split(' ', 1)
+if not is_valid_change_id(rhs):
+errors.append(
+"Line %d: value must be a single valid Gerrit change id"
+% rline)
+
+if line.startswith('Change-Id:'):
# Only expect one "Change-Id: " line
if changeid_line is not False:
errors.append(
@@ -196,6 +197,12 @@
changeid_line = rline
+(label, rhs) = line.split(' ', 1)
+if not is_valid_change_id(rhs):
+errors.append(
+"Line %d: value must be a single valid Gerrit change id"
+% rline)
+
last_lineno = rline
last_line = line
diff --git a/commit_message_validator/tests/data/check_message_errors.msg
b/commit_message_validator/tests/data/check_message_errors.msg
index 3d2343f..d28e005 100644
--- a/commit_message_validator/tests/data/check_message_errors.msg
+++ b/commit_message_validator/tests/data/check_message_errors.msg
@@ -11,12 +11,12 @@
Bug: T12
Bug: t13
-Depends-On: I1234
+Depends-On: I395fd614bfa8bcfc46a35f955fb77ec6f03f7a01
Bug: t14
-Depends-On: I5678
-Change-Id: I1234567890abcdef123456
-Change-Id: I1234567890abcdef123457
+Depends-On: I00d0f7c3b294c3ddc656f9a5447df89c63142203
+Change-Id: I20c70ff250085de49e4b961da4b53258858df1dd
+Change-Id: I62a5bbf5a94da6f121a7836859371a2a2f60873d
Depends-On: I9abc
Foo bar
diff --git a/commit_message_validator/tests/data/check_message_errors.out
b/commit_message_validator/tests/data/check_message_errors.out
index 2d22d18..fff8ef8 100644
--- a/commit_message_validator/tests/data/check_message_errors.out
+++ b/commit_message_validator/tests/data/check_message_errors.out
@@ -22,6 +22,7 @@
Line 16: The phabricator task ID must use uppercase T
Line 18: Extra Change-Id found, next at 19
Line 20: Expected blank line before Depends-On:
+Line 20: value must be a single valid Gerrit change id
Line 21: Unexpected blank line after
[MediaWiki-commits] [Gerrit] integration/commit-message-validator[master]: Validate Change-Id and Depends-On values
BryanDavis has uploaded a new change for review.
https://gerrit.wikimedia.org/r/304432
Change subject: Validate Change-Id and Depends-On values
..
Validate Change-Id and Depends-On values
A valid Gerrit change-id is a SHA-1 hash represented as a 40 digit hex
string prefixed with I to distinguish it from a commit hash.
Bug: T142801
Change-Id: I98733c7e87f02ca835b0968415ce1e3253996cf6
---
M commit_message_validator/__init__.py
M commit_message_validator/tests/data/check_message_errors.msg
M commit_message_validator/tests/data/check_message_errors.out
A commit_message_validator/tests/data/invalid_change_id.msg
A commit_message_validator/tests/data/invalid_change_id.out
A commit_message_validator/tests/data/invalid_depends_on.msg
A commit_message_validator/tests/data/invalid_depends_on.out
A commit_message_validator/tests/data/multiple_change_id.msg
A commit_message_validator/tests/data/multiple_change_id.out
9 files changed, 46 insertions(+), 16 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/integration/commit-message-validator
refs/changes/32/304432/1
diff --git a/commit_message_validator/__init__.py
b/commit_message_validator/__init__.py
index ee64e30..59964d4 100644
--- a/commit_message_validator/__init__.py
+++ b/commit_message_validator/__init__.py
@@ -98,17 +98,10 @@
if m.group(2) != ' ':
yield "Expected one space after 'Depends-On:'"
-change_id = m.group(3).strip()
-change_id_is_hex = True
-try:
-int(change_id[1:], 16)
-except ValueError:
-change_id_is_hex = False
-if change_id.upper().startswith('I') and change_id_is_hex:
-if change_id[0] != 'I':
-yield "The Depends-On value must use uppercase I"
-else:
-yield "The Depends-On value is not a Gerrit Change-Id"
+
+def is_valid_change_id(s):
+"""A Gerrit change id is a 40 character hex string prefixed with 'I'."""
+return re.match('^I[a-f0-9]{40}$', s)
def check_message(lines):
@@ -120,8 +113,10 @@
- For any "^Bug: " line, prior line is another Bug: line or empty
- For any "^Depends-On: " line, next line is not blank
- For any "^Depends-On: " line, prior line is Bug|Depends-On or empty
+- For any "^Depends-On: " line, the RHS is a valid change id
- Exactly one "Change-Id: " line per commit
- For "Change-Id: " line, prior line is empty or "^(Bug|Depends-On): "
+- For "Change-Id: " line, the RHS is a valid change id
- No blank lines between any "(Bug|Depends-On): " lines and "Change-Id: "
- Only "(cherry picked from commit" can follow "Change-Id: "
- Message has at least 3 lines (subject, blank, Change-Id)
@@ -166,7 +161,13 @@
errors.append(
"Line %d: Expected blank line before Depends-On:" % rline)
-if line.startswith('Change-Id: I'):
+(label, rhs) = line.split(' ', 1)
+if not is_valid_change_id(rhs):
+errors.append(
+"Line %d: value must be a single valid Gerrit change id"
+% rline)
+
+if line.startswith('Change-Id:'):
# Only expect one "Change-Id: " line
if changeid_line is not False:
errors.append(
@@ -196,6 +197,12 @@
changeid_line = rline
+(label, rhs) = line.split(' ', 1)
+if not is_valid_change_id(rhs):
+errors.append(
+"Line %d: value must be a single valid Gerrit change id"
+% rline)
+
last_lineno = rline
last_line = line
diff --git a/commit_message_validator/tests/data/check_message_errors.msg
b/commit_message_validator/tests/data/check_message_errors.msg
index 3d2343f..d28e005 100644
--- a/commit_message_validator/tests/data/check_message_errors.msg
+++ b/commit_message_validator/tests/data/check_message_errors.msg
@@ -11,12 +11,12 @@
Bug: T12
Bug: t13
-Depends-On: I1234
+Depends-On: I395fd614bfa8bcfc46a35f955fb77ec6f03f7a01
Bug: t14
-Depends-On: I5678
-Change-Id: I1234567890abcdef123456
-Change-Id: I1234567890abcdef123457
+Depends-On: I00d0f7c3b294c3ddc656f9a5447df89c63142203
+Change-Id: I20c70ff250085de49e4b961da4b53258858df1dd
+Change-Id: I62a5bbf5a94da6f121a7836859371a2a2f60873d
Depends-On: I9abc
Foo bar
diff --git a/commit_message_validator/tests/data/check_message_errors.out
b/commit_message_validator/tests/data/check_message_errors.out
index 2d22d18..fff8ef8 100644
--- a/commit_message_validator/tests/data/check_message_errors.out
+++ b/commit_message_validator/tests/data/check_message_errors.out
@@ -22,6 +22,7 @@
Line 16: The phabricator task ID must use uppercase T
Line 18: Extra Change-Id found, next at 19
Line 20: Expected blank line before Depends-On:
+Line 20: value must be a single valid Gerrit change id
Line 21: Unexpected blank line after
