-
Notifications
You must be signed in to change notification settings - Fork 44
Moved C Bridge from dotnet-sdk to core-sdk repo (💥 BREAKING CHANGE) #494
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
Conversation
b2e9c42 to
906202c
Compare
|
All checks passed and package build is successful: https://github.com/temporalio/sdk-dotnet/actions/runs/16027792907/job/45221106052?pr=494 I'll remove the trigger and then the PR is ready for review. |
cretz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Besides shared lib name, looks like no user-facing changes which is great.
| included when using modern versions of .NET on a common platform. If you are using .NET framework, you may have to | ||
| explicitly set the platform to `x64` or `arm64` because `AnyCPU` will not choose the proper library. | ||
| This SDK requires a built-in unmanaged, native shared library built in Rust. It is named | ||
| `temporal_sdk_core_c_bridge.dll` on Windows, `libtemporal_sdk_core_c_bridge.so` on Linux, and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make a big note in PR description that we are changing the shared library filename? We also have to make a big, clear note in release notes when we do our next release. I'm pretty sure some users have existing build code that copies artifacts to, say, docker containers or their prod systems that may hardcode expectations of artifact names. So it's kinda a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
💥 BREAKING CHANGE: bridge library file name change
Previously, the C bridge library had the filename
temporal_sdk_bridge.dllon Windows,libtemporal_sdk_bridge.soon Linux, andlibtemporal_sdk_bridge.dylibon macOS.The filename has changed to
temporal_sdk_core_c_bridge.dllon Windows,libtemporal_sdk_core_c_bridge.soon Linux, andlibtemporal_sdk_core_c_bridge.dylibon macOS.This does not affect most users as the library is still copied to the final build directory alongside other artifacts. But if there are any scripts or configuration files that refer to the library file by name, that name needs to be updated.
What was changed
Moved C Bridge from dotnet-sdk to core-sdk repo. PR on core-sdk side: temporalio/sdk-core#951
There were a few breaking changes so other interop code had to be updated as well - mostly due to new symbol prefixes.
Why?
It makes the C bridge available for other languages that prefer to use C interface to interact with Rust.
Checklist
Closes [Feature Request] Move Rust C bridge to sdk-core #423
How was this tested:
Code builds and all tests pass.