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

update to LMDB 0.9.33 and fix Android #34

Closed
wants to merge 2 commits into from

Conversation

fiatjaf
Copy link

@fiatjaf fiatjaf commented Dec 16, 2024

I started this branch because https://github.com/LMDB/lmdb has the variable named __ANDROID__ instead of ANDROID, which in my experience is required in order to compile this with NDK/gomobile.

So instead of just changing that I thought I would update the entire LMDB code in order to get other bug fixes and improvements they could have.

But then when I went to fetch v0.9.33 from https://git.openldap.org/openldap/openldap/-/tree/master/libraries/liblmdb?ref_type=heads then it weirdly didn't have the __ANDROID__ variable, so I ended up modifying it myself in ad-hoc fashion.


In short: what I really want is the second commit, but I include the first here because it may interest you. If it doesn't let me know if you would merge the second so I can make a PR with just that.

@wojas
Copy link
Member

wojas commented Dec 20, 2024

Hi, thank you for your contribution.

I do not see any newer version tag at https://github.com/LMDB/lmdb/tags, which we use in our update-lmdb.sh script, which is how we upgraded to newer versions until now. The OpenLDAP repo you linked to does appear to be the authoritative source for this, so we probably need to switch to that one. I do however want to see if we can update our script for this, and perform the actual upgrade ourselves to ensure that we mirror the actual upstream version without changes.

We do not want to patch the upstream files, unless there is really no other way. Is it possible to solve your issues without modifying the upstream code? If not, please explain why and what is going wrong in a new issue so that we can find the best solution.

@wojas
Copy link
Member

wojas commented Dec 20, 2024

I opened #35 for the upgrade to 0.9.33 and a switch to the official repo in our update script.

No ideas/action on the Android issue yet.

@fiatjaf
Copy link
Author

fiatjaf commented Dec 20, 2024

Ok, that's understandable. Thank you.

I don't know what is happening either, I just see that on GitHub they have a build variable called __ANDROID__ while on the other repo the same variable is called ANDROID -- and apparently the custom android NDK Clang compiler wants it to be called __ANDROID__.

I can probably live with just making my own fork and renaming that for a while.

@fiatjaf fiatjaf closed this Dec 20, 2024
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.

2 participants