Reference ILLink analyzer matching ILLink version #32045
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,7 @@ public class ProcessFrameworkReferences : TaskBase | |
|
|
||
| public bool ReadyToRunUseCrossgen2 { get; set; } | ||
|
|
||
| public bool RequiresILLinkPack { get; set; } | ||
|
|
||
| public bool AotEnabled { get; set; } | ||
|
|
||
|
|
@@ -384,7 +384,7 @@ var runtimeRequiredByDeployment | |
| } | ||
| } | ||
|
|
||
| if (RequiresILLinkPack) | ||
| { | ||
| if (!AddToolPack(ToolPackType.ILLink, _normalizedTargetFrameworkVersion, packagesToDownload, implicitPackageReferences)) | ||
| { | ||
|
|
@@ -712,6 +712,15 @@ private bool AddToolPack( | |
| implicitPackageReferences.Add(buildPackage); | ||
| } | ||
|
|
||
| // Before net8.0, ILLink analyzers shipped in a separate package. | ||
| // Add the analyzer package with version taken from KnownILLinkPack. | ||
| if (normalizedTargetFrameworkVersion < new Version(8, 0) && toolPackType is ToolPackType.ILLink) | ||
| { | ||
| var analyzerPackage = new TaskItem("Microsoft.NET.ILLink.Analyzers"); | ||
| analyzerPackage.SetMetadata(MetadataKeys.Version, packVersion); | ||
| implicitPackageReferences.Add(analyzerPackage); | ||
|
Comment on lines
+719
to
+721
There was a problem hiding this comment. Just curious: where do we download this from? It seems it's not on NuGet... is the SDK adding some other public feeds for this? There was a problem hiding this comment. I think it's downloaded from NuGet: https://www.nuget.org/packages/Microsoft.NET.ILLink.Analyzers/ For internal builds the specific version would be available on an internal feed. There was a problem hiding this comment. That package was submitted by mistake - it only has one 8.0 preview version, it definitely doesn't contain downlevel versions. There was a problem hiding this comment. I'll ask for it to be uploaded to nuget. |
||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,8 +22,14 @@ Copyright (c) .NET Foundation. All rights reserved. | |
| <!-- PublishAot depends on PublishTrimmed. This must be set early enough for the KnownILLinkPack to be restored. --> | ||
| <PublishTrimmed Condition="'$(PublishTrimmed)' == '' And '$(PublishAot)' == 'true'">true</PublishTrimmed> | ||
| <IsTrimmable Condition="'$(IsTrimmable)' == '' and '$(IsAotCompatible)' == 'true'">true</IsTrimmable> | ||
| <_RequiresILLinkPack Condition="'$(_RequiresILLinkPack)' == '' And ( | ||
| '$(PublishTrimmed)' == 'true' Or | ||
| '$(PublishSingleFile)' == 'true' Or | ||
|
There was a problem hiding this comment. Can we rename There was a problem hiding this comment. dotnet/runtime uses that property already. @sbomer can you please take care of updating it when these changes are being consumed? https://github.com/dotnet/runtime/blob/98dd1e83ba8b17299af9d3d78000f8a9c000a12e/eng/illink.targets#L5C1-L6 |
||
| '$(IsTrimmable)' == 'true' Or | ||
| '$(EnableAotAnalyzer)' == 'true' Or | ||
| '$(EnableTrimAnalyzer)' == 'true' Or | ||
| '$(EnableSingleFileAnalyzer)' == 'true')">true</_RequiresILLinkPack> | ||
| <_RequiresILLinkPack Condition="'$(_RequiresILLinkPack)' == ''">false</_RequiresILLinkPack> | ||
| <DynamicCodeSupport Condition="'$(DynamicCodeSupport)' == '' And '$(PublishAot)' == 'true'">false</DynamicCodeSupport> | ||
| </PropertyGroup> | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should the corresponding version number be removed from
Versions.props? Otherwise I believe it will still be there and available for use, but won't ever get updated.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.
It looks like we were using the
ILLink.Taskspackage version, and thisVersion.Details.xmlversion was never used. I removed theILLink.Taskspackagereference and cleaned up some left-over analyzer layout logic that was using it.I left in the
ILLink.Tasksentry inVersions.propsandVersion.Details.xmleven though these are technically unused. We test with anILLink.Tasksversion matchingMicrosoftNETCoreAppRefPackageVersion, which should always be the same, but it kind of seems nice to make the dependency explicit at least inVersions.props. Let me know if you'd prefer me to remove it and I will.