Re: [PATCH 1/5] pylibfdt: Allow switch to Python 3 via environment variable PYTHON

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On 11/25/18 11:27 AM, David Gibson wrote:
On Tue, Nov 20, 2018 at 01:13:48PM +0100, Lumir Balhar wrote:
Hello.

Could we please finish this?
Yes, sorry.  I did some preliminary changes aiming towards
simultaneous testing of Python2 and Python3, but then got side tracked
before completing it.
No worries.
I did a rebase of all my patches to the top of current master branch. My py3
branch is available here: https://github.com/frenzymadness/dtc/ and all
patches are attached.
Thanks.

When I test it with Python 2 or Python 3, everything works. Python version
has to be specified by environment variable PYTHON in order to change the
default (python2).


$ make check
      DEP tests/dumptrees.c
      DEP tests/trees.S
      DEP tests/testutils.c
      DEP tests/value-labels.c
…
Basic sanity check for the FdtRo class ... ok
testCreate (__main__.PyLibfdtSwTests) ... ok

----------------------------------------------------------------------
Ran 35 tests in 0.004s

OK
********** TEST SUMMARY
*     Total testcases:    1826
*                PASS:    1826
*                FAIL:    0
*   Bad configuration:    0
* Strange test result:    0
**********

$ PYTHON=python3 make check
      DEP tests/dumptrees.c
      DEP tests/trees.S
      DEP tests/testutils.c
      DEP tests/value-labels.c
…
********** TEST SUMMARY
*     Total testcases:    1790
*                PASS:    1790
*                FAIL:    0
*   Bad configuration:    0
* Strange test result:    0
**********
So, the reason many less tests are run in this case is that the
Python3 setup.py uses a different name for the extension moule .so.
That means that the test scripts think there isn't Python support and
doesn't run those tests.  We need to at least fix that before this is
ready to go.

That's a great point! In my latest version I am able to run all tests under both Python 2 and 3. However, Python 3 module has to be renamed after build.

With Python 2:

$ make check
…
********** TEST SUMMARY
*     Total testcases:    1880
*                PASS:    1880
*                FAIL:    0
*   Bad configuration:    0
* Strange test result:    0
**********

WIth Python 3:

$ PYTHON=python3 make
$ mv pylibfdt/_libfdt{.cpython-37m-x86_64-linux-gnu,}.so
$ PYTHON=python3 make check
…
********** TEST SUMMARY
*     Total testcases:    1880
*                PASS:    1880
*                FAIL:    0
*   Bad configuration:    0
* Strange test result:    0
**********

The question is whether we need to support dual stack with both Pythons in the same time or not.

If yes, we have to find a way how to store both built modules and pick the right one for tests.

If no, we can just rename it automatically after every build to the standard name _libfdt.so

What is the preferred way?


I've merged some bits and pieces from your series, and I've made a few
other changes to get this closer.  I hope this time I can actually get
to the rest soon.
Great, thanks. I did some more small changes, rebased and I am attaching all patches again (except the one already merged, of course).

>From 064a470664f8ac78162033602e7621b0d207618e Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Tue, 10 Jul 2018 09:08:07 +0200
Subject: [PATCH 4/4] pylibfdt: Change how passing tests are recognized

When some warning appears in test result, "ok" is still
at the end of the line but without three dots.

Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
---
 tests/run_tests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index ca3fc86..1fa14ff 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -940,7 +940,7 @@ pylibfdt_tests () {
     # and the summary line for total tests (e.g. 'Ran 17 tests in 0.002s').
     # We could add pass + fail to get total tests, but this provides a useful
     # sanity check.
-    pass_count=$(grep "\.\.\. ok$" $TMP | wc -l)
+    pass_count=$(grep "ok$" $TMP | wc -l)
     fail_count=$(grep "^ERROR: " $TMP | wc -l)
     total_tests=$(sed -n 's/^Ran \([0-9]*\) tests.*$/\1/p' $TMP)
     cat $TMP
-- 
2.19.1

>From 8a19d132ca0db7e10ec816a39ae660f728df9e9d Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 14:59:30 +0200
Subject: [PATCH 3/4] pylibfdt: Test fdt.setprop take bytes on Python 3, add
 error handling

Signed-off-by: Petr Viktorin <pviktori@xxxxxxxxxx>
---
 pylibfdt/libfdt.i       |  4 ++++
 tests/pylibfdt_tests.py | 12 +++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
index 084bc5b..4f14403 100644
--- a/pylibfdt/libfdt.i
+++ b/pylibfdt/libfdt.i
@@ -1084,6 +1084,10 @@ typedef uint32_t fdt32_t;
 /* typemap used for fdt_setprop() */
 %typemap(in) (const void *val) {
     %#if PY_VERSION_HEX >= 0x03000000
+        if (!PyBytes_Check($input)) {
+            SWIG_exception_fail(SWIG_TypeError, "bytes expected in method '" "$symname"
+                "', argument " "$argnum"" of type '" "$type""'");
+        }
         $1 = PyBytes_AsString($input);
     %#else
         $1 = PyString_AsString($input);   /* char *str */
diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
index 4761c52..e6c13ff 100644
--- a/tests/pylibfdt_tests.py
+++ b/tests/pylibfdt_tests.py
@@ -82,6 +82,8 @@ TEST_VALUE64_1 = (TEST_VALUE64_1H << 32) | TEST_VALUE64_1L
 PHANDLE_1 = 0x2000
 PHANDLE_2 = 0x2001
 
+TEST_BYTES_1 = b'hello world'
+
 TEST_STRING_1 = 'hello world'
 TEST_STRING_2 = 'hi world'
 TEST_STRING_3 = u'unicode \u01d3'
@@ -443,21 +445,21 @@ class PyLibfdtBasicTests(unittest.TestCase):
     def testSetProp(self):
         """Test that we can update and create properties"""
         node = self.fdt.path_offset('/subnode@1')
-        self.fdt.setprop(node, 'compatible', TEST_STRING_1)
-        self.assertEquals(TEST_STRING_1, self.fdt.getprop(node, 'compatible'))
+        self.fdt.setprop(node, 'compatible', TEST_BYTES_1)
+        self.assertEquals(TEST_BYTES_1, self.fdt.getprop(node, 'compatible'))
 
         # Check that this property is missing, and that we don't have space to
         # add it
         self.assertEquals(-libfdt.NOTFOUND,
                           self.fdt.getprop(node, 'missing', QUIET_NOTFOUND))
         self.assertEquals(-libfdt.NOSPACE,
-                          self.fdt.setprop(node, 'missing', TEST_STRING_1,
+                          self.fdt.setprop(node, 'missing', TEST_BYTES_1,
                                            quiet=(libfdt.NOSPACE,)))
 
         # Expand the device tree so we now have room
         self.fdt.resize(self.fdt.totalsize() + 50)
-        self.fdt.setprop(node, 'missing', TEST_STRING_1)
-        self.assertEquals(TEST_STRING_1, self.fdt.getprop(node, 'missing'))
+        self.fdt.setprop(node, 'missing', TEST_BYTES_1)
+        self.assertEquals(TEST_BYTES_1, self.fdt.getprop(node, 'missing'))
 
     def testSetPropU32(self):
         """Test that we can update and create integer properties"""
-- 
2.19.1

>From c96c95413f0432fe1bf36796d5c8027b36ac5b93 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 12:41:13 +0200
Subject: [PATCH 2/4] pylibfdt: check_err accepts only integer as a first
 argument.

A list passed as an argument to check_err() means that
there is no error code to check and therefore it should
be returned back.

Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
---
 pylibfdt/libfdt.i | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
index 6f1f1dc..084bc5b 100644
--- a/pylibfdt/libfdt.i
+++ b/pylibfdt/libfdt.i
@@ -137,7 +137,7 @@ def check_err(val, quiet=()):
     Raises
         FdtException if val < 0
     """
-    if val < 0:
+    if isinstance(val, int) and val < 0:
         if -val not in quiet:
             raise FdtException(val)
     return val
-- 
2.19.1

>From 751a34fee080c9aaca6033aad9501a849a1ab8a7 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 12:40:09 +0200
Subject: [PATCH 1/4] pylibfdt: Proper handling of bytes/unicode strings and
 octal literals

Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
---
 pylibfdt/libfdt.i       | 14 +++++++++++---
 tests/pylibfdt_tests.py | 20 ++++++++++----------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/pylibfdt/libfdt.i b/pylibfdt/libfdt.i
index 462b5b0..6f1f1dc 100644
--- a/pylibfdt/libfdt.i
+++ b/pylibfdt/libfdt.i
@@ -669,7 +669,7 @@ class Fdt(FdtRo):
         Raises:
             FdtException if no parent found or other error occurs
         """
-        val = val.encode('utf-8') + '\0'
+        val = val.encode('utf-8') + b'\0'
         return check_err(fdt_setprop(self._fdt, nodeoffset, prop_name,
                                      val, len(val)), quiet)
 
@@ -1074,12 +1074,20 @@ typedef uint32_t fdt32_t;
 	if (!$1)
 		$result = Py_None;
 	else
-		$result = Py_BuildValue("s#", $1, *arg4);
+        %#if PY_VERSION_HEX >= 0x03000000
+            $result = Py_BuildValue("y#", $1, *arg4);
+        %#else
+            $result = Py_BuildValue("s#", $1, *arg4);
+        %#endif
 }
 
 /* typemap used for fdt_setprop() */
 %typemap(in) (const void *val) {
-    $1 = PyString_AsString($input);   /* char *str */
+    %#if PY_VERSION_HEX >= 0x03000000
+        $1 = PyBytes_AsString($input);
+    %#else
+        $1 = PyString_AsString($input);   /* char *str */
+    %#endif
 }
 
 /* typemaps used for fdt_next_node() */
diff --git a/tests/pylibfdt_tests.py b/tests/pylibfdt_tests.py
index 34c680d..4761c52 100644
--- a/tests/pylibfdt_tests.py
+++ b/tests/pylibfdt_tests.py
@@ -84,7 +84,7 @@ PHANDLE_2 = 0x2001
 
 TEST_STRING_1 = 'hello world'
 TEST_STRING_2 = 'hi world'
-TEST_STRING_3 = u'unicode ' + unichr(467)
+TEST_STRING_3 = u'unicode \u01d3'
 
 
 def get_err(err_code):
@@ -107,7 +107,7 @@ def _ReadFdt(fname):
     Returns:
         Fdt bytearray suitable for passing to libfdt functions
     """
-    return libfdt.Fdt(open(fname).read())
+    return libfdt.Fdt(open(fname, mode='rb').read())
 
 class PyLibfdtBasicTests(unittest.TestCase):
     """Test class for basic pylibfdt access functions
@@ -164,7 +164,7 @@ class PyLibfdtBasicTests(unittest.TestCase):
     def testBadFdt(self):
         """Check that a filename provided accidentally is not accepted"""
         with self.assertRaises(FdtException) as e:
-            fdt = libfdt.Fdt('a string')
+            fdt = libfdt.Fdt(b'a string')
         self.assertEquals(e.exception.err, -libfdt.BADMAGIC)
 
     def testSubnodeOffset(self):
@@ -239,7 +239,7 @@ class PyLibfdtBasicTests(unittest.TestCase):
         poffset = self.fdt.first_property_offset(root)
         prop = self.fdt.get_property_by_offset(poffset)
         self.assertEquals(prop.name, 'compatible')
-        self.assertEquals(prop, 'test_tree1\0')
+        self.assertEquals(prop, b'test_tree1\0')
 
         with self.assertRaises(FdtException) as e:
             self.fdt.get_property_by_offset(-2)
@@ -252,7 +252,7 @@ class PyLibfdtBasicTests(unittest.TestCase):
         """Check that we can read the contents of a property by name"""
         root = self.fdt.path_offset('/')
         value = self.fdt.getprop(root, "compatible")
-        self.assertEquals(value, 'test_tree1\0')
+        self.assertEquals(value, b'test_tree1\0')
         self.assertEquals(-libfdt.NOTFOUND, self.fdt.getprop(root, 'missing',
                                                              QUIET_NOTFOUND))
 
@@ -262,7 +262,7 @@ class PyLibfdtBasicTests(unittest.TestCase):
 
         node = self.fdt.path_offset('/subnode@1/subsubnode')
         value = self.fdt.getprop(node, "compatible")
-        self.assertEquals(value, 'subsubnode1\0subsubnode\0')
+        self.assertEquals(value, b'subsubnode1\0subsubnode\0')
 
     def testStrError(self):
         """Check that we can get an error string"""
@@ -591,7 +591,7 @@ class PyLibfdtSwTests(unittest.TestCase):
 
         # Make sure we can read from the tree too
         node = sw.path_offset('/subnode@1')
-        self.assertEqual('subnode1' + chr(0), sw.getprop(node, 'compatible'))
+        self.assertEqual(b'subnode1\0', sw.getprop(node, 'compatible'))
 
         # Make sure we did at least two resizes
         self.assertTrue(len(fdt.as_bytearray()) > FdtSw.INC_SIZE * 2)
@@ -609,15 +609,15 @@ class PyLibfdtRoTests(unittest.TestCase):
 
     def setUp(self):
         """Read in the device tree we use for testing"""
-        self.fdt = libfdt.FdtRo(open('test_tree1.dtb').read())
+        self.fdt = libfdt.FdtRo(open('test_tree1.dtb', mode='rb').read())
 
     def testAccess(self):
         """Basic sanity check for the FdtRo class"""
         node = self.fdt.path_offset('/subnode@1')
-        self.assertEqual('subnode1' + chr(0),
+        self.assertEqual(b'subnode1\0',
                          self.fdt.getprop(node, 'compatible'))
         node = self.fdt.first_subnode(node)
-        self.assertEqual('this is a placeholder string\0string2\0',
+        self.assertEqual(b'this is a placeholder string\0string2\0',
                          self.fdt.getprop(node, 'placeholder'))
 
 
-- 
2.19.1


[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux