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

Build and tests fail on MINGW platform #342

Open
thomasfire opened this issue May 3, 2023 · 7 comments
Open

Build and tests fail on MINGW platform #342

thomasfire opened this issue May 3, 2023 · 7 comments

Comments

@thomasfire
Copy link

Affected Operating Systems

  • Windows - MSYS2/MINGW64

Affected py-lmdb Version

1.4.1

py-lmdb Installation Method

See PKGBUILD file

Using bundled or distribution-provided LMDB library?

Bundled

Distribution name and LMDB library version

Both built from this package and using latest available from system.

Describe Your Problem

Build fails as setup.py script assumes that everything built on windows is built with MSVC, which is not true in case of MINGW setup.
env_test.py tests segfault on opening temp files.
See patch file for details and potential solution.

Related issues

Affects msys2/MINGW-packages#17025

Potential solution

See the patch above. I would suggest adding compilations flags specifically for mingw compilers on windows and either skip or fix segfault in tests.

@sbourdeauducq
Copy link

either skip or fix segfault in tests.

Why would skipping be a good idea at all?

@thomasfire
Copy link
Author

Why would skipping be a good idea at all?

If the temporary files are not supported by mingw for example (I am not sure though, it looks like it would work fine with MSVC), then probably it would be better to skip and mark as not supported features

@sbourdeauducq
Copy link

Why would temporary files not being supported cause a segfault?

@thomasfire
Copy link
Author

thomasfire commented May 10, 2023

Well the true issue appears to be in these lines:
https://github.com/LMDB/lmdb/blob/3947014aed7ffe39a79991fa7fb5b234da47ad1a/libraries/liblmdb/mdb.c#L1737-L1745
The same code is in this repo.
Looks like I'll need to change C API a bit to handle these strings properly

@sbourdeauducq
Copy link

Haha this is such garbage code. Can you just do something like static char buf[MSGSIZE]? It's not thread-safe, but I'm not sure if the current code is any better...

@sbourdeauducq
Copy link

And Python doesn't have real threads due to the GIL so it probably doesn't matter here.

@thomasfire
Copy link
Author

Updated patch with segfault fix: msys2/MINGW-packages#17095

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

No branches or pull requests

2 participants