Add IsAotCompatible flag to enable all analyzers and IsTrimmable#31766
Add IsAotCompatible flag to enable all analyzers and IsTrimmable#31766agocke merged 7 commits intodotnet:mainfrom
Conversation
ghost
added
agocke
changed the title
| // The below relies on the build above being cached so that no warnings are produced | ||
| // If this fails, it's likely because the build above was not cached | ||
| var publishCommand = new PublishCommand(Log, Path.Combine(testAsset.TestRoot, testProject.Name)); | ||
| publishCommand | ||
| .Execute(RuntimeIdentifier) | ||
| .Should().Pass() | ||
| .And.NotHaveStdOutContaining("warning IL3050") | ||
| .And.NotHaveStdOutContaining("warning IL3056") | ||
| .And.NotHaveStdOutContaining("warning IL2026") | ||
| .And.NotHaveStdOutContaining("warning IL3002"); |
There was a problem hiding this comment.
IMO - this is kind of weird behavior to test. Is there a reason we are verifying this in the test?
There was a problem hiding this comment.
I asked myself -- what's the difference between IsAotCompatible and PublishAot? And came up with -- IsAotCompatible just does analysis, it doesn't actually enable trimming. Seems like we should have a test to verify that they are semantically distinct.
There was a problem hiding this comment.
what's the difference between IsAotCompatible and PublishAot? And came up with -- IsAotCompatible just does analysis, it doesn't actually enable trimming.
Maybe it would make more sense to verify this another way then? Using the "does it emit warnings?" as a verification for "did it publish for AOT?" doesn't seem like a direct way to verify it - especially if that rule can fail, like mentioned in the comments.
Is there a better way to check if the app was published for AOT?
There was a problem hiding this comment.
Maybe you could check for the native subdirectory that would be present if it were published for AOT.
There was a problem hiding this comment.
I think checking the warnings is fine. That's also how we verify trimming for the linker. I've changed the code to more reliably detect the warnings, so I'm not worried about caching affecting things now.
src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAnAotApp.cs
Show resolved
Hide resolved
3 participants
Fixes #31284