Rename lib to scylladb#472
Conversation
📝 WalkthroughWalkthroughThis PR systematically renames the C++ driver library from Possibly related PRs
Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
coderabbitai
Bot
added
area/testing
There was a problem hiding this comment.
Pull request overview
This PR renames the driver’s built library artifact to scylladb (avoiding both hyphens and underscores) to resolve cross-platform naming issues—especially the Windows DLL/import-lib mismatch—and introduces optional cassandra-compat installation artifacts to ease migration for existing users.
Changes:
- Renames the Rust
[lib]output and updates CMake/install logic, pkg-config metadata, and build scripts to usescylladb/scylladb_static. - Adds optional
cassandracompatibility artifacts (symlinks +cassandra.pc) controlled byCASS_INSTALL_CASSANDRA_COMPAT(default ON). - Updates documentation and packaging/smoke-test infrastructure to reflect new linking/module names.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scylla-rust-wrapper/scylladb.pc.in | Updates shared pkg-config metadata to link with -lscylladb. |
| scylla-rust-wrapper/scylladb_static.pc.in | Updates static pkg-config metadata to link with -lscylladb_static. |
| scylla-rust-wrapper/CMakeLists.txt | Renames installed artifacts/targets, adjusts Windows handling, and adds optional cassandra-compat symlink/pkg-config install logic. |
| scylla-rust-wrapper/cassandra.pc.in | Adds cassandra pkg-config file for compatibility migrations. |
| scylla-rust-wrapper/Cargo.toml | Renames Rust library output to scylladb. |
| packaging/smoke-test-app/Makefile | Updates smoke-test package verification/removal paths for new artifact names. |
| packaging/smoke-test-app/CMakeLists.txt | Switches smoke-test build to consume scylladb pkg-config modules. |
| Makefile | Updates package smoke-test lookup patterns to find scylladb artifacts. |
| docs/source/topics/getting-started.md | Documents new linking names and migration guidance for existing users. |
| CMakeLists.txt | Adds CASS_INSTALL_CASSANDRA_COMPAT option and switches core target selection to scylladb(_static). |
| cmake/CMakeCargo.cmake | Updates diagrams/comments to reflect the new scylladb naming. |
| .github/workflows/build-lint-and-test.yml | Updates cached integration-test artifact patterns for renamed library files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-lint-and-test.yml:
- Around line 13-17: The workflow env block has two list-like entries
("build/cassandra-integration-tests" and "build/libscylladb.*") accidentally
placed where keys belong, breaking YAML and leaving INTEGRATION_TEST_BIN
undefined; fix by adding the missing environment key INTEGRATION_TEST_BIN and
set it to the intended path (e.g., INTEGRATION_TEST_BIN:
build/cassandra-integration-tests) and remove or move the stray
"build/libscylladb.*" entry (or place it under a proper key) so that
env.INTEGRATION_TEST_BIN is available for later uses like
INTEGRATION_TEST_BIN_CACHE_KEY and actions/cache/save.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ef252f98-707b-43b5-8f5c-f774ae136819
📒 Files selected for processing (12)
.github/workflows/build-lint-and-test.ymlCMakeLists.txtMakefilecmake/CMakeCargo.cmakedocs/source/topics/getting-started.mdpackaging/smoke-test-app/CMakeLists.txtpackaging/smoke-test-app/Makefilescylla-rust-wrapper/CMakeLists.txtscylla-rust-wrapper/Cargo.tomlscylla-rust-wrapper/cassandra.pc.inscylla-rust-wrapper/scylladb.pc.inscylla-rust-wrapper/scylladb_static.pc.in
wprzytula
force-pushed
the
rename-lib-to-scylladb
branch
2 times, most recently
from
926aa98 to
2a37d5a
Compare
coderabbitai
Bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/topics/getting-started.md`:
- Line 82: Update the wording around the dev-package names so Debian and RPM are
separated: replace the current sentence that lists "scylla-cpp-driver /
scylla-cpp-driver-dev" with two distro-family examples (Debian/Ubuntu:
scylla-cpp-driver and scylla-cpp-driver-dev; RPM/Fedora/CentOS:
scylla-cpp-driver and scylla-cpp-driver-devel) and ensure the text references
both "-dev" (Debian) and "-devel" (RPM) explicitly to avoid migration confusion;
update any adjacent examples or package lists that still imply a single "-dev"
name so they match the new split.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e5751da9-2e1e-4425-8a87-fa32314368e5
📒 Files selected for processing (15)
.github/workflows/build-lint-and-test.ymlCMakeLists.txtMakefilecmake/CMakeCargo.cmakedocs/source/topics/getting-started.mdpackaging/smoke-test-app/CMakeLists.txtpackaging/smoke-test-app/Makefilescylla-rust-wrapper/CMakeLists.txtscylla-rust-wrapper/Cargo.tomlscylla-rust-wrapper/cassandra.pc.inscylla-rust-wrapper/scylladb.pc.inscylla-rust-wrapper/scylladb_static.pc.inscylla-rust-wrapper/tests/integration/consistency.rsscylla-rust-wrapper/tests/integration/session.rsscylla-rust-wrapper/tests/integration/utils.rs
✅ Files skipped from review due to trivial changes (2)
- scylla-rust-wrapper/cassandra.pc.in
- cmake/CMakeCargo.cmake
🚧 Files skipped from review as they are similar to previous changes (9)
- .github/workflows/build-lint-and-test.yml
- scylla-rust-wrapper/scylladb.pc.in
- packaging/smoke-test-app/Makefile
- Makefile
- CMakeLists.txt
- packaging/smoke-test-app/CMakeLists.txt
- scylla-rust-wrapper/Cargo.toml
- scylla-rust-wrapper/scylladb_static.pc.in
- scylla-rust-wrapper/CMakeLists.txt
| | pkg-config (static) | `scylla-cpp-driver_static` | `scylladb_static` | | ||
| | SONAME | `libscylla-cpp-driver.so.1` | `libscylladb.so.1` | | ||
|
|
||
| Package names (DEB/RPM) remain `scylla-cpp-driver` / `scylla-cpp-driver-dev`. |
There was a problem hiding this comment.
Clarify RPM dev package name to avoid migration confusion.
Line 82 currently implies a single -dev dev-package name across DEB/RPM, but this page also shows RPM using -devel. Please split this by distro family (e.g., Debian -dev, RPM -devel) to keep migration instructions accurate.
🧰 Tools
🪛 LanguageTool
[grammar] ~82-~82: Ensure spelling is correct
Context: ... (DEB/RPM) remain scylla-cpp-driver / scylla-cpp-driver-dev. ### Migrating from ScyllaDB CPP Driver (C++ ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/topics/getting-started.md` at line 82, Update the wording around
the dev-package names so Debian and RPM are separated: replace the current
sentence that lists "scylla-cpp-driver / scylla-cpp-driver-dev" with two
distro-family examples (Debian/Ubuntu: scylla-cpp-driver and
scylla-cpp-driver-dev; RPM/Fedora/CentOS: scylla-cpp-driver and
scylla-cpp-driver-devel) and ensure the text references both "-dev" (Debian) and
"-devel" (RPM) explicitly to avoid migration confusion; update any adjacent
examples or package lists that still imply a single "-dev" name so they match
the new split.
Change the library artifact name to 'scylladb', avoiding both hyphens and underscores. This eliminates the need for the underscore-to-hyphen rename that was causing issues on Windows (scylladb#452). The Cargo [lib] name is now 'scylladb', producing libscylladb.so, libscylladb.a, scylladb.dll directly. The SONAME becomes libscylladb.so.<major>. The pkg-config module names change accordingly: scylla-cpp-driver -> scylladb scylla-cpp-driver_static -> scylladb_static Package names (DEB/RPM) remain 'scylla-cpp-driver' unchanged.
Add symlinks to ease migration from the Apache CPP Driver: - libcassandra.so -> libscylladb.so - libcassandra_static.a -> libscylladb_static.a - cassandra.pc pkg-config file (links against -lscylladb) These are installed by default (CASS_INSTALL_CASSANDRA_COMPAT=ON) and can be disabled if not needed.
The remove-driver-dev-dmg target was missing removal of scylladb_static.pc and libscylladb_static.a, which could leave stale files under /usr/local between smoke-test runs.
Add a Linking section to getting-started.md covering: - New users: how to link with -lscylladb / pkg-config - Upgrading from CPP RS Driver 1.x: name mapping table - Migrating from ScyllaDB CPP Driver (C++ fork) - Migrating from DataStax CPP Driver Also clarify the package-vs-library name discrepancy (package is scylla-cpp-driver, library is libscylladb).
wprzytula
force-pushed
the
rename-lib-to-scylladb
branch
from
2a37d5a to
c7e037b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/source/topics/getting-started.md (1)
82-82:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify dev-package naming for DEB vs RPM distributions.
Line 82 states package names remain
scylla-cpp-driver/scylla-cpp-driver-dev, but the examples earlier in the document (lines 33, 40) correctly show that Debian uses-devwhile RPM uses-devel. This single suffix is ambiguous for RPM users.Consider rephrasing to: "Package names remain
scylla-cpp-driverandscylla-cpp-driver-dev(Debian) orscylla-cpp-driver-devel(RPM)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/topics/getting-started.md` at line 82, The sentence that currently says package names remain `scylla-cpp-driver` / `scylla-cpp-driver-dev` is ambiguous for RPM users; update the wording to explicitly distinguish Debian vs RPM suffixes, e.g. state "Package names remain `scylla-cpp-driver` and `scylla-cpp-driver-dev` (Debian) or `scylla-cpp-driver-devel` (RPM)" so the examples using `-dev` and `-devel` are clearly mapped to their distributions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@docs/source/topics/getting-started.md`:
- Line 82: The sentence that currently says package names remain
`scylla-cpp-driver` / `scylla-cpp-driver-dev` is ambiguous for RPM users; update
the wording to explicitly distinguish Debian vs RPM suffixes, e.g. state
"Package names remain `scylla-cpp-driver` and `scylla-cpp-driver-dev` (Debian)
or `scylla-cpp-driver-devel` (RPM)" so the examples using `-dev` and `-devel`
are clearly mapped to their distributions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6584e739-b339-4781-8bf4-d03e2405786a
📒 Files selected for processing (16)
.github/workflows/build-lint-and-test.ymlCMakeLists.txtMakefilecmake/CMakeCargo.cmakedocs/source/topics/getting-started.mdpackaging/smoke-test-app/CMakeLists.txtpackaging/smoke-test-app/Makefilescylla-rust-wrapper/CMakeLists.txtscylla-rust-wrapper/Cargo.tomlscylla-rust-wrapper/cassandra.pc.inscylla-rust-wrapper/scylladb.pc.inscylla-rust-wrapper/scylladb_static.pc.inscylla-rust-wrapper/src/argconv.rsscylla-rust-wrapper/tests/integration/consistency.rsscylla-rust-wrapper/tests/integration/session.rsscylla-rust-wrapper/tests/integration/utils.rs
✅ Files skipped from review due to trivial changes (6)
- scylla-rust-wrapper/scylladb_static.pc.in
- scylla-rust-wrapper/cassandra.pc.in
- scylla-rust-wrapper/src/argconv.rs
- cmake/CMakeCargo.cmake
- scylla-rust-wrapper/tests/integration/consistency.rs
- scylla-rust-wrapper/tests/integration/utils.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- scylla-rust-wrapper/scylladb.pc.in
- packaging/smoke-test-app/Makefile
- .github/workflows/build-lint-and-test.yml
- scylla-rust-wrapper/Cargo.toml
- packaging/smoke-test-app/CMakeLists.txt
- Makefile
- CMakeLists.txt
- scylla-rust-wrapper/tests/integration/session.rs
- scylla-rust-wrapper/CMakeLists.txt
Let's put that in release notes as well.
This of course needs to be the most visible thing in release notes.
What is the reason for that? Being able to update from previous version? |
I thought we had copy step in general, not only on windows. Is thart not the case? And if it is the case, do we still need it? |
Lorak-mmk
left a comment
Lorak-mmk
left a comment
There was a problem hiding this comment.
Looks good. Please answer my questions above before merging.
This sounds extremely weird to install Also, I wanted to become again in line with DSx driver's naming:
|
The description you quoted was a leftover. See the updated cover letter (commit 3). |
|
Commit 3 is |
Previously (before a recent push) there was another commit at position 3 - a commit that introduces special case for Windows. With a recent push the commit was dropped, but its description was still present in the cover letter, and you quoted it. The reason for dropping the commit was that it introduces unnecessary complications without adequate reward. |
Opus explains it very well:
|
Fixes: #464
Summary
Rename the library artifact from
libscylla-cpp-drivertolibscylladb, eliminating both hyphens and underscores from the name. This fixes the Windows DLL naming mismatch (#452) where the import library internally referenced Cargo's underscore output (scylla_cpp_driver.dll) while the installed file used hyphens (scylla-cpp-driver.dll), causing "dll not found" errors at runtime.The new name
scylladbis consistent with the original DataStax driver's convention (cassandra) and works cleanly across all platforms.Commits
Rename library from scylla-cpp-driver to scylladb
Core rename: Cargo.toml
[lib] name, all CMake naming (INSTALL_NAME variables, SONAME, targets), pkg-config files (nowscylladb.pc/scylladb_static.pc), CI workflow, and Makefiles. Package names (DEB/RPM) intentionally remainscylla-cpp-driver.Add cassandra compatibility symlinks
Install
libcassandra.so->libscylladb.so,libcassandra_static.a->libscylladb_static.a, and acassandra.pcpkg-config file. Controlled byCASS_INSTALL_CASSANDRA_COMPAT(ON by default). This eases migration from the DataStax CPP Driver where users link with-lcassandra.smoke-test: Remove all dev artifacts in DMG cleanup
The remove-driver-dev-dmg target was missing removal of
scylladb_static.pcandlibscylladb_static.a, which could leave stale files under/usr/localbetween smoke-test runs.Document library naming and migration paths
Add a "Linking" section to
getting-started.mdcovering new users, upgraders from CPP RS 1.0.x, migrators from ScyllaDB CPP Driver, and migrators from DataStax CPP Driver. Includes a mapping table of old vs. new names and a note explaining the package-vs-library name discrepancy.Migration impact
-lscylladbinstead of-lscylla-cpp-driver) and pkg-config invocations.libcassandrasymlinks.scylla-cpp-driver,scylla-cpp-driver-dev).Pre-review checklist
[ ] I have implemented Rust unit tests for the features/changes introduced.[ ] I have enabled appropriate tests inMakefilein{SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.Fixes:annotations to PR description.