-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Add _UseNativeLibPrefix option for publishing a NativeAOT shared library on Unix #118299
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
Change IntermediateAssembly update to use the NativeBinary property
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.
Pull Request Overview
This PR adds a new _UseNativeLibPrefix
option to enable the "lib" prefix for NativeAOT shared libraries on Unix systems, addressing naming conventions expected by Unix systems and Android Java where libraries must follow the libMyLibrary.so
format.
Key changes:
- Introduces an opt-in
_UseNativeLibPrefix
property that defaults tofalse
- Adds
NativeBinaryPrefix
property that applies "lib" prefix for non-Windows, non-executable targets when enabled - Updates the
NativeBinary
path construction to include the prefix - Simplifies the
IntermediateAssembly
item group to directly reference@(NativeBinary)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
Microsoft.NETCore.Native.targets | Adds _UseNativeLibPrefix and NativeBinaryPrefix properties, updates NativeBinary path to include prefix |
Microsoft.NETCore.Native.Publish.targets | Simplifies IntermediateAssembly item group to use @(NativeBinary) directly |
Comments suppressed due to low confidence (1)
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets:74
- The property name
_UseNativeLibPrefix
uses a leading underscore which typically indicates internal/private properties in MSBuild. Consider usingUseNativeLibPrefix
without the underscore for a public-facing feature that users will set.
<_UseNativeLibPrefix Condition="'$(_UseNativeLibPrefix)' == ''">false</_UseNativeLibPrefix>
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
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.
Is the intention to set _UseNativeLibPrefix
in the Android SDK? If so, I'd prefer a non-underscored property that we could also document.
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets
Outdated
Show resolved
Hide resolved
I'm not sure yet if it's required in the Android SDK, but it's helpful in the tests in this repo. I'd want to give the naming some more thought before documenting it publicly. Maybe we can revisit if/when we find out it's necessary in the Android SDK? |
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 provided nothing outside this repo will set it. We'll eventually need a public mechanism for this I think.
Most Unix systems expect the library filename to be libMyLibrary.so / libMyLibrary.dylib. On Java on android, libraries will not be found unless they follow this naming format.
Tring to work around this by renaming the TargetName or AssemblyName can cause rd.xml files to fail to resolve the assembly in ilc, and it doesn't feel right to .
This PR adds a _UseNativeLibPrefix option to add the "lib" prefix to the output library when it's a shared library published for a non-windows OS. It's opt-in-only to avoid breaking existing setups, but it might be worth considering as a default in future releases to match the rest of the unix ecosystem.