-
Couldn't load subscription status.
- Fork 1.3k
CSHARP-5705: Use standard RID paths in MongoDB.Driver.Encryption packaging #1782
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
@adelinowona
adelinowona
commented
Sep 30, 2025
- Local testing: Verified that when an application consuming the encryption package is published for a specific platform, the correct native library is automatically copied to the publish output directory
- Cross-platform validation: Used Docker containers to test applications on different target platforms, confirming they can successfully locate and load the appropriate native libraries
- Ensured no NETSDK1152 errors occur during builds targeting specific Runtime Identifiers (RIDs)
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.
Added explicit linux-musl-x64 RID mapping to ensure correct native library selection
During local testing, I discovered that MSBuild's RID fallback mechanism was causing incorrect library resolution. According to the RID compatibility graph, linux-musl-x64 is compatible with linux-x64, which caused MSBuild to use the linux-x64 native library as a fallback for linux-musl-x64 targets.
Since we provide a dedicated native library built specifically for linux-musl-x64, this change ensures that the platform-specific library is selected instead of relying on the fallback mechanism.
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.
Switched to PreserveNewest as it follows MSBuild best practices by only copying files when they've been modified, rather than copying on every build. While this should theoretically improve build performance, the impact will be negligible given our small number of native libraries. Made the change for consistency with recommended practices.
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.
This might be problematic is some weird corner cases, and given that it's not a huge toll on our build I'd suggest leaving 'Always' just to be on the safe side.
cc @sanych-sun what do you think?
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.
I'm OK with any setting here. As on EG, while the packages creation, we will always start from scratch and there are no difference.
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.
The macOS paths remain unchanged because our packaged library is a universal binary that supports both arm64 and x64 architectures. According to MSBuild's RID compatibility fallback graph, both osx-x64 and osx-arm64 fall back to osx, so the current structure already handles both platforms correctly without requiring separate RID-specific paths.
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.
From my understanding this target file provides backward compatibility for older, non-SDK-style projects (e.g., .NET Framework with packages.config) that do not automatically handle native assets from the 'runtimes' folder.
The condition ensures this logic ONLY runs for non-SDK-style projects. Modern .NET SDK projects (which set 'UsingMicrosoftNETSdk' to 'true') will handle native asset copying automatically based on the RID.
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.
I have reverted part of this change. I removed the condition on UsingMicrosoftNETSdk which allows the target file to also run for SDK style projects on windows targeting .Net framework. This was motivated by smoketests failing on windows variants and a Microsoft recommending using the targets file for supporting SDK style AnyCPU projects targeting .Net Framework -> https://learn.microsoft.com/en-us/nuget/create-packages/native-files-in-net-packages#projects-targeting-net-framework
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 good, left one suggestion.
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.
This might be problematic is some weird corner cases, and given that it's not a huge toll on our build I'd suggest leaving 'Always' just to be on the safe side.
cc @sanych-sun what do you think?
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.
LGTM
ca347b2 to
5bc7bae
Compare
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.
LGTM
33a1986
into
mongodb:main