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 Depends-On:
Line 20: Unexpected line after Change-Id
Line 21: Unexpected line after Change-Id
diff --git a/commit_message_validator/tests/data/check_message_ok.msg
b/commit_message_validator/tests/data/check_message_ok.msg
index d90f841..c287bd6 100644
--- a/commit_message_validator/tests/data/check_message_ok.msg
+++ b/commit_message_validator/tests/data/check_message_ok.msg
@@ -21,8 +21,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)
diff --git a/commit_message_validator/tests/data/invalid_change_id.msg
b/commit_message_validator/tests/data/invalid_change_id.msg
new file mode 100644
index 0000000..6f6a5d7
--- /dev/null
+++ b/commit_message_validator/tests/data/invalid_change_id.msg
@@ -0,0 +1,3 @@
+This change id is not valid
+
+Change-Id: Ideafbeef
diff --git a/commit_message_validator/tests/data/invalid_change_id.out
b/commit_message_validator/tests/data/invalid_change_id.out
new file mode 100644
index 0000000..63b86ac
--- /dev/null
+++ b/commit_message_validator/tests/data/invalid_change_id.out
@@ -0,0 +1,4 @@
+commit-message-validator v%version%
+The following errors were found:
+Line 3: value must be a single valid Gerrit change id
+Please review
<https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines> and update
your commit message accordingly
diff --git a/commit_message_validator/tests/data/invalid_depends_on.msg
b/commit_message_validator/tests/data/invalid_depends_on.msg
new file mode 100644
index 0000000..4f8d180
--- /dev/null
+++ b/commit_message_validator/tests/data/invalid_depends_on.msg
@@ -0,0 +1,4 @@
+This Depends-On is not valid
+
+Depends-On: pizza
+Change-Id: I9cd485c1923dd40f05bd61ef776cb6a9a57187f0
diff --git a/commit_message_validator/tests/data/invalid_depends_on.out
b/commit_message_validator/tests/data/invalid_depends_on.out
new file mode 100644
index 0000000..63b86ac
--- /dev/null
+++ b/commit_message_validator/tests/data/invalid_depends_on.out
@@ -0,0 +1,4 @@
+commit-message-validator v%version%
+The following errors were found:
+Line 3: value must be a single valid Gerrit change id
+Please review
<https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines> and update
your commit message accordingly
diff --git a/commit_message_validator/tests/data/multiple_change_id.msg
b/commit_message_validator/tests/data/multiple_change_id.msg
new file mode 100644
index 0000000..7e5c8fb
--- /dev/null
+++ b/commit_message_validator/tests/data/multiple_change_id.msg
@@ -0,0 +1,3 @@
+Only one change id per line
+
+Change-Id: I62a5bbf5a94da6f121a7836859371a2a2f60873d,
I9cd485c1923dd40f05bd61ef776cb6a9a57187f0
diff --git a/commit_message_validator/tests/data/multiple_change_id.out
b/commit_message_validator/tests/data/multiple_change_id.out
new file mode 100644
index 0000000..63b86ac
--- /dev/null
+++ b/commit_message_validator/tests/data/multiple_change_id.out
@@ -0,0 +1,4 @@
+commit-message-validator v%version%
+The following errors were found:
+Line 3: value must be a single valid Gerrit change id
+Please review
<https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines> and update
your commit message accordingly
--
To view, visit https://gerrit.wikimedia.org/r/304432
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I98733c7e87f02ca835b0968415ce1e3253996cf6
Gerrit-PatchSet: 2
Gerrit-Project: integration/commit-message-validator
Gerrit-Branch: master
Gerrit-Owner: BryanDavis <bda...@wikimedia.org>
Gerrit-Reviewer: Hashar <has...@free.fr>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: Paladox <thomasmulhall...@yahoo.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
