-
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
ILC should run after ComputeResolvedFilesToPublishList #108909
Comments
We should ideally redo how the compiler is hooked up into publish in general. The way it's hooked up right now was inherited from CoreRT and never touched since. SDK is now allowed to know about PublishAot and we should use same mechanisms as PublishTrimmed and PublishSingleFile use (that the SDK knows about and special cases). It would fix other problems, like dotnet/sdk#40488. |
@sbomer Could you put this on your backlog? I think it's time to cleanup publishing for AOT and get this into a good state. |
For a servicing fix that addresses #109872, we may want to do a targeted change. E.g. take file versions into account in https://github.com/dotnet/runtime/blob/main/src/coreclr/tools/aot/ILCompiler.Build.Tasks/ComputeManagedAssembliesToCompileToNative.cs#L154 and let the file with higher version win. |
There are at least five places that can potentially do conflict resolution in a native AOT publish:
The goal is for ILC's special handling to go away and to use the built-in conflict resolution. There are multiple issues here:
|
Sorry to ask, but where are things with this issue? All the issues raised around this are closed, and while there has since been an SDK update I don't think is yet resolved? At the moment using AOT at all feels very precarious since these v9 dependencies can creep in as transitive dependencies very easily (as occurred with OpenTelemetry with 1.10.0 which first gave me the issue). Would be great to get an update. |
This issue tracks cleaning up how native AOT publish is hooked up. The NAOT dependency conflict resolution bug was tracked by #109872, and the fix was back ported to both .NET 8 and .NET 9 SDKs: #110059 and #110058 . If you still see a issues with NAOT dependency conflict resolution in the latest .NET SDK updates, please open a new issue on it. |
Native AOT hooks into the publish logic using
BeforeTargets="ComputeResolvedFilesToPublishList"
:runtime/src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.Publish.targets
Lines 20 to 22 in 44b7ba3
However, this target has some conflict resolution that should be done before ILC runs:
https://github.com/dotnet/sdk/blob/456aa42f693f9ff022c7e5f9f7473e0dc5d3b2f4/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L542-L559
I noticed this in a local build where ILLink was picking a different version of an assembly than ILC (because ILLink runs after
ComputeResolvedFilesToPublishList
:https://github.com/dotnet/sdk/blob/456aa42f693f9ff022c7e5f9f7473e0dc5d3b2f4/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets#L453-L456
The
ilc.rsp
file in my scenario has both:ILC seems to use the first dll passed on the command-line.
Compare this with the PublishSelfContained output. During
ComputeResolvedFilesToPublishList
, the conflict is resolved and only the dll from the nuget package is removed, leaving the one from a local build. The local build is passed to ILLink.Unfortunately I wasn't able to come up with a simple repro, but the project file looked like this:
Binlogs available upon request.
The text was updated successfully, but these errors were encountered: