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

[Benjamin Keith Ford]

Benjamin Keith Ford has uploaded this change for review. ( 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. A hook will be implemented that runs through some of
the parsing, gathers errors, 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
1 file changed, 18 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/repotools refs/changes/18/7718/1

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:

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

Gerrit-Project: repotools
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd5c86d69d32bc95d93c0cc81851083410d082a4
Gerrit-Change-Number: 7718
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171222/cc258906/attachment.html>