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.

Sign up for GitHub

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

Jump to bottom

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

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

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

wants to merge 53 commits into from

Conversation

lovesegfault
Copy link

@lovesegfault lovesegfault commented Nov 21, 2019 •
edited
Loading

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
librocksdb-sys: static -> vendored
d976bac
@lovesegfault
librocksdb-sys: add pkg-config dep
8883a5d
@lovesegfault
librocksdb-sys: add vendor module
5e35f53
@lovesegfault
librocksdb-sys: implement check_submodule
286832d
@lovesegfault
librocksdb-sys: use AsRef<Path> for check_submodule
dcfad61
@lovesegfault
librocksdb-sys: add vendor_dependencies stub
e2d5624
@lovesegfault
librocksdb-sys: add enforce_rerun
6486d69
@lovesegfault
librocksdb-sys: rerun and submodule checks
2ac1a64
@lovesegfault
librocksdb-sys: add wrapper.h for bindgen
dfd3032
@lovesegfault
librocksdb-sys: Set wrapper.h as bindgen's header
8b5634d
@lovesegfault
librocksdb-sys: comment out old main
e4812c1
@lovesegfault
librocksdb-sys: add vendor_dependencies call
5adb8ee
@lovesegfault
librocksdb-sys: replace build_snappy
d8d9b33
@lovesegfault
librocksdb-sys: add bindgen call to vendor_dependencies()
4e3789b
@lovesegfault
librocksdb-sys: replace build_lz4
d0bc77a
@lovesegfault
librocksdb-sys: vec-> &[]
794d4c8
@lovesegfault
librocksdb-sys: move cc, glob into mod vendor
724faad
@lovesegfault
librocksdb-sys: replace build_zstd
1363562
@lovesegfault
librocksdb-sys: replace build_zlib
9bb2a74
@lovesegfault
librocksdb-sys: replace build_bzip2
1d9c6b6
@lovesegfault
librocksdb-sys: replace build_rocksdb
a47c482
@lovesegfault
librocksdb-sys: nuke link (replaced with windows_link)
2728b28
@lovesegfault
librocksdb-sys: nuke fail_on_empty_directory
abec76f
@lovesegfault
librocksdb-sys: clean use statements
9dcc72c
@lovesegfault
librocksdb-sys: comment try_to_find_and_link_lib()
d5bcb24
@lovesegfault
librocksdb-sys: finish vendor_dependencies
74740f8
@lovesegfault
librocksdb-sys: add link module
fa5d651
@lovesegfault
librocksdb-sys: fix snappy-c.cc path
17de867
@lovesegfault
librocksdb-sys: call bindgen in main
b798435
@jonhoo
Copy link
Contributor

jonhoo commented Feb 20, 2020

This is awesome stuff @lovesegfault!

aleksuss
aleksuss reviewed Feb 20, 2020
View reviewed changes
librocksdb-sys/Cargo.toml Outdated Show resolved Hide resolved
@lovesegfault lovesegfault mentioned this pull request Feb 20, 2020
Open
vitvakatu
vitvakatu suggested changes Feb 21, 2020
View reviewed changes
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
@Connor1996
Add force_consistency_checks option (rust-rocksdb#342) (rust-rocksdb#354
a9b9137 
)

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 DaveWK mentioned this pull request Apr 19, 2023
Open
@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

This was referenced Jun 15, 2023
Merged
Closed
@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.

@cratelyn cratelyn mentioned this pull request Jun 13, 2024
Closed
@zaidoon1 zaidoon1 mentioned this pull request Jul 29, 2024
Merged
@Aranjedeath
Copy link

@Xuanwo, any news?

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

Any idea how much work or time would be needed to finish this out (at least to get it submitted for review)?

I can't do it, but I can throw $100 on github at somebody willing to work on it, if that helps.

@rust-rocksdb rust-rocksdb deleted a comment from spdfnet Aug 3, 2024
@mr-c mr-c mentioned this pull request Nov 12, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aleksuss aleksuss Awaiting requested review from aleksuss

@vitvakatu vitvakatu Awaiting requested review from vitvakatu

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Linking with system RocksDB