Multi-RID tool packages usability tweaks

[baronfel]

This PR:

• Migrates from an Item-based usage mode (<ToolPackageRuntimeIdentifier Include="linux-x64" Version="A.B.C" />) to a property-based usage mode (<ToolPackageRuntimeIdentifier>linux-x64</ToolPackageRuntimeIdentifier>).
  • This implies the removal of the Version from the RID-specific tool package configuration entries (the versions must always be aligned with the 'parent' RID-less package)
• Simplifies some of the packaging of the Tool packages
• Makes it so packaging the 'parent' package via dotnet pack triggers the packaging of the 'inner' per-RID packages as well
  • Caveat - this won't ever work if we're in AOT mode, so we don't do that for PublishAOT packages.

[Copilot]

Pull Request Overview

This PR adjusts the packaging mechanism for RID-specific tool packages by migrating from an item‐based to a property‐based usage mode, streamlining package creation for both parent and RID‐specific packages.

• Migrates from using an Item-based RID specification with a Version attribute to a property-based configuration without a version.
• Updates various build targets to correctly trigger and define inner and outer builds, including adjustments to packaging tasks and property conditions.
• Removes legacy code (such as the version handling in tool package resolution and associated unit tests) to simplify the overall tool package design.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets	Updates PackTool import condition to respect PackAsTool usage.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Common.targets	Adjusts condition to reference the new property ToolPackageRuntimeIdentifiers.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets	Refactors packaging targets to differentiate inner and outer builds and update package paths.
src/Tasks/Microsoft.NET.Build.Tasks/GenerateToolsSettingsFile.cs	Removes version attribute assignment in RID package nodes.
src/Cli/dotnet/ToolPackage/ToolPackageDownloaderBase.cs	Updates package version sourcing for installation and asset file creation.
src/Cli/dotnet/ToolPackage/ToolConfigurationDeserializer.cs	Eliminates version information when creating PackageIdentity from RID-specific data.
src/Cli/dotnet/ToolPackage/ToolConfigurationDeserialization/DotNetCliToolRuntimeIdentifierPackage.cs	Removes the Version attribute in favor of property-based configuration.
(Other files)	Removal of legacy test and resolution files in-line with the new design.

Comments suppressed due to low confidence (3)

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.Common.targets:57

• Verify that the new property 'ToolPackageRuntimeIdentifiers' is correctly defined and consistently propagated across the targets to avoid mismatches with legacy item names.

<_ToolPackageType Condition="'$(RuntimeIdentifier)' != '' And '$(ToolPackageRuntimeIdentifiers)' != ''">DotnetToolRidPackage</_ToolPackageType>

src/Cli/dotnet/ToolPackage/ToolConfigurationDeserializer.cs:70

• Ensure that all consumers of PackageIdentity are updated to handle a null version, as the version is now inherited from the parent package rather than being stored per RID-specific package.

var ridSpecificPackages = dotNetCliTool.RuntimeIdentifierPackages?.ToDictionary(p => p.RuntimeIdentifier, p => new PackageIdentity(p.Id, null))

src/Cli/dotnet/ToolPackage/ToolPackageDownloaderBase.cs:291

• Confirm that the 'packageVersion' variable is correctly assigned before this check, as replacing resolvedPackage.Version could lead to unintended behavior if packageVersion is not properly derived.

if (!IsPackageInstalled(new PackageId(resolvedPackage.Id), packageVersion, packageDownloadDir.Value))

[baronfel]

All of the failing tests are around packaging the shims. After my changes the shims are getting the NuGetRecursiveDir metadata added onto them, which is causing them to be put into additional nested subfolders due to logic in the Pack targets:

NuGet.Build.Tasks.Pack.targets:

  <Target Name="_GetTfmSpecificContentForPackage"
          DependsOnTargets="$(TargetsForTfmSpecificContentInPackage)"
          Returns="@(TfmSpecificPackageFileWithRecursiveDir)">
    <!-- The below workaround needs to be done due to msbuild bug https://github.com/Microsoft/msbuild/issues/3121 -->
    <ItemGroup>
      <TfmSpecificPackageFileWithRecursiveDir Include="@(TfmSpecificPackageFile)">
        <NuGetRecursiveDir>%(TfmSpecificPackageFile.RecursiveDir)</NuGetRecursiveDir>
        <BuildAction>%(TfmSpecificPackageFile.BuildAction)</BuildAction>
      </TfmSpecificPackageFileWithRecursiveDir>
    </ItemGroup>
  </Target>

Bad item:

Leads to bad content:

[baronfel]

I think I've got it - since NuGet forces the use of RecursiveDir if it's present, we just lean into that and don't try to be quite as precise with our PackagePath setting. I've pushed up a commit with comments and adjustments for that.

[baronfel]

Alright - this is super close so is probably ok for review now. The only tests failing don't fail locally, and cover a pretty specific scenario: It_uses_customized_PackagedShimOutputRootDirectory.

[baronfel]

Ok, I have added a test to cover the new multi-packaging feature, and have verified via binlogs locally that the shim output path stuff works. I want helix to try one more time to give me a binlog so I can see what's going on in the CI system, but then I want to skip those tests so we can get the good syntax in for p6.

[baronfel]

Ok we're good to go now! The test failures are watch and container timeouts, everything else is good.

[nagilson]

Another PR I wish I had more time to look over but I did what I could with the time we have :) I think using properties vs items is a great improvement and overall the change looks good. Kudos for the clever engineering here...

[baronfel]

I've addressed @nagilson's feedback (thanks!) and this should be good to merge now.

[nagilson]

mmmmm yes

[baronfel]

The only remaining failing test is the completions known issue. Let's merge!

[marcpopMSFT]

Known completion test issue.

[dsplaisted]

@baronfel Late review question about having different versions for RID-specific packages.

[dsplaisted]

Late review: Does this now mean that it's not possible to have different versions for the RID-specific packages? I thought we weren't going to support different version numbers for package production, but we were still going to support consuming packages where the primary package has a different version from the RID-specific packages if someone creates packages like that through some other means.

[baronfel]

Correct - I refactored that out in this pass. We could add it back (you're right that the production-side only supports making a single version across all tools), but I am not convinced that we'll ever actually see that scenario happen: IMO the only time you have separate packages being made now is related to AOT packaging, and even that should be really aligned version-wise.