Backport #51237 to release/6.0-preview4 #51672
Conversation
This fix is necessary for dotnet#51237 to work properly. Otherwise, the linker may crash when analyzing certain patterns.
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
agocke
changed the title
* eh fix * test change that inadvertently got checked in earlier * Suppresses the trimmer warning on TypeAnalysis ctor * Incorporating FB * Update src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs Co-authored-by: Eric Erhardt <[email protected]> * Update src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs Co-authored-by: Eric Erhardt <[email protected]> * Update src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs Co-authored-by: Eric Erhardt <[email protected]> * Fix DynamicDependency as per PR feedback * an earlier change got reverted * fixed proj file netcore app condition check * fixed NETCORE_ENGINEERING_TELEMETRY build failures * fixeing another NETCORE_ENGINEERING_TELEMETRY build failures * Adding RequiresUnreferencedCode to TypeAnalysis ctor instead of suppressing the warning to get FB, not fully fixed * PR FB and suppressing warnings for safe calls * propagated the warning all the way up * CI build break fix for one file * excluding NativeRTEventSrc from being build in a project * Missed couple of supppressions on NativeRTEventSrc * build break fixes * Trimmer warning fix related to EventSource manifest creation * incorporate fb * fix build break in some configs * comment feedback * build break Co-authored-by: Eric Erhardt <[email protected]> (cherry picked from commit ddaa1c3)
|
Approved by Tactics over email. |
agocke
added
the
linkable-framework
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer Issue DetailsDescriptionThis fixes the last trimming warning in HelloWorld. The last remaining trim warningis from EventSource – it’s warning about reflection on implementors. The first fix is just a backport of #51237 to add the appropriate annotation to preserve members on types inheriting from EventSource. The fix also requires the linker to understand how to keep types through inheritance, which we implemented shortly before the snap. A bug was found and fixed (dotnet/linker#1972) in that feature just after the snap. That fix is the sole change in the new version of the linker included here. Customer ImpactThere is an unactionable EventSource warning produced by almost all trimming on .NET 6. Regression?
Risk
The change in runtime is just attributes. The change in the linker fixes a bug that would still exist without the fix. Verification
Packaging changes reviewed?
|
|
Tagging subscribers to this area: @tannergooding Issue DetailsDescriptionThis fixes the last trimming warning in HelloWorld. The last remaining trim warningis from EventSource – it’s warning about reflection on implementors. The first fix is just a backport of #51237 to add the appropriate annotation to preserve members on types inheriting from EventSource. The fix also requires the linker to understand how to keep types through inheritance, which we implemented shortly before the snap. A bug was found and fixed (dotnet/linker#1972) in that feature just after the snap. That fix is the sole change in the new version of the linker included here. Customer ImpactThere is an unactionable EventSource warning produced by almost all trimming on .NET 6. Regression?
Risk
The change in runtime is just attributes. The change in the linker fixes a bug that would still exist without the fix. Verification
Packaging changes reviewed?
|
|
Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti Issue DetailsDescriptionThis fixes the last trimming warning in HelloWorld. The last remaining trim warningis from EventSource – it’s warning about reflection on implementors. The first fix is just a backport of #51237 to add the appropriate annotation to preserve members on types inheriting from EventSource. The fix also requires the linker to understand how to keep types through inheritance, which we implemented shortly before the snap. A bug was found and fixed (dotnet/linker#1972) in that feature just after the snap. That fix is the sole change in the new version of the linker included here. Customer ImpactThere is an unactionable EventSource warning produced by almost all trimming on .NET 6. Regression?
Risk
The change in runtime is just attributes. The change in the linker fixes a bug that would still exist without the fix. Verification
Packaging changes reviewed?
|
Will this version of the linker also get backported into the preview4 SDK? |
|
Looks like the preview4 SDK already has a later version (they snapped a couple days later than runtime) https://github.com/dotnet/sdk/blob/c95bbcc2cb86abe93d83782c9812c87655e1c4d2/eng/Versions.props#L77 |
|
Ah, well, even better. |
I would expect it is needed because CI was failing for @LakshanF without the linker bug fix. So I think what you have now is correct. |
ghost
locked as resolved and limited conversation to collaborators
Description
This fixes the last trimming warning in HelloWorld. The last remaining trim warningis from EventSource – it’s warning about reflection on implementors. The first fix is just a backport of #51237 to add the appropriate annotation to preserve members on types inheriting from EventSource. The fix also requires the linker to understand how to keep types through inheritance, which we implemented shortly before the snap. A bug was found and fixed (dotnet/linker#1972) in that feature just after the snap. That fix is the sole change in the new version of the linker included here.
Customer Impact
There is an unactionable EventSource warning produced by almost all trimming on .NET 6.
Regression?
Risk
The change in runtime is just attributes. The change in the linker fixes a bug that would still exist without the fix.
Verification
Packaging changes reviewed?