Skip to content

fix: remove openssl dependency to fix Linux release builds#317

Merged
dubadub merged 1 commit intomainfrom
fix/remove-openssl-dependency
Apr 9, 2026
Merged

fix: remove openssl dependency to fix Linux release builds#317
dubadub merged 1 commit intomainfrom
fix/remove-openssl-dependency

Conversation

@dubadub
Copy link
Copy Markdown
Member

@dubadub dubadub commented Apr 9, 2026

Summary

  • Bump cooklang-sync-client from 0.4.8 to 0.4.11 which uses rustls-tls instead of default-tls (native-tls/OpenSSL)
  • Remove native-tls from dev-dependency reqwest for consistency
  • Update runner.rs to match new cooklang-sync-client 0.4.11 API (SyncContext replaces raw CancellationToken)

This eliminates openssl-sys and ~15 related crates from the dependency tree, fixing all 5 failing Linux targets in the release workflow and reducing binary size.

Test plan

  • cargo fmt -- --check passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • cargo test passes
  • Verified native-tls and openssl-sys are absent from cargo tree -e no-dev

Bump cooklang-sync-client from 0.4.8 to 0.4.11, which uses rustls-tls
instead of default-tls (native-tls/OpenSSL). This eliminates the openssl-sys
dependency that was causing all Linux release builds to fail in CI since
cross-compilation containers lack OpenSSL dev libraries.

Also removes native-tls from dev-dependency reqwest for consistency.
@claude
Copy link
Copy Markdown

claude bot commented Apr 9, 2026

Code Review

This is a clean, well-scoped fix that addresses a real CI problem. The approach is sound — swapping native-tls/OpenSSL for rustls is the right call for cross-compilation scenarios, and rustls being memory-safe is a net security improvement.

✅ What's good

  • Removes ~15 OpenSSL-related crates from the dependency tree — significantly cleaner
  • rustls is memory-safe and removes the need for system OpenSSL dev libraries during cross-compilation
  • The runner.rs changes correctly adopt the new SyncContext API and are minimal/correct
  • The None argument removal (a config option that was unused) is a welcome cleanup in the new API

🔍 Things worth checking

reqwest dev-dependency loses HTTP/2

# Before
reqwest = { version = "0.12", features = ["json", "rustls-tls"] }
# After
reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] }

default-features = false also drops http2 (the h2 crate, visible in the Cargo.lock diff). If any integration tests exercise HTTP/2 endpoints, they may silently fall back to HTTP/1.1 or fail. Worth a quick check through the test suite to confirm nothing relied on HTTP/2 in reqwest.

libsqlite3-sys is new in cooklang-sync-client 0.4.11

The Cargo.lock diff shows libsqlite3-sys added as a transitive dep of the new sync client version. Cargo.toml already declares libsqlite3-sys = { version = "0.35", features = ["bundled"], optional = true } — so this is consistent with the bundled SQLite approach and actually removes another system library dependency, which aligns with the goal of this PR. Just worth noting the minor binary size increase when the sync feature is enabled.

Arc<SyncContext> wrapping

pub struct SyncHandle {
    context: Arc<SyncContext>,
    ...
}

If SyncContext::new() already returns an Arc-backed type internally (which it must, since clone() is used to share it across threads), wrapping it in another Arc in SyncHandle is redundant but harmless. If SyncContext exposes a clone() that is cheap (arc-clone), you could drop the outer Arc — but this is a minor nit and the compiler won't complain either way.

Summary

This is a good fix. The reqwest HTTP/2 point is worth a quick sanity check on the test suite, but nothing here looks like a blocker. The overall dependency reduction (225 lines removed from Cargo.lock) speaks for itself.

@dubadub dubadub merged commit f1d4d56 into main Apr 9, 2026
6 checks passed
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.

1 participant