Uh oh!
There was an error while loading. Please reload this page.
Use current runtime version in ILLink.Tasks package - #90197
Conversation
ghost
commented
Aug 8, 2023
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsFixes #90015 The ILLink.Tasks package should have a runtimeconfig.json that matches the runtime version bundled with the SDK that references it. This used to be done by stomping the runtime version in the dotnet/sdk build, but ILLink.Tasks is no longer bundled with the SDK so we need to do this ourselves. Tests that use live ILLink bits need to continue working with the SDK specified in global.json, so this preserves the existing illink.runtimeconfig.json in the build output, but defines an additional illink.runtimeconfig.pack.json file that uses the current runtime version. The pack.json file is included in the package (with the name illink.runtimeconfig.json) instead of the original. This includes a fix for #88929 - we weren't actually using live illink for tests due to an ordering issue that I introduced in response to feedback on the PR. @ViktorHofer the Directory.Build.props is imported too early (before the nuget props that this is trying to override) so I think we need to import Microsoft.NET.ILLink.props directly from the project file - but let me know if you can see a better solution.
|
Uh oh!
There was an error while loading. Please reload this page.
ViktorHofer
left a comment
There was a problem hiding this comment.
Would using something similar to
runtime/eng/targetingpacks.targets
Lines 167 to 174 in 943af3a
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
sbomer
commented
Aug 9, 2023
Unfortunately no - using |
| <_BuildOutputPackageFile Include="$(OutputPath)**" | ||
| Exclude="$(OutputPath)publish\**; | ||
| $(OutputPath)" /> | ||
| <TfmSpecificPackageFile Include="@(_BuildOutputPackageFile)" |
There was a problem hiding this comment.
Can you please explain this change? I'm not sure what it is doing.
There was a problem hiding this comment.
Before this change, the PackagePath was just tools/$(TargetFramework)/ - the %(RecursiveDir)%(FileName)%(Extension) part was empty because the itemgroup hadn't been defined, and the build output was showing:
packaging.targets(280,30): message : MSB4120: Item 'TfmSpecificPackageFile' definition within target references itself via (qualified or unqualified) metadatum 'RecursiveDir'. This can lead to unintended expansion and cross-applying of pre-existing items. More info: https://aka.ms/msbuild/metadata-self-ref [/home/svbomer/src/runtime/src/tools/illink/src/ILLink.Tasks/ILLink.Tasks.csproj::TargetFramework=net8.0]
I needed the filename to be included to make it easier to filter later.
Fixes#90015
The ILLink.Tasks package should have a runtimeconfig.json that matches the runtime version bundled with the SDK that references it. This used to be done by stomping the runtime version in the dotnet/sdk build, but ILLink.Tasks is no longer bundled with the SDK so we need to do this ourselves.
Tests that use live ILLink bits need to continue working with the SDK specified in global.json, so this preserves the existing illink.runtimeconfig.json in the build output, but defines an additional illink.runtimeconfig.pack.json file that uses the current runtime version. The pack.json file is included in the package (with the name illink.runtimeconfig.json) instead of the original.
This includes a fix for #88929 - we weren't actually using live illink for tests due to an ordering issue that I introduced in response to feedback on the PR. @ViktorHofer the Directory.Build.props is imported too early (before the nuget props that this is trying to override) so I think we need to import Microsoft.NET.ILLink.props directly from the project file - but let me know if you can see a better solution.