[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 Depends
[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 D