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

Rework build.rs and support linking to system RocksDB #354

Open
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

lovesegfault
Copy link

@lovesegfault lovesegfault commented Nov 21, 2019

This PR completely reworks librocksdb-sys's build system. Here are the main achievements:

  • Complete separation of vendoring and system linking logic
  • Feature-based selection of linking mode (static or dynamic)
  • Feature-based selection of vendoring (defaults to on)
  • Helpful error messages with panics
  • Simplifies code where reasonable

With this I hope our vendoring and linking stories become clearer.

cc. @jonhoo

Fixes #310

@vitvakatu vitvakatu self-requested a review November 21, 2019 08:58
@lovesegfault lovesegfault marked this pull request as ready for review February 20, 2020 01:35
@jonhoo
Copy link
Contributor

jonhoo commented Feb 20, 2020

This is awesome stuff @lovesegfault!

librocksdb-sys/Cargo.toml Outdated Show resolved Hide resolved
Copy link

@vitvakatu vitvakatu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, looks good in general. Could you document vendor feature somewhere? Ideally, in the crate docs, so it would be visible on docs.rs

librocksdb-sys/build/system.rs Outdated Show resolved Hide resolved
librocksdb-sys/build/system.rs Show resolved Hide resolved
librocksdb-sys/build/vendor.rs Outdated Show resolved Hide resolved
BusyJay pushed a commit to BusyJay/rust-rocksdb that referenced this pull request Jul 23, 2020
)

The option is useful for catching LSM tree corruption. Adding it.

Signed-off-by: Yi Wu <yiwu@pingcap.com>
@grtlr
Copy link

grtlr commented Oct 22, 2021

Thanks for the awesome work @lovesegfault! Is there any status update on this PR, or is there another way to use the system's RocksDB? RocksDB is one of the biggest chunks of our compile time—we'd be happy to shave some part of it off.

@Ralith
Copy link

Ralith commented Mar 5, 2023

Would love to see this picked back up.

@Iamknownasfesal
Copy link

Any updates about this? We would love this change

@DaveWK
Copy link

DaveWK commented Apr 19, 2023

reposting here for anyone else who happens to find this pr via search -- you can use the precompiled rocksdb from your package manager by setting ROCKSDB_LIB_DIR (in my case for fedora 38, ROCKSDB_LIB_DIR=/usr/lib64

@Xuanwo
Copy link

Xuanwo commented Jul 13, 2023

Hi, @aleksuss, would you like to take another review of this PR? I'm willing to continue the work to get it merged.

@aleksuss
Copy link
Member

@Xuanwo ok. I will look at this when the merge conflicts will be resolved.

@aleksuss
Copy link
Member

aleksuss commented Aug 8, 2023

@Xuanwo, I like the idea overall, but I guess the PR should be updated.

@Xuanwo
Copy link

Xuanwo commented Aug 8, 2023

@Xuanwo, I like the idea overall, but I guess the PR should be updated.

I will continue the work.

@jhult
Copy link

jhult commented May 29, 2024

@Xuanwo, any news?

@Xuanwo
Copy link

Xuanwo commented Jun 3, 2024

@Xuanwo, any news?

Apologies for not having enough time for this. Feel free to take over if you're interested.

@spdfnet
Copy link

spdfnet commented Jun 4, 2024

Don't want t cause any flame wars, but the future might be https://github.com/speedb-io/rust-speedb

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.

Linking with system RocksDB