Hello.
Could we please finish this?
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.
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
**********
Thank you and have a nice day.
Lumír
On 8/8/18 9:11 AM, Simon Glass wrote:
Hi Lumir,
On 7 August 2018 at 13:33, Lumir Balhar <lbalhar@xxxxxxxxxx> wrote:
Hello.
On 08/07/2018 07:07 PM, Simon Glass wrote:
Hi Lumir,
On 6 August 2018 at 00:24, Lumir Balhar <lbalhar@xxxxxxxxxx> wrote:
Hello.
On 07/16/2018 01:33 AM, Simon Glass wrote:
Hi,
On 12 July 2018 at 18:54, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx>
wrote:
On Thu, Jul 12, 2018 at 04:10:41PM +0200, frenzy@xxxxxxxxx wrote:
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Python 2 is still the default but it can be changed by
setting environment variable PYTHON before build/test.
Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
As I noted on an earlier version, I'd like to get Simon Glass's review
before merging these, since the Python bindings are his work.
However, a few preliminary comments from me.
---
Makefile | 3 ++-
pylibfdt/Makefile.pylibfdt | 4 ++--
tests/Makefile.tests | 6 +++---
tests/run_tests.sh | 2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 6d55e13..c84eb66 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,7 @@ BISON = bison
LEX = flex
SWIG = swig
PKG_CONFIG ?= pkg-config
+PYTHON ?= python2
INSTALL = /usr/bin/install
INSTALL_PROGRAM = $(INSTALL)
@@ -133,7 +134,7 @@ all: $(BIN) libfdt
# We need both Python and swig to build/install pylibfdt.
# This builds the given make ${target} if those deps are found.
check_python_deps = \
- if $(PKG_CONFIG) --cflags python2 >/dev/null 2>&1; then \
+ if $(PKG_CONFIG) --cflags $(PYTHON) >/dev/null 2>&1; then \
if which swig >/dev/null 2>&1; then \
can_build=yes; \
fi; \
diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 9507d3d..a05984c 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -8,13 +8,13 @@ PYMODULE = $(PYLIBFDT_objdir)/_libfdt.so
define run_setup
SOURCES="$(1)" CPPFLAGS="$(CPPFLAGS)" OBJDIR="$(PYLIBFDT_objdir)"
VERSION="$(dtc_version)"
- $(PYLIBFDT_objdir)/setup.py --quiet $(2)
+ $(PYTHON) $(PYLIBFDT_objdir)/setup.py --quiet $(2)
endef
Using the environment variable to select the default python for
install is fine, but..
$(PYMODULE): $(PYLIBFDT_srcs)
@$(VECHO) PYMOD $@
$(call run_setup, $^, build_ext --inplace)
- mv _libfdt.so $@
+ mv _libfdt.*so $@
install_pylibfdt: $(PYMODULE)
$(VECHO) INSTALL-PYLIB; \
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 2c2c4fd..554c840 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -76,13 +76,13 @@ tests_clean:
rm -f $(TESTS_CLEANFILES)
check: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
- cd $(TESTS_PREFIX); ./run_tests.sh
+ cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh
checkm: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
- cd $(TESTS_PREFIX); ./run_tests.sh -m 2>&1 | tee vglog.$$$$
+ cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh -m 2>&1 | tee
vglog.$$$$
..for testing, I'd be much happied if we ran the pylibfdt tests under
*both* Python2 and Python3 if available. Makes it much likelier that
we'll catch breakages for one or the other early.
I definitely agree with that, it is pretty important.
I am not sure how this can be done without breaking backward compatibility.
I have to say that I am not dtc nor libfdt expert so I might be missing
something.
AFAIK it's not possible to build the extension module for both Pythons in
the same time. In Makefile.pylibfdt on line 17, there is a `mv` command
which renames built extension to standard name pylibfdt/_libfdt.so. When we
build it again with a different Python version, the previous one will be
replaced here.
I can try to prepare some code which will handle whether use Python 2 or
Python 3 (or both) for testing based on extension file name but it will mean
that we cannot rename built extension file.
It should be possible to build the modules one after the other,
though, which is what we are asking.
In other words, can it handle both Python 2 and 3?
I think that yes. We can change the move command on line 17 of Makefile.pylibfdt from
mv _libfdt.*so $@
to
mv _libfdt.*so $(PYLIBFDT_objdir)/
which causes that Python 2 and Python 3 build results will have a different names. For example
$ ls -1 pylibfdt/*.so
pylibfdt/_libfdt.cpython-36m-x86_64-linux-gnu.so
pylibfdt/_libfdt.so
Which is completely correct and it works for both Pythons but I was afraid that the module name is for some reason important for you and I cannot change it.
When the modules have different names, we can automatically use the proper Python version for tests and if both modules will be available, we can run tests twice.
Is that the correct description of what you want to implement?
I think that is correct. The libfdt.py name is important, but I
presume that the .so will be found automatically regardless of its
name?
David, any more thoughts on this one?
Regards,
Simon
>From e312928b98ded94ab7e5642e9a5d510a93854e07 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Tue, 10 Jul 2018 09:08:07 +0200
Subject: [PATCH 5/5] 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 23e6176..176863a 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -939,7 +939,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 5fafdbfdb4a3a84f068c4c42427d8c548853b751 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 14:59:30 +0200
Subject: [PATCH 4/5] 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 f87e6b2..dd8d7a0 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 970d6acec7e118ac240bca50d99e04d15f62f568 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 12:41:13 +0200
Subject: [PATCH 3/5] 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 0dc86904544d31bc19b375d8a15146017cd701d0 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 12:40:09 +0200
Subject: [PATCH 2/5] 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 | 16 ++++++++--------
2 files changed, 19 insertions(+), 11 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 fbb1ce2..f87e6b2 100644
--- a/tests/pylibfdt_tests.py
+++ b/tests/pylibfdt_tests.py
@@ -69,7 +69,7 @@ TEST_ADDR_2H = 0
TEST_ADDR_2L = 123456789
TEST_ADDR_2 = (TEST_ADDR_2H << 32) | TEST_ADDR_2L
TEST_SIZE_2H = 0
-TEST_SIZE_2L = 010000
+TEST_SIZE_2L = 0x10000
TEST_SIZE_2 = (TEST_SIZE_2H << 32) | TEST_SIZE_2L
TEST_VALUE_1 = 0xdeadbeef
@@ -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"""
@@ -422,7 +422,7 @@ class PyLibfdtBasicTests(unittest.TestCase):
self.assertEquals(2, self.fdt.num_mem_rsv())
self.assertEquals([ 0xdeadbeef00000000, 0x100000],
self.fdt.get_mem_rsv(0))
- self.assertEquals([123456789, 010000], self.fdt.get_mem_rsv(1))
+ self.assertEquals([123456789, 0o10000], self.fdt.get_mem_rsv(1))
def testEmpty(self):
"""Test that we can create an empty tree"""
--
2.19.1
>From c91134d2cbdec5b7f448475f06896568fe5f0df2 Mon Sep 17 00:00:00 2001
From: Lumir Balhar <lbalhar@xxxxxxxxxx>
Date: Mon, 9 Jul 2018 12:38:59 +0200
Subject: [PATCH 1/5] pylibfdt: Allow switch to Python 3 via environment
variable PYTHON
Python 2 is still the default but it can be changed by
setting environment variable PYTHON before build/test.
Signed-off-by: Lumir Balhar <lbalhar@xxxxxxxxxx>
---
Makefile | 3 ++-
pylibfdt/Makefile.pylibfdt | 4 ++--
tests/Makefile.tests | 6 +++---
tests/run_tests.sh | 2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Makefile b/Makefile
index 7a472b7..d9495f0 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,7 @@ BISON = bison
LEX = flex
SWIG = swig
PKG_CONFIG ?= pkg-config
+PYTHON ?= python2
INSTALL = /usr/bin/install
INSTALL_PROGRAM = $(INSTALL)
@@ -147,7 +148,7 @@ all: $(BIN) libfdt
# We need both Python and swig to build/install pylibfdt.
# This builds the given make ${target} if those deps are found.
check_python_deps = \
- if $(PKG_CONFIG) --cflags python2 >/dev/null 2>&1; then \
+ if $(PKG_CONFIG) --cflags $(PYTHON) >/dev/null 2>&1; then \
if which swig >/dev/null 2>&1; then \
can_build=yes; \
fi; \
diff --git a/pylibfdt/Makefile.pylibfdt b/pylibfdt/Makefile.pylibfdt
index 8f00b3d..f6abe02 100644
--- a/pylibfdt/Makefile.pylibfdt
+++ b/pylibfdt/Makefile.pylibfdt
@@ -13,10 +13,10 @@ endif
$(PYMODULE): $(PYLIBFDT_srcs) $(LIBFDT_archive) $(SETUP) $(VERSION_FILE)
@$(VECHO) PYMOD $@
- $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_objdir)
+ $(PYTHON) $(SETUP) $(SETUPFLAGS) build_ext --build-lib=../$(PYLIBFDT_objdir)
install_pylibfdt: $(PYMODULE)
@$(VECHO) INSTALL-PYLIB
- $(SETUP) $(SETUPFLAGS) install $(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX))
+ $(PYTHON) $(SETUP) $(SETUPFLAGS) install $(if $(SETUP_PREFIX),--prefix=$(SETUP_PREFIX))
PYLIBFDT_cleanfiles = libfdt_wrap.c libfdt.py libfdt.pyc _libfdt.so
diff --git a/tests/Makefile.tests b/tests/Makefile.tests
index 6ddf444..8b63f60 100644
--- a/tests/Makefile.tests
+++ b/tests/Makefile.tests
@@ -81,18 +81,18 @@ tests_clean:
rm -rf $(TESTS_CLEANDIRS)
check: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
- cd $(TESTS_PREFIX); ./run_tests.sh
+ cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh
ifeq ($(NO_VALGRIND),1)
checkm:
@echo "make checkm requires valgrind, but NO_VALGRIND=1"
else
checkm: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
- cd $(TESTS_PREFIX); ./run_tests.sh -m
+ cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh -m
endif
checkv: tests ${TESTS_BIN} $(TESTS_PYLIBFDT)
- cd $(TESTS_PREFIX); ./run_tests.sh -v
+ cd $(TESTS_PREFIX); PYTHON=$(PYTHON) ./run_tests.sh -v
ifneq ($(DEPTARGETS),)
-include $(TESTS_DEPFILES)
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 547aedc..23e6176 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -933,7 +933,7 @@ fdtoverlay_tests() {
pylibfdt_tests () {
run_dtc_test -I dts -O dtb -o test_props.dtb test_props.dts
TMP=/tmp/tests.stderr.$$
- python pylibfdt_tests.py -v 2> $TMP
+ $PYTHON pylibfdt_tests.py -v 2> $TMP
# Use the 'ok' message meaning the test passed, 'ERROR' meaning it failed
# and the summary line for total tests (e.g. 'Ran 17 tests in 0.002s').
--
2.19.1
