[kteam-tools][PATCH 1/2] debian.py: fix changelog[content, bugs] off-by-one

Kamal Mostafa kamal at canonical.com
Fri Jun 15 20:44:31 UTC 2018


The debian changelog parser in debian.py (both implementations) is bugged
such that the ['content'] and ['bugs'] values for each changelog[N] actually
get placed in changelog[N+1].  I.e. changelog[0] represents the topmost
changelog entry but its changelog[0]['content'] and changelog[0]['bugs']
are always empty; their values end up in changelog[1].

The only known caller that accesses the ['content'] and ['bugs'] elements
is verify-release-ready -- it has been "working around" the buggy
changelog[0] behavior by accessing the changelog[1] entry for just those
elements.

Another subtle bug in verify-release-ready also gets fixed here: That
"work around" for the shifted changelog elements was also being applied
(inappropriately) to the non-shifted 'series' and 'package' values.  So
verify-release-ready was actually verifying the series and package of
the previous changelog block, not the topmost block as intended.

This commit fixes the buggy debian.py changelog parser behavior and
verify-release-ready's expectation of it.

IMPORTANT: Any other out-of-tree users of this debian.py changelog
parser will see a behavior change w.r.t. the ['content'] and ['bugs']
elements.

Signed-off-by: Kamal Mostafa <kamal at canonical.com>
---
 ktl/debian.py                     | 41 ++++++++++++++++++++++-----------------
 maintscripts/verify-release-ready | 14 ++++++-------
 py3/lib/debian.py                 | 33 ++++++++++++++++++-------------
 3 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/ktl/debian.py b/ktl/debian.py
index 9adda08..3a2394a 100644
--- a/ktl/debian.py
+++ b/ktl/debian.py
@@ -53,6 +53,7 @@ class Debian:
     bug_nr_rc = compile("#([0-9]+)")
 
     parent_bug_section_rc = compile("^\s*\[ Ubuntu: .*\]")
+    endsection_line_rc = compile("^ -- ")
 
     # debian_directories
     #
@@ -180,11 +181,7 @@ class Debian:
                 section['release'] = release
                 section['series']  = release
                 section['pocket']  = pocket
-                section['content'] = content
-                section['own-content'] = own_content
                 section['package'] = package
-                section['bugs'] = set(bugs)
-                section['own-bugs'] = set(own_bugs)
 
                 m = cls.version_rc.match(version)
                 if m is not None:
@@ -194,26 +191,34 @@ class Debian:
                 else:
                     debug('The version (%s) failed to match the regular expression.\n' % version, cls.debug)
 
-                retval.append(section)
                 content = []
                 own_content = []
                 bugs = []
                 own_bugs = []
                 parsing_own_bugs = True
-            else:
-                if cls.parent_bug_section_rc.match(line):
-                    parsing_own_bugs = False
-
-                # find bug numbers and append them to the list
-                for bug_line_match in finditer(cls.bug_rc, line):
-                    bug_matches = findall(cls.bug_nr_rc, bug_line_match.group(0))
-                    bugs.extend(bug_matches)
-                    if parsing_own_bugs:
-                        own_bugs.extend(bug_matches)
-
-                content.append(line)
+                continue
+
+            content.append(line)
+            if parsing_own_bugs:
+                own_content.append(line)
+
+            if cls.parent_bug_section_rc.match(line):
+                parsing_own_bugs = False
+
+            # find bug numbers and append them to the list
+            for bug_line_match in finditer(cls.bug_rc, line):
+                bug_matches = findall(cls.bug_nr_rc, bug_line_match.group(0))
+                bugs.extend(bug_matches)
                 if parsing_own_bugs:
-                    own_content.append(line)
+                    own_bugs.extend(bug_matches)
+
+            m = cls.endsection_line_rc.match(line)
+            if m is not None:
+                section['content'] = content
+                section['own-content'] = own_content
+                section['bugs'] = set(bugs)
+                section['own-bugs'] = set(own_bugs)
+                retval.append(section)
 
         return retval
 
diff --git a/maintscripts/verify-release-ready b/maintscripts/verify-release-ready
index 3c2abe3..1e6982e 100755
--- a/maintscripts/verify-release-ready
+++ b/maintscripts/verify-release-ready
@@ -227,7 +227,7 @@ class VerifyReleaseReady():
         # Look to see if a tracking bug has been added to the changelog
         #
         found_tracker = False
-        for line in changelog[1]['content']:
+        for line in changelog[0]['content']:
             if '-proposed tracker' in line:
                 found_tracker = True
                 try:
@@ -252,7 +252,7 @@ class VerifyReleaseReady():
                 master_changelog = Debian.master_changelog()
                 found_tracker = False
                 master_tracker_id = 'NOT-FOUND'
-                for line in master_changelog[1]['content']:
+                for line in master_changelog[0]['content']:
                     if '-proposed tracker' in line:
                         try:
                             (junk, master_tracker_id) = line.split('#')
@@ -344,7 +344,7 @@ class VerifyReleaseReady():
         # entries were inserted by debian/scripts/misc/git-ubuntu-log
         msg = 'no "Miscellaneous" entries'
         changelog = Debian.changelog()
-        content = changelog[1]['own-content']
+        content = changelog[0]['own-content']
         for line in content:
             m = re.match("\* Miscellaneous .* changes$", line.strip())
             if m:
@@ -355,9 +355,9 @@ class VerifyReleaseReady():
 
     def verify_changelog_bugs(s):
         changelog = Debian.changelog()
-        changelog_bugs = changelog[1]['own-bugs']
-        changelog_series = changelog[1]['series']
-        changelog_source_package = changelog[1]['package']
+        changelog_bugs = changelog[0]['own-bugs']
+        changelog_series = changelog[0]['series']
+        changelog_source_package = changelog[0]['package']
 
         tracker = None
 
@@ -409,7 +409,7 @@ class VerifyReleaseReady():
 
     def verify_content(s):
         changelog = Debian.changelog()
-        content = changelog[1]['content']
+        content = changelog[0]['content']
 
         # It's important that there be a single blank line at the beginning of
         # the content. (after the package/version/pocket line)
diff --git a/py3/lib/debian.py b/py3/lib/debian.py
index 8f65f23..937458c 100644
--- a/py3/lib/debian.py
+++ b/py3/lib/debian.py
@@ -55,6 +55,7 @@ class Debian:
     bug_rc = compile("LP:\s*#[0-9]+(?:\s*,\s*#[0-9]+)*")
     bug_nr_rc = compile("#([0-9]+)")
     ubuntu_master_re = compile(r'^\s.*\[ Ubuntu: ([0-9.-]*) \]$')
+    endsection_line_rc = compile("^ -- ")
 
     @classmethod
     def fdr(cls, cmd):
@@ -210,9 +211,7 @@ class Debian:
                 section['release'] = release
                 section['series']  = release
                 section['pocket']  = pocket
-                section['content'] = content
                 section['package'] = package
-                section['bugs'] = set(bugs)
 
                 m = cls.version_rc.match(version)
                 if m is not None:
@@ -222,19 +221,27 @@ class Debian:
                 else:
                     debug('The version (%s) failed to match the regular expression.\n' % version, cls.debug)
 
-                retval.append(section)
                 content = []
                 bugs = []
-            else:
-                # find bug numbers and append them to the list
-                for bug_line_match in finditer(cls.bug_rc, line):
-                    bug_matches = findall(cls.bug_nr_rc, bug_line_match.group(0))
-                    bugs.extend(bug_matches)
-                m = cls.ubuntu_master_re.match(line)
-                if m is not None and not section.get("master"):
-                    section['master'] = m.group(1)
-
-                content.append(line)
+                continue
+
+            content.append(line)
+
+            # find bug numbers and append them to the list
+            for bug_line_match in finditer(cls.bug_rc, line):
+                bug_matches = findall(cls.bug_nr_rc, bug_line_match.group(0))
+                bugs.extend(bug_matches)
+
+            m = cls.ubuntu_master_re.match(line)
+            if m is not None and not section.get("master"):
+                section['master'] = m.group(1)
+                continue
+
+            m = cls.endsection_line_rc.match(line)
+            if m is not None:
+                section['content'] = content
+                section['bugs'] = set(bugs)
+                retval.append(section)
 
         return retval
 
-- 
2.7.4





More information about the kernel-team mailing list