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 zstd-sys from 1.6 to 2.0 #651

Merged
merged 1 commit into from Jul 4, 2022

Conversation

slightknack
Copy link
Contributor

@slightknack slightknack commented Jun 29, 2022

Issue

When using rocksdb, it is not possible to use another rust crate that depends on a more recent version of zstd-sys, because of a major version conflict1.

Fix

-zstd-sys = { version = "1.6", default-features = false, optional = true }
+zstd-sys = { version = "2.0", features = ["zdict_builder"], optional = true }

Updates zstd-sys from 1.6 to 2.0 in librocksdb-sys. We additionally add the zdict_builder feature flag. Reading through the zstd-rs commit log, the reason for the breaking change is as follows:

commit 3a6c7fa76b49928bd4afc6a68ae82fce842df8dd (tag: v0.11.0)
Author: Alexandre Bury <alexandre.bury@gmail.com>
Date:   Wed Mar 9 15:48:49 2022 -0500

    Bump version to 0.11.0

    zstd-sys 2.0.0
    zstd-safe 5.0.0

    Breaking changes:
    - Dictionary-training operations have been moved behind a zdict_builder feature.

Adding the zdict_builder feature flag ensures that no regressions are introduced: all rocksdb tests pass locally running cargo test; additionally, running this patch against another codebase that depends on rocksdb does not appear to introduce any regressions.

Motivation

We are currently using a fork of this codebase due to this issue, and would appreciate it if this minimal fix could be upstreamed. Thank you!

Footnotes

  1. This is because "Only one package in the dependency graph may specify the same links value. This helps ensure that only one copy of a native library is linked in the final binary." See https://doc.rust-lang.org/cargo/reference/resolver.html?highlight=links#links for more info.

@stanislav-tkach stanislav-tkach requested a review from aleksuss Jun 30, 2022
@aleksuss aleksuss merged commit 39dc822 into rust-rocksdb:master Jul 4, 2022
6 checks passed
@slightknack slightknack deleted the zstd-2.0 branch Jul 4, 2022
jasl pushed a commit to jasl/rust-rocksdb that referenced this issue Aug 9, 2022
vldm pushed a commit to velas/rust-rocksdb that referenced this issue Sep 8, 2022
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.

None yet

3 participants