launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:check-implicit-pointer-functions-wrapper into launchpad-buildd:master.

Commit message:
Add a proper wrapper for check-implicit-pointer-functions

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/383494

This makes unit testing easier, and is also easier to deal with at the packaging level than the previous approach of poking in a symlink.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:check-implicit-pointer-functions-wrapper into launchpad-buildd:master.

diff --git a/bin/check-implicit-pointer-functions b/bin/check-implicit-pointer-functions
new file mode 100755
index 0000000..6e20f3f
--- /dev/null
+++ b/bin/check-implicit-pointer-functions
@@ -0,0 +1,33 @@
+#! /usr/bin/python -u
+#
+# Copyright 2020 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Scan for compiler warnings that are likely to cause 64-bit problems."""
+
+from __future__ import print_function
+
+from argparse import ArgumentParser
+import sys
+
+from lpbuildd.check_implicit_pointer_functions import filter_log
+
+
+def main():
+    parser = ArgumentParser(description=__doc__)
+    parser.add_argument(
+        "--inline", default=False, action="store_true",
+        help="Pass through input, inserting errors in-line")
+    parser.add_argument(
+        "--warnonly", default=False, action="store_true",
+        help="Exit zero even if problems are found")
+    args = parser.parse_args()
+    problems = filter_log(sys.stdin, sys.stdout, in_line=args.inline)
+    if problems and not args.warnonly:
+        return 2
+    else:
+        return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())
diff --git a/debian/changelog b/debian/changelog
index 867b35e..b16f8f2 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -19,8 +19,12 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
   * Only include mock in tests_require for Python 2.
   * Fix snap proxy testing for Python 3.
   * Rename [slave] configuration section to [builder].
+<<<<<<< debian/changelog
   * Convert translation templates builds to the VCS mixin, thereby adding
     git support.
+=======
+  * Add a proper wrapper for check-implicit-pointer-functions.
+>>>>>>> debian/changelog
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Tue, 28 Apr 2020 10:19:27 +0100
 
diff --git a/debian/launchpad-buildd.install b/debian/launchpad-buildd.install
index 79ce868..981c763 100644
--- a/debian/launchpad-buildd.install
+++ b/debian/launchpad-buildd.install
@@ -1,5 +1,6 @@
 bin/builder-prep			usr/share/launchpad-buildd/bin
 bin/buildrecipe				usr/share/launchpad-buildd/bin
+bin/check-implicit-pointer-functions	usr/share/launchpad-buildd/bin
 bin/in-target				usr/share/launchpad-buildd/bin
 bin/sbuild-package			usr/share/launchpad-buildd/bin
 bin/snap-git-proxy			usr/share/launchpad-buildd/bin
diff --git a/debian/launchpad-buildd.links b/debian/launchpad-buildd.links
deleted file mode 100755
index 2fe5ccb..0000000
--- a/debian/launchpad-buildd.links
+++ /dev/null
@@ -1,2 +0,0 @@
-#! /usr/bin/dh-exec
-${LIBDIR}/lpbuildd/check_implicit_pointer_functions.py	usr/share/launchpad-buildd/bin/check-implicit-pointer-functions
diff --git a/debian/rules b/debian/rules
index 6266e95..8468ec0 100755
--- a/debian/rules
+++ b/debian/rules
@@ -1,6 +1,6 @@
 #!/usr/bin/make -f
 #
-# Copyright 2009-2017 Canonical Ltd.  
+# Copyright 2009-2020 Canonical Ltd.  
 # 
 # This software is licensed under the GNU Affero General Public License version
 # 3 (see the file LICENSE).
@@ -18,10 +18,5 @@ override_dh_auto_build:
 	--arch=i386 --port=8221 --name=default --host=buildd.buildd \
 		> buildd-slave-example.conf
 
-override_dh_auto_install:
-	dh_auto_install
-	# This should be split into a testable library and a wrapper.
-	chmod +x debian/python-lpbuildd/$(LIBDIR)/lpbuildd/check_implicit_pointer_functions.py
-
 override_dh_builddeb:
 	dh_builddeb -- -Zgzip
diff --git a/lpbuildd/check_implicit_pointer_functions.py b/lpbuildd/check_implicit_pointer_functions.py
index 32e1fe6..26372ae 100755
--- a/lpbuildd/check_implicit_pointer_functions.py
+++ b/lpbuildd/check_implicit_pointer_functions.py
@@ -3,7 +3,7 @@
 #
 # Copyright (c) 2004 Hewlett-Packard Development Company, L.P.
 #       David Mosberger <davidm@xxxxxxxxxx>
-# Copyright 2010 Canonical Ltd.
+# Copyright 2010-2020 Canonical Ltd.
 #
 # Permission is hereby granted, free of charge, to any person
 # obtaining a copy of this software and associated documentation
@@ -52,28 +52,19 @@ pointer_pattern = re.compile(
     r"|"
     r"cast to pointer from integer of different size)")
 
-def main():
+
+def filter_log(in_file, out_file, in_line=False):
     last_implicit_filename = ""
     last_implicit_linenum = -1
     last_implicit_func = ""
 
-    errlist = ""
-
-    in_line = False
-    warn_only = False
+    errlist = []
 
-    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()
+        line = in_file.readline()
         if in_line:
-            sys.stdout.write(line)
-            sys.stdout.flush()
+            out_file.write(line)
+            out_file.flush()
         if line == '':
             break
         m = implicit_pattern.match(line)
@@ -91,15 +82,13 @@ def main():
                     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 = 2
+                    errlist.append(err)
+                    out_file.write(err + "\n")
 
-    if len(errlist):
+    if errlist:
         if in_line:
-            print(errlist)
-            print("""
+            out_file.write("\n".join(errlist) + "\n\n")
+            out_file.write("""
 
 Our automated build log filter detected the problem(s) above that will
 likely cause your package to segfault on architectures where the size of
@@ -114,7 +103,4 @@ More information can be found at:
 http://wiki.debian.org/ImplicitPointerConversions
 
     """)
-    sys.exit(rv)
-
-if __name__ == '__main__':
-    main()
+    return len(errlist)
diff --git a/lpbuildd/tests/test_check_implicit_pointer_functions.py b/lpbuildd/tests/test_check_implicit_pointer_functions.py
index c6f8281..582f1f6 100644
--- a/lpbuildd/tests/test_check_implicit_pointer_functions.py
+++ b/lpbuildd/tests/test_check_implicit_pointer_functions.py
@@ -1,10 +1,17 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import re
+
+from six import StringIO
 from testtools import TestCase
+from testtools.matchers import MatchesRegex
 
-from lpbuildd.check_implicit_pointer_functions import implicit_pattern
-from lpbuildd.check_implicit_pointer_functions import pointer_pattern
+from lpbuildd.check_implicit_pointer_functions import (
+    filter_log,
+    implicit_pattern,
+    pointer_pattern,
+    )
 
 
 class TestPointerCheckRegexes(TestCase):
@@ -26,7 +33,7 @@ class TestPointerCheckRegexes(TestCase):
         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
+        # 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: "
@@ -41,3 +48,60 @@ class TestPointerCheckRegexes(TestCase):
             "warning: implicit declaration of function 'foo'")
         self.assertIsNot(None, implicit_pattern.match(line))
 
+
+class TestFilterLog(TestCase):
+
+    def test_out_of_line_no_errors(self):
+        in_file = StringIO("Innocuous build log\nwith no errors\n")
+        out_file = StringIO()
+        self.assertEqual(0, filter_log(in_file, out_file))
+        self.assertEqual("", out_file.getvalue())
+
+    def test_out_of_line_errors(self):
+        in_file = StringIO(
+            "Build log with errors\n"
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            "warning: implicit declaration of function 'foo'\n"
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            "warning: assignment makes pointer from integer without a cast\n"
+            "More build log\n")
+        out_file = StringIO()
+        self.assertEqual(1, filter_log(in_file, out_file))
+        self.assertEqual(
+            "Function `foo' implicitly converted to pointer at "
+            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94\n",
+            out_file.getvalue())
+
+    def test_in_line_no_errors(self):
+        in_file = StringIO("Innocuous build log\nwith no errors\n")
+        out_file = StringIO()
+        self.assertEqual(0, filter_log(in_file, out_file, in_line=True))
+        self.assertEqual(
+            "Innocuous build log\nwith no errors\n", out_file.getvalue())
+
+    def test_in_line_errors(self):
+        in_file = StringIO(
+            "Build log with errors\n"
+            "/build/gtk/ubuntumenuproxymodule.c:94: "
+            "warning: implicit declaration of function 'foo'\n"
+            "/build/gtk/ubuntumenuproxymodule.c:94: "
+            "warning: assignment makes pointer from integer without a cast\n"
+            "More build log\n")
+        out_file = StringIO()
+        self.assertEqual(1, filter_log(in_file, out_file, in_line=True))
+        self.assertThat(out_file.getvalue(), MatchesRegex(
+            r"^" +
+            re.escape(
+                "Build log with errors\n"
+                "/build/gtk/ubuntumenuproxymodule.c:94: "
+                "warning: implicit declaration of function 'foo'\n"
+                "/build/gtk/ubuntumenuproxymodule.c:94: "
+                "warning: assignment makes pointer from integer without a "
+                "cast\n"
+                "Function `foo' implicitly converted to pointer at "
+                "/build/gtk/ubuntumenuproxymodule.c:94\n"
+                "More build log\n"
+                "Function `foo' implicitly converted to pointer at "
+                "/build/gtk/ubuntumenuproxymodule.c:94\n\n\n\n") +
+            r"Our automated build log filter.*",
+            flags=re.M | re.S))