-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Enable building .dll if both 'CustomNativeMain' and 'NativeLib' are set #103504
Conversation
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
Would it make sense to do this on all platforms with shared libraries? I think this mode might be used by Unity when they add NativeAOT support so it'd be nice if getting a shared library from it wasn't Windows only. |
Sounds good to me. |
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
Co-authored-by: Jan Kotas <[email protected]>
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Unix.targets
Outdated
Show resolved
Hide resolved
@@ -23,7 +23,7 @@ The .NET Foundation licenses this file to you under the MIT license. | |||
<FullRuntimeName>Runtime.WorkstationGC</FullRuntimeName> | |||
<FullRuntimeName Condition="'$(ServerGarbageCollection)' == 'true' or '$(IlcLinkServerGC)' == 'true' or '$(ControlFlowGuard)' == 'Guard'">Runtime.ServerGC</FullRuntimeName> | |||
<BootstrapperName>bootstrapper</BootstrapperName> | |||
<BootstrapperName Condition="'$(NativeLib)' != '' or '$(CustomNativeMain)' == 'true'">bootstrapperdll</BootstrapperName> | |||
<BootstrapperName Condition="'$(NativeLib)' != ''">bootstrapperdll</BootstrapperName> |
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.
@jkotas should we also keep this condition like in the Unix file? As in, is the bootstrapper .dll always going to be used when a custom main is used, regardless of whether the native binary being produced is actually a .dll or an .exe?
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.
Yes, the Windows and Unix should be in sync.
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
…e.Windows.targets
src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Windows.targets
Outdated
Show resolved
Hide resolved
…e.Windows.targets
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, assuming that this works for CsWinRT E2E.
Thank you! If we want to wait to merge this until we can confirm that, is there a guide on how to generate a local working SDK we can use to try things out, or something? I've only ever tried using nightly SDKs downloaded from dotnet/sdk before 😅 |
https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/nativeaot.md#building-packages has the steps for building private version of the naot package and referencing it in your app. |
@jkotas so I'm trying to test this to get it merged before the RC1 snap, but something seems off. I've pushed a small tweak that now correctly makes NAOT produce a native .dll instead of an .exe. But, if I do `dumpbin /exports', I don't see any exports 🤔
Is there something else we forgot to tweak? 😅 |
Nevermind, all good! Special thanks to @MichalStrehovsky 😄 For the custom main, I had to add: <ItemGroup>
<IlcArg Include="--export-dynamic-symbol:__managed__Main" />
</ItemGroup> To the .csproj. For the export, I had just forgotten to set @jkotas we should be good to merge this and hopefully we're still in time for RC1? 🤞 |
Note that the EventPipe is not able to shutdown cleanly in NativeAOT published libraries (#89346 has detailed). You may run into this issue if you ever try to collect EventPipe-based trace with this setup. The workaround is to end the trace session manually before the process shutdown. |
@Sergio0694 Might want to consider adding this (under the right conditions) to ILCompiler targets as well. Might want to also consider adding a test for this, similar to src\tests\nativeaot\CustomMain. Just to make sure we don't break this. If there's a test, it won't be broken. No guarantees without a test. |
Oh. Yeah that's not good, we don't want to take dependencies on undocumented stuff 😅 |
Closes #103451
This PR changes the NativeAOT .targets on Windows to publish a .dll for executables using
CustomNativeMain
.