Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix python CI crash in mergin-py-client #184

Merged
merged 3 commits into from
Aug 5, 2022
Merged

Conversation

wonder-sk
Copy link
Contributor

In mergin-py-client we're getting segfaults in CI with the new geodiff release:

  File "/opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/site-packages/pygeodiff/geodifflib.py", line 542 in __init__
  File "/opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/site-packages/pygeodiff/geodifflib.py", line 501 in next_entry
  File "/opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/site-packages/pygeodiff/geodifflib.py", line 509 in __next__
  File "/home/runner/work/mergin-py-client/mergin-py-client/mergin/report.py", line 162 in changeset_report
  File "/home/runner/work/mergin-py-client/mergin-py-client/mergin/report.py", line 269 in create_report
  File "/home/runner/work/mergin-py-client/mergin-py-client/mergin/test/test_client.py", line 1684 in test_report

GDB further points to GEODIFF_CE_newValue() function.

0x00007ffff51382a0 in GEODIFF_CE_newValue ()
   from /opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/site-packages/pygeodiff/libpygeodiff-2.0.0-python.so
#0  0x00007ffff51382a0 in GEODIFF_CE_newValue ()
   from /opt/hostedtoolcache/Python/3.10.5/x64/lib/python3.10/site-packages/pygeodiff/libpygeodiff-2.0.0-python.so
#1  0x00007ffff5bffff5 in ?? () from /lib/x86_64-linux-gnu/libffi.so.7
#2  0x00007ffff5bff40a in ?? () from /lib/x86_64-linux-gnu/libffi.so.7

It looks like in geodifflib.py, few C functions didn't get arguments updated when contexts were added in #172 - with GEODIFF_CE_newValue being one of them. On my local machine there's no segfault, so I can't properly verify the fix (but there's nothing else looking suspicious there).

Also:

  • switched from nose2 to pytest for python tests so that we use the same tool everywhere and fixed few places where pytest complained about things
  • added tests for column_is_pkey in changeset reader test

@PeterPetrik PeterPetrik merged commit 465b16a into master Aug 5, 2022
@PeterPetrik PeterPetrik deleted the fix-python-ci-crash branch August 5, 2022 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants