Repository navigation
Pin floating dependency versions: Cargo.lock + sqlite-jdbc - #31
Conversation
Rust (D2): commit bindings/rust/Cargo.lock (50 packages; rusqlite 0.40.2, uuid 1.28.0, json 0.12.4) so this repo's builds are reproducible and future crate bumps arrive as reviewable diffs. Cargo ignores a library's lock downstream, so this costs dependents nothing. Tighten the loose requirements that let the float bite: rusqlite ^0 -> ^0.40, json ^0 -> ^0.12, uuid (dev) ^0 -> ^1 (was silently stuck at 0.8.2), cargo_smoke_test rusqlite ^0.25 -> ^0.40, json ^0 -> ^0.12. JDBC (D4): pin sqlite-jdbc 3.53.4.0, replacing the open range [3.31.1,) that resolved to the newest release on the day of each build. sqlite-jdbc bundles the SQLite engine shipped inside the JDBC artifact; its version now changes at maintenance touches, not by calendar. Downstream Maven apps keep final say via dependency management.
mlin
left a comment
mlin
left a comment
There was a problem hiding this comment.
The JDBC change looks reasonable for downstream applications: the plain Maven version is overridable. I verified both a direct application dependency and dependencyManagement selecting sqlite-jdbc 3.31.1 instead, and successfully created/read a compressed database using the new JDBC binding with that version. Committing Cargo.lock also makes sense in principle and does not constrain downstream Cargo consumers. Two Rust-side issues below need attention: the narrowed public dependency requirement introduces a reproducible application conflict, and tracking the lockfile interacts with the existing generated-version wrapper.
|
|
||
| [dependencies.rusqlite] | ||
| version = "^0" | ||
| version = "^0.40" |
There was a problem hiding this comment.
[P2] Please preserve a tested compatibility range here, independently of the development lockfile. I reproduced a consumer with two dependencies: GenomicSQLite and another library using rusqlite 0.25. This change makes Cargo fail resolution because rusqlite 0.40 requires libsqlite3-sys 0.38, whereas rusqlite 0.25 requires libsqlite3-sys 0.22; Cargo permits only one package with links = "sqlite3". The same consumer compiles with both the previous ^0 requirement and >=0.25, <0.41, including passing GenomicSQLite's returned Connection to the other library. A bounded, tested compatibility range would let the application unify on a supported version while Cargo.lock selects our development version. Please retain an older-version compatibility check alongside the current-version smoke test. Cargo documents the native-library constraint here: https://doc.rust-lang.org/cargo/reference/resolver.html#links
|
|
||
| [[package]] | ||
| name = "genomicsqlite" | ||
| version = "0.9.4-7-g1fa5e5c" |
There was a problem hiding this comment.
[P2] Please reconcile the tracked lockfile with bindings/rust/cargo's generated package version. In a disposable clean Git checkout, the first wrapper invocation rewrites this root-package version to the current git describe result, leaving Cargo.lock modified. The next invocation sees that modification through git describe --dirty, adds -dirty, and rewrites the version again. The PR's lint log already demonstrates this: cargo check builds 0.9.4-7-g845e23e, then the following wrapper invocation reports 0.9.4-7-g845e23e-dirty. Keeping the lockfile is useful, but ordinary builds should not feed their own generated lockfile changes back into the package version. Please adjust the version/dirty handling or use a stable development package version so this remains consistent.
1. rusqlite requirement ^0.40 -> >=0.25, <0.41 (Cargo.toml.in). The narrowed pin broke consumers: rusqlite 0.40 needs libsqlite3-sys 0.38 while rusqlite 0.25 needs 0.22, and Cargo allows only one links = "sqlite3" package per graph, so an app using rusqlite 0.25 elsewhere failed resolution outright. The bounded range lets the app unify rusqlite inside the tested span while this repo's Cargo.lock still develops against 0.40.2. Verified: a consumer pinned to rusqlite 0.25.4 resolves (one libsqlite3-sys) and type-checks passing Connections across the boundary. cargo_smoke_test's manifest is restored to main's: its rusqlite ^0.25 IS the older-version compatibility check, alongside the lock-based current-version tests. (json stays ^0.12: it has no links constraint, so nothing forces cross-version unification, and a wider range would overclaim Object type compatibility that pre-1.0 duplication does not deliver.) 2. Shim version no longer feeds on the tracked lockfile. Both cargo wrappers now stamp the nearest tag (git describe --abbrev=0) instead of the per-commit describe, and add -dirty only for changes other than Cargo.lock itself. Ordinary builds leave the lock untouched (verified: cargo check on a clean tree leaves git status empty), and consecutive wrapper runs report the same version. The lock's root entry is now the stable 0.9.4.
Two rulings, one theme: dependency versions in this repo change deliberately, not by calendar.
Rust (D2): commit bindings/rust/Cargo.lock (50 packages: rusqlite 0.40.2, uuid 1.28.0, json 0.12.4) so repo builds are reproducible and crate bumps arrive as reviewable diffs. Cargo ignores a library's lock downstream. Tightened loose requirements: rusqlite ^0 -> ^0.40, json ^0 -> ^0.12, uuid (dev) ^0 -> ^1 (was stuck at 0.8.2), cargo_smoke_test rusqlite ^0.25 -> ^0.40.
JDBC (D4): pin sqlite-jdbc 3.53.4.0, replacing the open range [3.31.1,) that resolved to the newest release on the day of each build. sqlite-jdbc bundles the SQLite engine shipped inside the JDBC artifact. Downstream Maven apps keep final say via dependency management.
Verified locally: lock resolution as listed; crate builds against the locked graph; sqlite-jdbc 3.53.4.0 exists on Maven Central. CI runs the full matrix.