Merge ~cjwatson/launchpad-buildd:py3-check-implicit-pointer-functions-bytes into launchpad-buildd:master

[Colin Watson]

Fix check-implicit-pointer-functions on Python 3

Build logs aren't necessarily UTF-8, so process them as bytes.

75194b4... by Colin Watson on 2020-09-28

Fix check-implicit-pointer-functions on Python 3

Build logs aren't necessarily UTF-8, so process them as bytes.

LP: #1897461

[Tom Wardill (twom)]

review: Approve

diff --git a/bin/check-implicit-pointer-functions b/bin/check-implicit-pointer-functions
index 7ff5389..c42d8a6 100755
--- a/bin/check-implicit-pointer-functions
+++ b/bin/check-implicit-pointer-functions
@@ -22,7 +22,12 @@ def main():
         "--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)
+    stdin = sys.stdin
+    stdout = sys.stdout
+    if sys.version_info[0] >= 3:
+        stdin = stdin.buffer
+        stdout = stdout.buffer
+    problems = filter_log(stdin, stdout, in_line=args.inline)
     if problems and not args.warnonly:
         return 2
     else:
diff --git a/debian/changelog b/debian/changelog
index 547c167..7dad9a1 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,6 +2,8 @@ launchpad-buildd (193) UNRELEASED; urgency=medium
 
   * Fix handling of bytes arguments passed to BuildManager.runSubProcess.
   * Fix bytes/text handling in RecipeBuilder.buildTree.
+  * Fix check-implicit-pointer-functions on Python 3: build logs aren't
+    necessarily UTF-8, so process them as bytes (LP: #1897461).
 
  -- Colin Watson <[e-mail]>  Thu, 24 Sep 2020 16:45:27 +0100
 
diff --git a/lpbuildd/check_implicit_pointer_functions.py b/lpbuildd/check_implicit_pointer_functions.py
index 9b8b191..7f28338 100755
--- a/lpbuildd/check_implicit_pointer_functions.py
+++ b/lpbuildd/check_implicit_pointer_functions.py
@@ -35,28 +35,27 @@
 from __future__ import print_function
 
 import re
-import sys
 
 implicit_pattern = re.compile(
-    r"([^:]*):(\d+):(\d+:)? warning: implicit declaration "
-    r"of function [`']([^']*)'")
+    br"([^:]*):(\d+):(\d+:)? warning: implicit declaration "
+    br"of function [`']([^']*)'")
 pointer_pattern = re.compile(
-    r"([^:]*):(\d+):(\d+:)? warning: "
-    r"("
-     r"(assignment"
-     r"|initialization"
-     r"|return"
-     r"|passing arg \d+ of `[^']*'"
-     r"|passing arg \d+ of pointer to function"
-     r") makes pointer from integer without a cast"
-    r"|"
-    r"cast to pointer from integer of different size)")
+    br"([^:]*):(\d+):(\d+:)? warning: "
+    br"("
+     br"(assignment"
+     br"|initialization"
+     br"|return"
+     br"|passing arg \d+ of `[^']*'"
+     br"|passing arg \d+ of pointer to function"
+     br") makes pointer from integer without a cast"
+    br"|"
+    br"cast to pointer from integer of different size)")
 
 
 def filter_log(in_file, out_file, in_line=False):
-    last_implicit_filename = ""
+    last_implicit_filename = b""
     last_implicit_linenum = -1
-    last_implicit_func = ""
+    last_implicit_func = b""
 
     errlist = []
 
@@ -65,7 +64,7 @@ def filter_log(in_file, out_file, in_line=False):
         if in_line:
             out_file.write(line)
             out_file.flush()
-        if line == '':
+        if line == b'':
             break
         m = implicit_pattern.match(line)
         if m:
@@ -79,16 +78,18 @@ def filter_log(in_file, out_file, in_line=False):
                 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)
+                    err = (
+                        b"Function `%s' implicitly converted to pointer at "
+                        b"%s:%d" % (
+                            last_implicit_func, last_implicit_filename,
+                            last_implicit_linenum))
                     errlist.append(err)
-                    out_file.write(err + "\n")
+                    out_file.write(err + b"\n")
 
     if errlist:
         if in_line:
-            out_file.write("\n".join(errlist) + "\n\n")
-            out_file.write("""
+            out_file.write(b"\n".join(errlist) + b"\n\n")
+            out_file.write(b"""
 
 Our automated build log filter detected the problem(s) above that will
 likely cause your package to segfault on architectures where the size of
diff --git a/lpbuildd/tests/test_check_implicit_pointer_functions.py b/lpbuildd/tests/test_check_implicit_pointer_functions.py
index 582f1f6..8d604ec 100644
--- a/lpbuildd/tests/test_check_implicit_pointer_functions.py
+++ b/lpbuildd/tests/test_check_implicit_pointer_functions.py
@@ -1,9 +1,12 @@
 # Copyright 2011-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import io
+import os.path
 import re
+import subprocess
+import sys
 
-from six import StringIO
 from testtools import TestCase
 from testtools.matchers import MatchesRegex
 
@@ -20,88 +23,165 @@ class TestPointerCheckRegexes(TestCase):
         # 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")
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"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")
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94:7: "
+            b"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 don't include the
         # column number.
         line = (
-            "/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
-            "warning: implicit declaration of function 'foo'")
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"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'")
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94:7: "
+            b"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()
+        in_file = io.BytesIO(b"Innocuous build log\nwith no errors\n")
+        out_file = io.BytesIO()
         self.assertEqual(0, filter_log(in_file, out_file))
-        self.assertEqual("", out_file.getvalue())
+        self.assertEqual(b"", 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()
+        in_file = io.BytesIO(
+            b"Build log with errors\n"
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: implicit declaration of function 'foo'\n"
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: assignment makes pointer from integer without a cast\n"
+            b"More build log\n")
+        out_file = io.BytesIO()
         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",
+            b"Function `foo' implicitly converted to pointer at "
+            b"/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()
+        in_file = io.BytesIO(b"Innocuous build log\nwith no errors\n")
+        out_file = io.BytesIO()
         self.assertEqual(0, filter_log(in_file, out_file, in_line=True))
         self.assertEqual(
-            "Innocuous build log\nwith no errors\n", out_file.getvalue())
+            b"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()
+        in_file = io.BytesIO(
+            b"Build log with errors\n"
+            b"/build/gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: implicit declaration of function 'foo'\n"
+            b"/build/gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: assignment makes pointer from integer without a cast\n"
+            b"More build log\n")
+        out_file = io.BytesIO()
         self.assertEqual(1, filter_log(in_file, out_file, in_line=True))
         self.assertThat(out_file.getvalue(), MatchesRegex(
-            r"^" +
+            br"^" +
             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.*",
+                b"Build log with errors\n"
+                b"/build/gtk/ubuntumenuproxymodule.c:94: "
+                b"warning: implicit declaration of function 'foo'\n"
+                b"/build/gtk/ubuntumenuproxymodule.c:94: "
+                b"warning: assignment makes pointer from integer without a "
+                b"cast\n"
+                b"Function `foo' implicitly converted to pointer at "
+                b"/build/gtk/ubuntumenuproxymodule.c:94\n"
+                b"More build log\n"
+                b"Function `foo' implicitly converted to pointer at "
+                b"/build/gtk/ubuntumenuproxymodule.c:94\n\n\n\n") +
+            br"Our automated build log filter.*",
+            flags=re.M | re.S))
+
+
+class TestCheckImplicitPointerFunctionsScript(TestCase):
+
+    def setUp(self):
+        super(TestCheckImplicitPointerFunctionsScript, self).setUp()
+        top = os.path.dirname(
+            os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+        self.script = os.path.join(
+            top, "bin", "check-implicit-pointer-functions")
+
+    def test_out_of_line_no_errors(self):
+        in_bytes = b"Innocuous build log\nwith no errors\n\x80\x81\x82\n"
+        process = subprocess.Popen(
+            [sys.executable, self.script],
+            stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+        out_bytes, _ = process.communicate(in_bytes)
+        self.assertEqual(0, process.poll())
+        self.assertEqual(b"", out_bytes)
+
+    def test_out_of_line_errors(self):
+        in_bytes = (
+            b"Build log with errors\n"
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: implicit declaration of function 'foo'\n"
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: assignment makes pointer from integer without a cast\n"
+            b"\x80\x81\x82\n")
+        process = subprocess.Popen(
+            [sys.executable, self.script],
+            stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+        out_bytes, _ = process.communicate(in_bytes)
+        self.assertEqual(2, process.poll())
+        self.assertEqual(
+            b"Function `foo' implicitly converted to pointer at "
+            b"/build/buildd/gtk+3.0-3.0.0/./gtk/ubuntumenuproxymodule.c:94\n",
+            out_bytes)
+
+    def test_in_line_no_errors(self):
+        in_bytes = (b"Innocuous build log\nwith no errors\n\x80\x81\x82\n")
+        process = subprocess.Popen(
+            [sys.executable, self.script, "--inline"],
+            stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+        out_bytes, _ = process.communicate(in_bytes)
+        self.assertEqual(0, process.poll())
+        self.assertEqual(
+            b"Innocuous build log\nwith no errors\n\x80\x81\x82\n", out_bytes)
+
+    def test_in_line_errors(self):
+        in_bytes = (
+            b"Build log with errors\n"
+            b"/build/gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: implicit declaration of function 'foo'\n"
+            b"/build/gtk/ubuntumenuproxymodule.c:94: "
+            b"warning: assignment makes pointer from integer without a cast\n"
+            b"\x80\x81\x82\n")
+        process = subprocess.Popen(
+            [sys.executable, self.script, "--inline"],
+            stdin=subprocess.PIPE, stdout=subprocess.PIPE)
+        out_bytes, _ = process.communicate(in_bytes)
+        self.assertEqual(2, process.poll())
+        self.assertThat(out_bytes, MatchesRegex(
+            br"^" +
+            re.escape(
+                b"Build log with errors\n"
+                b"/build/gtk/ubuntumenuproxymodule.c:94: "
+                b"warning: implicit declaration of function 'foo'\n"
+                b"/build/gtk/ubuntumenuproxymodule.c:94: "
+                b"warning: assignment makes pointer from integer without a "
+                b"cast\n"
+                b"Function `foo' implicitly converted to pointer at "
+                b"/build/gtk/ubuntumenuproxymodule.c:94\n"
+                b"\x80\x81\x82\n"
+                b"Function `foo' implicitly converted to pointer at "
+                b"/build/gtk/ubuntumenuproxymodule.c:94\n\n\n\n") +
+            br"Our automated build log filter.*",
             flags=re.M | re.S))