Skip to content

fix: resolve static linking failures with OpenSSL and duplicate symbols#438

Open
dkropachev wants to merge 5 commits intomasterfrom
fix/164-static-linking-openssl
Open

fix: resolve static linking failures with OpenSSL and duplicate symbols#438
dkropachev wants to merge 5 commits intomasterfrom
fix/164-static-linking-openssl

Conversation

@dkropachev
Copy link
Copy Markdown
Contributor

@dkropachev dkropachev commented Apr 2, 2026

Summary

Fix the static-linking regressions behind #164 at the installed-package boundary, not just for the in-tree integration binary. The branch now publishes the static driver's transitive link requirements through pkg-config, teaches the package smoke app to consume that metadata safely, removes duplicate testing stubs from the C++ side, and keeps the package CI coverage readable with dedicated static-smoke targets.

Fixes: #164

What Changed

  1. Support installed static consumers and package checks
  • publish transitive static-link dependencies in scylla-cpp-driver_static.pc
  • resolve static pkg-config compile/link flags in the smoke app, including Apple framework flags
  • keep the Windows external OpenSSL path explicit when the static integration build falls back to the vendored project
  • align the package/static-integration checks with the installed artifacts rather than only the in-tree target
  1. Remove duplicate static-integration stubs
  • drop the C++ stubs from src/testing_unimplemented.cpp that are already provided by the Rust integration layer during cpp_integration_testing
  1. Fix Windows OPENSSL_LIBS assembly
  • build OPENSSL_LIBS with an explicit join so openssl-sys receives real library names in package and static-integration builds
  1. Split static package smoke checks into dedicated CI targets
  • add dedicated make targets for static smoke builds/runs
  • call those targets directly from the package workflow instead of threading SCYLLA_SMOKE_BUILD_STATIC=ON through CI YAML

Commit Structure

  • c1eeee8 static linking: support installed consumers and package checks
  • 61d6d5f testing: remove duplicate static-integration stubs
  • 1dd2538 windows: assemble OPENSSL_LIBS from discovered libraries
  • 08cec82 ci: split static package smoke checks into dedicated targets
  • 5f4f201 gitignore: ignore build_static

Testing

  • cmake -S . -B /tmp/cpp-rs-driver-cmake-check -G Ninja -DCMAKE_BUILD_TYPE=Release
  • make check
  • make run-test-unit
  • cmake -S . -B /tmp/cpp-rs-driver-install-build -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/cpp-rs-driver-install-prefix && cmake --build /tmp/cpp-rs-driver-install-build -j 4 && cmake --install /tmp/cpp-rs-driver-install-build
  • PKG_CONFIG_PATH=/tmp/cpp-rs-driver-install-prefix/lib/pkgconfig:/usr/lib/x86_64-linux-gnu/pkgconfig:/usr/share/pkgconfig cmake -S packaging/smoke-test-app -B /tmp/cpp-rs-driver-smoke-build -G Ninja -DCMAKE_BUILD_TYPE=Release -DSCYLLA_SMOKE_BUILD_STATIC=ON && PKG_CONFIG_PATH=/tmp/cpp-rs-driver-install-prefix/lib/pkgconfig:/usr/lib/x86_64-linux-gnu/pkgconfig:/usr/share/pkgconfig cmake --build /tmp/cpp-rs-driver-smoke-build -j 4
  • cmake --build build_static -j 4 --target cassandra-integration-tests
  • make -n OS=Windows_NT build-driver CMAKE_BUILD_TYPE=Release | rg "OPENSSL_LIBS|Join\(':', @\("
  • make -n OS=Windows_NT build-static-integration-test-bin | rg "OPENSSL_LIBS|Join\(':', @\("
  • make -n test-package-deb-static-smoke
  • make -n test-package-rpm-native-static-smoke SCYLLA_HOST=scylla SKIP_DOCKER_COMPOSE=1
  • make -n test-package-macos-static-smoke

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • PR description sums up the changes and reasons why they should be introduced.
  • I have implemented Rust unit tests for the features/changes introduced.
  • I have enabled appropriate tests in Makefile in {SCYLLA,CASSANDRA}_(NO_VALGRIND_)TEST_FILTER.
  • I added appropriate Fixes: annotations to PR description.

@dkropachev dkropachev force-pushed the fix/164-static-linking-openssl branch 10 times, most recently from 97356ff to 3b91f17 Compare April 3, 2026 14:02
@dkropachev dkropachev marked this pull request as ready for review April 3, 2026 18:37
@dkropachev dkropachev self-assigned this Apr 3, 2026
@dkropachev
Copy link
Copy Markdown
Contributor Author

I am not sure if I took correct way regarding Duplicate symbol guards.

@wprzytula
Copy link
Copy Markdown
Contributor

wprzytula commented Apr 7, 2026

I am not sure if I took correct way regarding Duplicate symbol guards.

No, it's not correct. The real problem in the first place is is that they are duplicated in the sources. I think a proper solution is to delete them from the testing_unimplemented.cpp. This will resolve the linking issues.

Optionally, we could make the stubs present in integration.rs print an error message and call sysexit. Unfortunately, throwing C++ exceptions is not possible from pure Rust with C FFI. @Lorak-mmk Should we bother with this?

@Lorak-mmk
Copy link
Copy Markdown
Contributor

No, it's not correct. The real problem in the first place is is that they are duplicated in the sources. I think a proper solution is to delete them from the testing_unimplemented.cpp. This will resolve the linking issues.

Agreed.

@Lorak-mmk Should we bother with this?

Do we need to? Can't we use the stubs from integration.rs?

@wprzytula
Copy link
Copy Markdown
Contributor

@Lorak-mmk Should we bother with this?

Do we need to? Can't we use the stubs from integration.rs?

I do propose using stubs from integration.rs. Additionally, I say that optionally we can make them call sysexit. Currently, they are just no-ops. The stubs in testing_unimplemented.cpp have a nice feature of throwing an exception, which fails nonsupported tests with a meaningful message.
But perhaps it's not important at all that nonsupported tests throw such exception / end in a distinctive way. I think so. Let's not bother.

@Lorak-mmk
Copy link
Copy Markdown
Contributor

Sysexit or other abort method sounds reasonable.

@dkropachev
Copy link
Copy Markdown
Contributor Author

Guys, could you please tell me what to do with the pr

@Lorak-mmk
Copy link
Copy Markdown
Contributor

Section "Fix: Duplicate symbol guards (src/testing_unimplemented.cpp)" should be reverted. The correct fix is to remove those symbols from testing_unimplemented.cpp

Copy link
Copy Markdown
Contributor

@wprzytula wprzytula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please improve commit messages. I can't read them.

Comment on lines 84 to 131
- name: Build and test RPM packages
run: make test-package-rpm-native SCYLLA_HOST=scylla SKIP_DOCKER_COMPOSE=1
run: make test-package-rpm-native SCYLLA_HOST=scylla SKIP_DOCKER_COMPOSE=1 SCYLLA_SMOKE_BUILD_STATIC=ON

- name: Collect artifacts
if: inputs.save-artifacts
run: make collect-package-artifacts

- uses: actions/upload-artifact@v4
if: inputs.save-artifacts
with:
name: linux-rpm-packages
path: artifacts/linux
retention-days: 7

macos:
name: macOS packages
runs-on: macos-15-intel
steps:
- uses: actions/checkout@v4

- uses: actions-rust-lang/setup-rust-toolchain@v1

- name: Install GNU make
run: brew install make

- name: Build and test macOS packages (PKG + DMG)
run: gmake test-package-macos
run: gmake test-package-macos SCYLLA_SMOKE_BUILD_STATIC=ON

- name: Verify static linking builds (regression test for issue #164)
run: |
gmake build-static-integration-test-bin
rm -rf build_static

- name: Collect artifacts
if: inputs.save-artifacts
run: gmake collect-package-artifacts

- uses: actions/upload-artifact@v4
if: inputs.save-artifacts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Why are you adding SCYLLA_SMOKE_BUILD_STATIC=ON everywhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, removed it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit Keep Windows package builds from exporting a literal OpenSSL variable name: commit message is not readable. Also, it contains magic formulas like Rejected: etc. Please write the commit message suitable for human reviewers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Publish the static driver's transitive dependencies in generated pkg-config metadata, teach the smoke app to consume static pkg-config output safely, and add package-level/static-integration checks that exercise the installed artifacts.\n\nThis keeps installed static consumers aligned with the in-tree target and makes the Windows OpenSSL external-project path explicit during static integration builds.
The Rust integration layer already provides these test-only symbols when cpp_integration_testing is enabled. Drop the duplicate C++ stubs from testing_unimplemented.cpp so static integration builds link against a single definition.
Build the OPENSSL_LIBS environment variable with an explicit join instead of relying on PowerShell interpolation. This keeps openssl-sys from seeing a literal variable name in package and static-integration builds.
Add dedicated static-smoke make targets and call them directly from the package workflow. This keeps CI failures scoped to one smoke mode and avoids threading SCYLLA_SMOKE_BUILD_STATIC through the workflow YAML.
@dkropachev dkropachev force-pushed the fix/164-static-linking-openssl branch from c3db5d3 to 5f4f201 Compare April 8, 2026 18:18
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.

Make -DCASS_BUILD_INTEGRATION_TESTS=ON -DCASS_USE_STATIC_LIBS=ON work

3 participants