launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/launchpad/buildd-regex-bug-719162 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #719162 [Natty] check-implicit-pointer-functions fails on natty, resulting possibly broken packages
  https://bugs.launchpad.net/bugs/719162

For more details, see:
https://code.launchpad.net/~julian-edwards/launchpad/buildd-regex-bug-719162/+merge/49796

= Summary =
Make buildd pointer check regexes work on natty

== Proposed fix ==
64-bit architecture builds have a possible problem with implicit casting of 
integers to pointers, which will cause segfaults in the real world.  A check 
is made for this in the buildds, but unfortunately it's broken on natty 
because the regex that's used doesn't pick up the subtle change in compiler 
output.

== Pre-implementation notes ==
See the bug.

== Implementation details ==
There's a few changes as I needed to juggle stuff around so that tests could 
be written:

 * lib/canonical/buildd/check-implicit-pointer-functions now has a symlink 
lib/canonical/buildd/check_implicit_pointer_functions.py so that I can import 
it for testing
 * check-implicit-pointer-functions was altered so the code is all inside a 
function so it doesn't get run when it's imported.
 * new tests at 
lib/canonical/buildd/tests/test_check_implicit_pointer_functions.py

== Tests ==
bin/test -cvv test_check_implicit_pointer_functions

== Demo and Q/A ==
Quite hard, requires some builds on dogfood/staging to test it, but manual 
shell testing shows the script is working fine.
-- 
https://code.launchpad.net/~julian-edwards/launchpad/buildd-regex-bug-719162/+merge/49796
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~julian-edwards/launchpad/buildd-regex-bug-719162 into lp:launchpad.

=== modified file 'lib/canonical/buildd/check-implicit-pointer-functions'
--- lib/canonical/buildd/check-implicit-pointer-functions	2010-01-13 23:16:43 +0000
+++ lib/canonical/buildd/check-implicit-pointer-functions	2011-02-15 12:36:51 +0000
@@ -35,10 +35,11 @@
 import re
 import sys
 
-implicit_pattern = re.compile("([^:]*):(\d+): warning: implicit declaration "
-                              + "of function [`']([^']*)'")
+implicit_pattern = re.compile(
+    "([^:]*):(\d+):(\d+:)? warning: implicit declaration "
+    "of function [`']([^']*)'")
 pointer_pattern = re.compile(
-    "([^:]*):(\d+): warning: "
+    "([^:]*):(\d+):(\d+:)? warning: "
     + "("
     +  "(assignment"
     +  "|initialization"
@@ -48,53 +49,55 @@
     +  ") makes pointer from integer without a cast"
     + "|"
     + "cast to pointer from integer of different size)")
-last_implicit_filename = ""
-last_implicit_linenum = -1
-last_implicit_func = ""
-
-errlist = ""
-
-in_line = False
-warn_only = False
-
-for arg in sys.argv[1:]:
-    if arg == '--inline':
-        in_line = True
-    elif arg == '--warnonly':
-        warn_only = True
-
-rv = 0
-while True:
-    line = sys.stdin.readline()
-    if in_line:
-        sys.stdout.write(line)
-        sys.stdout.flush()
-    if line == '':
-        break
-    m = implicit_pattern.match(line)
-    if m:
-        last_implicit_filename = m.group(1)
-        last_implicit_linenum = int(m.group(2))
-        last_implicit_func = m.group(3)
-    else:
-        m = pointer_pattern.match(line)
+
+def main():
+    last_implicit_filename = ""
+    last_implicit_linenum = -1
+    last_implicit_func = ""
+
+    errlist = ""
+
+    in_line = False
+    warn_only = False
+
+    for arg in sys.argv[1:]:
+        if arg == '--inline':
+            in_line = True
+        elif arg == '--warnonly':
+            warn_only = True
+
+    rv = 0
+    while True:
+        line = sys.stdin.readline()
+        if in_line:
+            sys.stdout.write(line)
+            sys.stdout.flush()
+        if line == '':
+            break
+        m = implicit_pattern.match(line)
         if m:
-            pointer_filename = m.group(1)
-            pointer_linenum = int(m.group(2))
-            if (last_implicit_filename == pointer_filename
-                and last_implicit_linenum == pointer_linenum):
-                err = "Function `%s' implicitly converted to pointer at " \
-                      "%s:%d" % (last_implicit_func, last_implicit_filename,
-                                 last_implicit_linenum)
-                errlist += err+"\n"
-                print err
-                if not warn_only:
-                    rv = 3
+            last_implicit_filename = m.group(1)
+            last_implicit_linenum = int(m.group(2))
+            last_implicit_func = m.group(4)
+        else:
+            m = pointer_pattern.match(line)
+            if m:
+                pointer_filename = m.group(1)
+                pointer_linenum = int(m.group(2))
+                if (last_implicit_filename == pointer_filename
+                    and last_implicit_linenum == pointer_linenum):
+                    err = "Function `%s' implicitly converted to pointer at " \
+                          "%s:%d" % (last_implicit_func, last_implicit_filename,
+                                     last_implicit_linenum)
+                    errlist += err+"\n"
+                    print err
+                    if not warn_only:
+                        rv = 3
 
-if len(errlist):
-    if in_line:
-	print errlist
-    print """
+    if len(errlist):
+        if in_line:
+            print errlist
+            print """
 
 Our automated build log filter detected the problem(s) above that will
 likely cause your package to segfault on architectures where the size of
@@ -106,7 +109,10 @@
 on ia64, they are errors.  Please correct them for your next upload.
 
 More information can be found at:
-    http://wiki.debian.org/ImplicitPointerConversions
-
-"""
-sys.exit(rv)
+http://wiki.debian.org/ImplicitPointerConversions
+
+    """
+    sys.exit(rv)
+
+if __name__ == '__main__':
+    main()

=== added symlink 'lib/canonical/buildd/check_implicit_pointer_functions.py'
=== target is u'check-implicit-pointer-functions'
=== added file 'lib/canonical/buildd/tests/test_check_implicit_pointer_functions.py'
--- lib/canonical/buildd/tests/test_check_implicit_pointer_functions.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/buildd/tests/test_check_implicit_pointer_functions.py	2011-02-15 12:36:51 +0000
@@ -0,0 +1,43 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from lp.testing import TestCase
+
+from canonical.buildd.check_implicit_pointer_functions import implicit_pattern
+from canonical.buildd.check_implicit_pointer_functions import pointer_pattern
+
+
+class TestPointerCheckRegexes(TestCase):
+
+    def test_catches_pointer_from_integer_without_column_number(self):
+        # Regex should match compiler errors that don't include the
+        # column number.
+        line = (
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            "warning: assignment makes pointer from integer without a cast")
+        self.assertIsNot(None, pointer_pattern.match(line))
+
+    def test_catches_pointer_from_integer_with_column_number(self):
+        # Regex should match compiler errors that do include the
+        # column number.
+        line = (
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94:7: "
+            "warning: assignment makes pointer from integer without a cast")
+        self.assertIsNot(None, pointer_pattern.match(line))
+
+    def test_catches_implicit_function_without_column_number(self):
+        # Regex should match compiler errors that do include the
+        # column number.
+        line = (
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            "warning: implicit declaration of function 'foo'")
+        self.assertIsNot(None, implicit_pattern.match(line))
+
+    def test_catches_implicit_function_with_column_number(self):
+        # Regex should match compiler errors that do include the
+        # column number.
+        line = (
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94:7: "
+            "warning: implicit declaration of function 'foo'")
+        self.assertIsNot(None, implicit_pattern.match(line))
+