Skip to content

Include resolved tool version in invalid DotnetToolSettings install/update errors - #54822

Merged
baronfel merged 3 commits into
mainfrom
copilot/fix-dotnet-tool-version-logging
Jun 23, 2026
Merged

baronfel merged 3 commits into
mainfrom
copilot/fix-dotnet-tool-version-logging

Conversation

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

dotnet tool install/update could surface DotnetToolSettings.xml errors without indicating which package version was actually resolved, making tool-author diagnosis harder when version selection/ranges were involved. This updates the error path to include concrete package identity details.

  • Error payload now includes resolved package identity

    • MissingToolSettingsFile now formats with package id + resolved version.
    • Message changed from a generic package-level error to:
      • Settings file 'DotnetToolSettings.xml' was not found in package '{id}@{version}'.
  • Version propagation at throw site

    • In ToolPackageInstance, the missing-settings exception now uses the resolved library.Version (the concrete selected version), not just package id context.
    • Added a small helper to centralize formatting for this specific error path.
  • Focused unit coverage

    • Added ToolPackageInstanceTests to assert the missing-settings message includes package identity in <package>@<version> form.
throw new ToolConfigurationException(
    GetMissingToolSettingsFileMessage(packageId, library.Version));

internal static string GetMissingToolSettingsFileMessage(PackageId packageId, NuGetVersion packageVersion)
    => string.Format(CliStrings.MissingToolSettingsFile, packageId, packageVersion.ToNormalizedString());

Copilot AI changed the title [WIP] Fix dotnet tool install to log bad version information Include resolved tool version in invalid DotnetToolSettings install/update errors Jun 17, 2026
Copilot AI requested a review from baronfel June 17, 2026 14:25
Comment thread src/Cli/dotnet/CliStrings.resx Outdated
@baronfel
baronfel marked this pull request as ready for review June 17, 2026 14:27
Copilot AI review requested due to automatic review settings June 17, 2026 14:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the dotnet tool install/update error surfaced when DotnetToolSettings.xml is missing so it includes the resolved package identity (ID + concrete version), improving diagnosability when version ranges/floating resolution are involved.

Changes:

  • Update MissingToolSettingsFile to include package ID and resolved version.
  • Propagate the resolved library.Version into the thrown ToolConfigurationException via a small helper.
  • Add a unit test asserting the error message includes both package name and version.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/Microsoft.DotNet.PackageInstall.Tests/ToolPackageInstanceTests.cs Adds coverage for the new error message formatting (ID + version).
src/Cli/dotnet/ToolPackage/ToolPackageInstance.cs Uses resolved library.Version and centralizes formatting in a helper method.
src/Cli/dotnet/CliStrings.resx Updates the localized string template to include {0} (id) and {1} (version).

Comment thread src/Cli/dotnet/CliStrings.resx
Copilot AI requested a review from baronfel June 17, 2026 14:41
Copilot AI and others added 3 commits June 17, 2026 09:42
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
@baronfel
baronfel force-pushed the copilot/fix-dotnet-tool-version-logging branch from bf0a00f to c595959 Compare June 17, 2026 14:42
@baronfel
baronfel requested review from a team and richlander June 17, 2026 14:43
@baronfel baronfel added the cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on) label Jun 17, 2026
@baronfel
baronfel enabled auto-merge (squash) June 17, 2026 14:43
@baronfel

Copy link
Copy Markdown
Member

/ba-g unrelated flaky test failure in static web assets

@baronfel
baronfel requested a review from dsplaisted June 23, 2026 17:41
@baronfel
baronfel merged commit 88e93d6 into main Jun 23, 2026
24 of 26 checks passed
@baronfel
baronfel deleted the copilot/fix-dotnet-tool-version-logging branch June 23, 2026 22:03
dsplaisted added a commit that referenced this pull request Jun 24, 2026
ToolPackageInstanceTests.cs (added by #54822) used the xUnit [Fact] attribute, but
Microsoft.DotNet.PackageInstall.Tests was migrated to MSTest.Sdk (#54822 was validated
on a pre-migration base and merged without re-running CI against current main, so the
incompatibility wasn't caught). The project no longer references xUnit, so [Fact] fails
to compile with CS0246, breaking the build on main.

Use [TestClass]/[TestMethod] to match the rest of the (MSTest) test project.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-preview6 milestone Jun 24, 2026
nagilson pushed a commit that referenced this pull request Jul 6, 2026
ToolPackageInstanceTests.cs (added by #54822) used the xUnit [Fact] attribute, but
Microsoft.DotNet.PackageInstall.Tests was migrated to MSTest.Sdk (#54822 was validated
on a pre-migration base and merged without re-running CI against current main, so the
incompatibility wasn't caught). The project no longer references xUnit, so [Fact] fails
to compile with CS0246, breaking the build on main.

Use [TestClass]/[TestMethod] to match the rest of the (MSTest) test project.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dkurepa pushed a commit that referenced this pull request Jul 7, 2026
ToolPackageInstanceTests.cs (added by #54822) used the xUnit [Fact] attribute, but
Microsoft.DotNet.PackageInstall.Tests was migrated to MSTest.Sdk (#54822 was validated
on a pre-migration base and merged without re-running CI against current main, so the
incompatibility wasn't caught). The project no longer references xUnit, so [Fact] fails
to compile with CS0246, breaking the build on main.

Use [TestClass]/[TestMethod] to match the rest of the (MSTest) test project.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants