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

Remove the vendored feature #141

Closed
Kerollmops opened this issue Dec 24, 2022 · 4 comments · Fixed by #128
Closed

Remove the vendored feature #141

Kerollmops opened this issue Dec 24, 2022 · 4 comments · Fixed by #128
Milestone

Comments

@Kerollmops
Copy link
Member

Kerollmops commented Dec 24, 2022

We should consider removing the vendored feature from heed as it can bring more issues than help. I would like to remove the possibility of being able to use another version of LMDB than the one provided by heed.

@GregoryConrad
Copy link
Contributor

Hi, how would this work if removed? What issues is vendored currently creating? If I am not mistaken, removing vendored would require all consumers of heed to have liblmdb installed locally on their system, binaries would not be portable, and would disallow custom definitions in LMDB (see #140). (At least based on my understanding, if I am wrong, please do correct me.)

I think the real solution (for a lot of different projects) would be a Rust-idiomatic rewrite of LMDB in pure Rust, which, based on the issues I have had to battle LMDB with over the past few weeks, I have seriously considered. I think LMDB is fantastic software on its own, and frankly wouldn't try to compete with it; however, a pure-Rust solution would simply be the way to go even if it has ever-so slightly slower performance. The heart of lmdb, mdb.c, is only ~11k lines of C. I'm surprised nobody has tried a rewrite already (at least none that I am aware of).

@Kerollmops
Copy link
Member Author

Kerollmops commented Jan 4, 2023

Hi, sorry for the confusion. I updated the original message. I want to remove the vendored feature and never search for a library available on the system. I prefer that heed always comes with the right provided version of LMDB.

I think the real solution (for a lot of different projects) would be a Rust-idiomatic rewrite of LMDB in pure Rust, which, based on the issues I have had to battle LMDB with over the past few weeks, I have seriously considered. I think LMDB is fantastic software on its own, and frankly wouldn't try to compete with it; however, a pure-Rust solution would simply be the way to go even if it has ever-so slightly slower performance. The heart of lmdb, mdb.c, is only ~11k lines of C. I'm surprised nobody has tried a rewrite already (at least none that I am aware of).

There already are a lot of solutions made in pure Rust. I also took a little bit of my spare time to port LMDB to rust by using c2rust which currently only compiles on Linux.

@GregoryConrad
Copy link
Contributor

GregoryConrad commented Jan 4, 2023

Ah! Yes, I think it could be wise to remove vendored then, especially for the case of heed & milli which shouldn’t be using any system LMDB to begin with (because you need your automatic page-freeing customizations that don’t come standard in LMDB). It’ll also ensure that any executables made with heed are portable.

There already are a lot of solutions made in pure Rust. I also took a little bit of my spare time to port LMDB to rust by using c2rust which currently only compiles on Linux.

Those are some great resources; thanks!

@Kerollmops
Copy link
Member Author

[...] especially for the case of heed & milli which shouldn’t be using any system LMDB to begin with (because you need your automatic page-freeing customizations that don’t come standard in LMDB).

We will remove that in the next version of heed and directly put a submodule in the heed repo to reduce the burden of bumping LMDB when patches are released.

@Kerollmops Kerollmops added this to the v0.20.0 milestone Jan 11, 2023
@Kerollmops Kerollmops linked a pull request Jan 11, 2023 that will close this issue
20 tasks
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 a pull request may close this issue.

2 participants