Skip to content

Conversation

@baronfel
Copy link
Member

@baronfel baronfel commented Jun 20, 2025

This builds on top of #49501 and #49521, so it's in draft still.

Fixes #49499 by adding support for the any RID. This RID can be specified, but if it is it must never trigger any of the apphost, linking, etc related Publish* properties. So I've added clauses to always force the Publish* properties to false for the any RID, and then everything else more or less falls into place!

I've been able to test this end to end by removing the win-x64 RID from my set of supported RIDs and then running the generated tool. Because we do RID negotiation, and because the any RID is explicitly packed as a FDD tool, everything just works!

All of the actual changes are in the .targets file, the rest is just

  • supporting tests/infrastructure
  • adding more diag logging to tool execution in case we need to debug things

@baronfel baronfel marked this pull request as ready for review June 20, 2025 04:45
Copilot AI review requested due to automatic review settings June 20, 2025 04:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for the special "any" RID in multi-RID tools by ensuring that Publish* properties are forced to false when "any" is specified. Key changes include updating test tool settings to add Trimmed and IncludeAnyRid flags, extending end-to-end tests to verify the new behavior, and modifying MSBuild targets to conditionally disable publish features when the "any" RID is used.

Reviewed Changes

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

File Description
test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs Adds new properties and updates identifier string logic to include "trimmed" and "anyrid" suffixes.
test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs Introduces additional tests covering trimmed and any RID scenarios.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets Adjusts MSBuild conditions to disable publish features for RID-agnostic builds and adds a new property to detect RID-specific builds.
Comments suppressed due to low confidence (2)

test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs:40

  • [nitpick] Consider adding parentheses to the nested ternary expressions in the GetIdentifier method for improved readability and maintainability.
            public string GetIdentifier() => $"{ToolPackageId}-{ToolPackageVersion}-{ToolCommandName}-{(NativeAOT ? "nativeaot" : SelfContained ? "selfcontained" : Trimmed ? "trimmed" : "managed")}{(IncludeAnyRid ? "-anyrid" : "")}";

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets:140

  • The removal of quotes around $(RuntimeIdentifier) in the condition may affect string comparison behavior; please verify that the condition is evaluated as intended.
    <PropertyGroup Condition="'$(_HasRIDSpecificTools)' != '' And $(RuntimeIdentifier) != ''">

@baronfel baronfel force-pushed the tinker-any-rid-tool branch 6 times, most recently from a6c399a to 7abd619 Compare June 25, 2025 14:42
@baronfel
Copy link
Member Author

Rebased now that the two parent PRs have made it into main. Let's see if that has cleared up the ILLink failures or if I have some more work to do here.

@baronfel baronfel force-pushed the tinker-any-rid-tool branch from fd4e6bb to e9d2919 Compare June 26, 2025 19:36
@baronfel baronfel force-pushed the tinker-any-rid-tool branch from 983d928 to 38e7482 Compare June 27, 2025 01:19
@baronfel baronfel requested a review from a team June 27, 2025 12:36
Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing cleanup and #nullable disable removals as you were going along. I really appreciate it! 🤩

@baronfel baronfel enabled auto-merge (squash) June 27, 2025 23:10
@baronfel baronfel merged commit 401ace4 into dotnet:main Jun 28, 2025
26 checks passed
@baronfel baronfel deleted the tinker-any-rid-tool branch June 30, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The new RID-specific tools should support a fallback for framework dependent, RID-agnostic tools

2 participants