On Mon, Feb 18, 2019 at 06:46:15PM +0100, Lumir Balhar wrote: > Hello Simon. > > On 2/18/19 4:42 PM, Simon Glass wrote: > > Hi Lumir, > > > > On Mon, 18 Feb 2019 at 04:13, Lumir Balhar <lbalhar@xxxxxxxxxx> wrote: > > > Hello. > > > > > > Could please anybody take a look on these patches? Should I change > > > anything? From my point of view, these patches are finished and ready > > > for merge since November. > > > > > > I did a rebase and I am attaching a fresh copy of my patches. > > Is it possible to send these as patches (using patman or git > > send-email) instead of attachments? > > It is and I did it multiple times. I can try it again but I am afraid that > it'll open a new thread. Nothing wrong with opening a new thread - that's normal practice with reposting a series for dtc amongst other projects. I see that you did repost and I've now merged the patches. Sorry I let the get held up so long. Unfortunately I sometimes have trouble finding time for dtc work. > > We'll see. > > Have a nice day. > > Lumír > > > > > Regards, > > Simon > > > > > Thank you. > > > > > > Lumír > > > > > > On 12/11/18 9:35 AM, Lumir Balhar wrote: > > > > On 12/3/18 8:33 AM, David Gibson wrote: > > > > > On Mon, Nov 26, 2018 at 02:56:04PM +0100, Lumir Balhar wrote: > > > > > > 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 > > > > > > ********** > > > > > Right. The trick is we need to work out how to either not make the > > > > > tests depend on the extension module being in that exact file, or > > > > > automatically move it as part of the build, and we need to do so in a > > > > > way that's robust across Python revisions. > > > > > > > > > > > The question is whether we need to support dual stack with both > > > > > > Pythons in > > > > > > the same time or not. > > > > > I'd certainly prefer it, but one at a time is better than nothing. > > > > > > > > > > > If yes, we have to find a way how to store both built modules and > > > > > > pick the > > > > > > right one for tests. > > > > > Exactly. In fact I think if we don't override the destination > > > > > setuptools will already put the modules in different locations. The > > > > > trick is working out how to make our tests link against the right one > > > > > at runtime. > > > > I rebased all changes to the top of current master and attaching all > > > > patches again. > > > > > > > > I did one more small change in run_tests.sh file where Python module > > > > is now recognized for both Python versions. > > > > > > > > So now, you can run all tests without any manual work. > > > > > > > > $ git clean -fdx > > > > $ make check > > > > … > > > > OK > > > > ********** TEST SUMMARY > > > > * Total testcases: 1880 > > > > * PASS: 1880 > > > > * FAIL: 0 > > > > * Bad configuration: 0 > > > > * Strange test result: 0 > > > > ********** > > > > > > > > $ git clean -fdx > > > > $ PYTHON=python3 make check > > > > … > > > > OK > > > > ********** TEST SUMMARY > > > > * Total testcases: 1880 > > > > * PASS: 1880 > > > > * FAIL: 0 > > > > * Bad configuration: 0 > > > > * Strange test result: 0 > > > > ********** > > > > > > > > You can also have dual stack support but it needs Python 3 version to > > > > be built first. Otherwise, when you build Python 2 extension first, > > > > build of Python 3 extension is skipped because _libfdt.so file is > > > > detected so make skips the build. The other way around, it works > > > > because Python 3 produces .so file with a platform suffix in its name > > > > and therefore it's not detected by make. > > > > > > > > Is this sufficient? > > > > > > > > Have a nice day. Lumír > > > > > > > > > > If no, we can just rename it automatically after every build to the > > > > > > standard > > > > > > name _libfdt.so > > > > > Right, but while that's easy to do by hand, working out how to > > > > > reliably do it automatically isn't quite so obvious. > > > > > > > > > > > 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 > > > > > > >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""" > > > > > > >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 > > > > > > >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')) > -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
