dgibson / dtc

Device Tree Compiler
219 stars 130 forks source link

`testCreate`/`testReserveMap` fails with Python 3.12 #123

Closed sergiodj closed 3 months ago

sergiodj commented 8 months ago

Hi,

This bug is affecting both Debian and Ubuntu. I spent some time investigating it and I believe it's caused by some Python 3.12 change that's affecting SWIG, but I'm not entirely sure.

The relevant log snippet follows:

testCreate (__main__.PyLibfdtSwTests.testCreate) ... FAIL

======================================================================
FAIL: testReserveMap (__main__.PyLibfdtBasicTests.testReserveMap)
Test that we can access the memory reserve map
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/<<PKGBUILDDIR>>/tests/./pylibfdt_tests.py", line 421, in testReserveMap
    self.assertEqual([ 0xdeadbeef00000000, 0x100000],
AssertionError: Lists differ: [16045690981097406464, 1048576] != [0, 16045690981097406464, 1048576]

First differing element 0:
16045690981097406464
0

Second list contains 1 additional elements.
First extra element 2:
1048576

- [16045690981097406464, 1048576]
+ [0, 16045690981097406464, 1048576]
?  +++

======================================================================
FAIL: testCreate (__main__.PyLibfdtSwTests.testCreate)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/<<PKGBUILDDIR>>/tests/./pylibfdt_tests.py", line 585, in testCreate
    self.assertEqual([TEST_ADDR_1, TEST_SIZE_1], fdt.get_mem_rsv(0))
AssertionError: Lists differ: [9223372036854775808, 1048576] != [0, 9223372036854775808, 1048576]

First differing element 0:
9223372036854775808
0

Second list contains 1 additional elements.
First extra element 2:
1048576

- [9223372036854775808, 1048576]
+ [0, 9223372036854775808, 1048576]
?  +++

----------------------------------------------------------------------
Ran 39 tests in 0.004s

FAILED (failures=2)
********** TEST SUMMARY
*     Total testcases:  2100
*                PASS:  2098
*                FAIL:  0
*   Bad configuration:  0
* Strange test result:  0
**********
make[1]: *** [tests/Makefile.tests:89: check] Error 1

The relevant code here seems to be:

https://github.com/dgibson/dtc/blob/3fbfdd08afd2a7a25b27433f6f5678c0fe694721/pylibfdt/libfdt.i#L1189-L1197

When running with Python 3.11, if (PyTuple_GET_SIZE(resultobj) == 0) is true the first time we reach this point, which makes the list have the right number of elements. However, on Python 3.12 that check is never true (possibly because resultobj has been initialized before and holds one element, but I couldn't find more details), which makes the final list have 0 at the beginning.

dgibson commented 8 months ago

I hit this on Fedora as well, and it's been there for a while. It seems to be an incompatible change in swig, or maybe in Python, or in the combination of the two somehow. Unfortunately, I don't know Python FFI well enough to fix it, and I don't really have time to learn. I've been hoping someone Python knowledgable would fix this :(.

dgibson commented 8 months ago

Fwiw, some more information here

sergiodj commented 8 months ago

Thanks, @dgibson.

To be clear, did you disable these specific tests on Fedora for now? I'm considering doing the same in Ubuntu/Debian.

zumbi commented 8 months ago

FWIW I tried the testcases with Python3.12 with newer swig 4.2.0 that claims to have support for py3.12 and it fails the same way.

dgibson commented 8 months ago

@sergiodj , sorry, I didn't mean the Fedora package of dtc - I'm not involved with that packaging. I meant that the upstream tests also fail running on my Fedora 39 system.

ncopa commented 6 months ago

This has now become a blocker for python 3.12 for alpine. Does anybody have a fix or workaround?

Fix or workaround from Debian: https://sources.debian.org/patches/device-tree-compiler/1.7.0-2/fix-tests-for-Python3.12.patch/

sergiodj commented 6 months ago

Unfortunately not. I haven't had the time to continue debugging the issue, and meanwhile decided to skip the specific test.

blmaier commented 3 months ago

@sergiodj this should be fixed by https://github.com/dgibson/dtc/commit/822123856980f84562406cc7bd1d4d6c2b8bc184

sergiodj commented 3 months ago

Thanks, @blmaier!

dgibson commented 3 months ago

@sergiodj can you confirm that @blmaier 's commit fixes the problem? If so we can close the issue.

sergiodj commented 3 months ago

@dgibson I did a local build of the device-tree-compiler Debian package here with @blmaier's patch backported, and it works fine. Cc @zumbi

dgibson commented 3 months ago

@sergiodj thanks for the confirmation.