-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Validate AoT compatibility #1745
Conversation
Add a console project that validates whether the new-in-v8 Polly assemblies are AoT compatible. Relates to App-vNext#1732.
6b03918
to
c17fa37
Compare
test/Polly.AotTest/Program.cs
Outdated
|
||
// See https://github.com/App-vNext/Polly/issues/1732#issuecomment-1782466692. | ||
// This code is needed as a workaround until https://github.com/dotnet/runtime/issues/94131 is resolved. | ||
var pipeline = CompositeComponent.Create(new[] { PipelineComponent.Empty, PipelineComponent.Empty }, null!, null!); |
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.
Can we create a more elaborate pipeline (possibly of all strategies we have).
Would cover much more code that way.
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.
As I understood it, this is only needed to detect the generic cycles that are missed in the analyzer due to dotnet/runtime#94131. Otherwise, all the code is inspected by the analyser through the presence of the <TrimmerRootAssembly>
items, so we don't actually need to exercise all the code paths in the actual program except for this specific case.
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 TrimmerRootAssembly
items in the project will "root" all code in the assemblies it specified, regardless if it's used by the Main
here. The compiler will also analyze all of that code, and report warnings if there are any.
I would think you should not need any code in Main
, but generic cycle detection is tricky, so it might need to be "Triggered" like this. You should try to remove this though and see if it still reports the cycle (I hope it will).
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 like you're right, the hint is redundant when compiling.
Compilation still fails without the hints.
This PR demonstrates the repro finds the issue; the changes have been integrated into #1737. |
Add native AoT project to repro issues from #1732 and #1744.
Builds on top of #1743, so needs rebasing once that is merged.