[Asterisk-code-review] digium commits.py: Fix parsing crashes and add error feedback. (repotools[master])

[Kevin Harwell]

Kevin Harwell has submitted this change and it was merged. ( https://gerrit.asterisk.org/7718 )

Change subject: digium_commits.py: Fix parsing crashes and add error feedback.
......................................................................

digium_commits.py: Fix parsing crashes and add error feedback.

The mkrelease.py script could crash when attempting to parse a commit message
that was not properly formatted. For example, opening parentheses would be
detected, and the script would parse until the index of a closing parentheses.
If there was no closing parentheses, the release script would crash.

This is a two-step fix. The first step is to ensure that when an error like
this occurs, the script does not fail. Try except blocks have been added to
parsing sections that can cause known crashes. When an error is detected, the
new errors list in the DigiumCommitMessageParser class will have the error
appended to it.

The second step will use this error message to provide feedback when a review
is submitted to Gerrit. Jenkins will run a job that goes through some of the
parsing, gathers errors with the parse_commit.py script, and reports them back
in the form of a -1 on the code review, along with the error messages
themselves.

Change-Id: Ibd5c86d69d32bc95d93c0cc81851083410d082a4
---
M digium_commits.py
A parse_commit.py
2 files changed, 39 insertions(+), 13 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/digium_commits.py b/digium_commits.py
index 448e119..3da1a42 100644
--- a/digium_commits.py
+++ b/digium_commits.py
@@ -36,11 +36,15 @@
         the commit was actually an svnmerge block commit, and not actually a
         commit of code.  The blocked attribute will be set to True if the
         string "Blocked revisions " is found in the commit message.
+
+        Another attribute, errors, is defined to keep track of any problems
+        encountered while parsing the commit message.
         '''
         self.message = unicode(message)
         self.author = get_user_by_username(author.strip())
         self.raw = raw
         self.block = False
+        self.errors = []
 
         if self.message.find("Blocked revisions ") >= 0:
             self.block = True
@@ -60,14 +64,22 @@
 
         name = raw_user.strip()
         if ':' in name:
-            name.replace(':', '')
+            name = name.replace(':', '')
         if '<' in name:
-            realname = name[:name.index('<')]
-            email = name[(name.index('<') + 1):name.index('>')]
+            try:
+                realname = name[:name.index('<')]
+                email = name[(name.index('<') + 1):name.index('>')]
+            except:
+                self.errors.append("Error while parsing '{0}': Is there a "
+                                   "closing '>'?".format(name))
         if '(' in name:
             if not realname:
-                realname = name[:name.index('(')]
-                license = name[(name.index('(') + 1):name.index(')')]
+                try:
+                    realname = name[:name.index('(')]
+                    license = name[(name.index('(') + 1):name.index(')')]
+                except:
+                    self.errors.append("Error while parsing '{0}': Is there "
+                                       "a closing ')'?".format(name))
         if not realname:
             realname = name
         if len(realname) == 0:
@@ -165,14 +177,7 @@
             if 'submitted by' in token.lower():
                 name = token[token.lower().index('submitted by') + 12:]
             if name:
-                user = None
-                try:
-                    user = self.extract_user(name)
-                    break
-                except:
-                    print("INFO: Unable to parse message for users: ")
-                    print(self.message)
-
+                user = self.extract_user(name)
                 if user and user not in coders:
                     coders.append(user)
         if len(coders) == 0:
diff --git a/parse_commit.py b/parse_commit.py
new file mode 100644
index 0000000..c47389c
--- /dev/null
+++ b/parse_commit.py
@@ -0,0 +1,21 @@
+#!/usr/bin/env python
+
+import sys
+from argparse import ArgumentParser
+from digium_commits import DigiumCommitMessageParser
+
+def parse_commit(commit, author):
+    commit_parser = DigiumCommitMessageParser(commit, author)
+    commit_parser.get_coders()
+    if len(commit_parser.errors) > 0:
+        for error in commit_parser.errors:
+            print error
+        sys.exit(-1)
+
+if __name__ == '__main__':
+    arg_parser = ArgumentParser()
+    arg_parser.add_argument('-c', '--commit', required=True, help='The commit message')
+    arg_parser.add_argument('-a', '--author', required=True, help='The author of the commit')
+    args = arg_parser.parse_args()
+    parse_commit(args.commit, args.author)
+    sys.exit(0)

-- 
To view, visit https://gerrit.asterisk.org/7718
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd5c86d69d32bc95d93c0cc81851083410d082a4
Gerrit-Change-Number: 7718
Gerrit-PatchSet: 3
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180111/3e90bbca/attachment-0001.html>