-
Notifications
You must be signed in to change notification settings - Fork 704
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
Linking with system RocksDB #310
Comments
Related to this, it'd be great if these environment variables where documented in the README and the docs! |
This would be really useful! |
The build file for |
I'm working on this |
@lovesegfault How's your progress on this? Building rocksdb is by far the biggest part of my project's build time at the moment 😢 |
What is a problem to use system library now? AFAIK, it works via pkg-config or env variable well. |
@aleksuss Well, on current $ ls /usr/lib/librocksdb.so*
/usr/lib/librocksdb.so /usr/lib/librocksdb.so.6 /usr/lib/librocksdb.so.6.5.3 |
I had stopped working on this because other things took priority in the office. I'm going on vacation for the next ~2 weeks, so hopefully I can pick this back up :) |
Note that #354 doesn’t add any library discovery magic. Mostly because RocksDB doesn’t support pkg-config or any other similar tool. In the future a simple clause to that looks for the libraries in the usual places in case the corresponding envvar isn’t set could easily be added. |
Ah, I see, so if the crate currently fails to find a system-wide RocksDB, that will continue to be the case. I wonder what's making it fail. Maybe it looks for exactly the same version of the |
Well, the current crate will attempt to find a system rocksdb and vendor if it can't. #354 means you select, through a feature, whether to vendor or not. If you don't vendor, then you must set I can add a follow up that attempts to find the lib if the envvar isn't set, I'm just generally not a fan of that behavior. |
@lovesegfault Not sure I'm reading that correctly? So, you're saying that currently, if I'm curious -- why is it you would be against using the system library by default? Vendoring brings huge compile times, and I'm not sure what the upside is? |
@jonhoo New behavior If the Possible future behavior |
Doing this with a feature is a little odd, because it means that if any crate in your dependency tree includes |
I agree with you -- the solution is to not vendor. At the very least, vendoring should not be in the default feature set. One option is to have vendoring be opt-in using an environment variable. |
That could be done, @aleksuss @vitvakatu what are you guys' thoughts? |
Features are indeed hard to control in case of transitive dependencies, and compiling RocksDB from source costs a lot (for our project, it's about 30% increase of the build time!). However, I don't like the idea of complete removing of vendoring. On some platforms we do not have newest rocksdb version available: we were even forced to make our own PPA for older Ubuntu distributions. So, I'd prefer to save the current behavior, but maybe improve it (automatic libraries search?). Perhaps we should force user to manually enable vendoring (via env var), but it would be a bit annoying for transitive dependencies as well.
This also looks pretty weird and probably should be fixed. |
@vitvakatu Yeah, I think "inverting" the behavior so that users opt-in to vendoring is probably the right way to go :)
Note that I do not have any env vars set, so I believe this is intended behavior per today? |
Alright, I inverted the behavior :) |
@lovesegfault I think if we are inverting the behavior, then it's probably also necessary to have |
Alright, so pending work is::
|
* Fix titan config (rust-rocksdb#306) * Add titan read option (rust-rocksdb#309) Signed-off-by: Connor1996 <zbk602423539@gmail.com>
I'm using Having to specify the environment variables everywhere (IDE, command line, etc.) isn't great (as mentioned). My workaround is to define those variables in .cargo/config.toml: [env]
ROCKSDB_LIB_DIR = "/usr/lib/"
SNAPPY_LIB_DIR = "/usr/lib/" Now every Cargo instance that runs within the project directory has |
#166 introduced the ability to link with an already-built instance of rocksdb (such as one installed system-wide). However, it requires that you explicitly give the path to search in (
ROCKSDB_LIB_DIR
,SNAPPY_LIB_DIR
, etc.). It'd be really neat ifrocksdb-sys
checked ifrocksdb
andsnappy
were available in the standard OS locations too (originally suggested in #166 (comment)). It's a little unfortunate to have to pass custom environment variables every time we build even though those libraries are installed system-wide.Alternatively, even just a
ROCKSDB_USE_SYSTEM=1
would be a step up.The text was updated successfully, but these errors were encountered: