-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Clarify that NativeLibrary can be used for transitive dependencies #42815
base: main
Are you sure you want to change the base?
Conversation
Also @MichalStrehovsky |
<NativeLibrary Include="Dependency.lib" Condition="$(RuntimeIdentifier.StartsWith('win'))" /> | ||
<NativeLibrary Include="Dependency.a" Condition="!$(RuntimeIdentifier.StartsWith('win'))" /> | ||
<NativeLibrary Include="/path/to/Dependency.lib" Condition="$(RuntimeIdentifier.StartsWith('win'))" /> | ||
<NativeLibrary Include="/path/to/Dependency.a" Condition="!$(RuntimeIdentifier.StartsWith('win'))" /> | ||
<!-- Specify an additional library to link against, if necessary --> | ||
<NativeLibrary Include="/path/to/TransitiveDependency.lib" Condition="$(RuntimeIdentifier.StartsWith('win'))" /> | ||
<NativeLibrary Include="/path/to/TransitiveDependency.a" Condition="!$(RuntimeIdentifier.StartsWith('win'))" /> |
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 would leave this section as is and add a note/remarks after. Something to the tune of:
- If the native library has a dependency on other static libraries, the transitive dependencies need to be provided as
NativeLibrary
items as well. Native linker will fail with a message with missing symbols if the transitive dependencies are not provided. - Platform conventions are followed when it comes to the format of
NativeLibrary
entries. The underlying native linker may search standard library paths or allow rooted file 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.
Thanks for the review. Should I make the requested changes, or would it be better for someone from Microsoft to provide the correct wording?
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.
@sweemer You can make the suggested changes. Thank you.
I have added the following two clarifications:
NativeLibrary
can be used more than once. Originally I thought that it can only be used for the library specified inDirectPInvoke
, but after some time I realized it can be used for transitive dependencies as well. My change makes this more clear./path/to
to make it clear that paths are accepted and that the file does not need to be manually moved to the project output directory in order to be found.Internal previews