Skip to content

change test for file path to account for empty string#15109

Merged
brettfo merged 1 commit into
mainfrom
dev/brettfo/nuget-empty-path
May 22, 2026
Merged

change test for file path to account for empty string#15109
brettfo merged 1 commit into
mainfrom
dev/brettfo/nuget-empty-path

Conversation

@brettfo
Copy link
Copy Markdown
Contributor

@brettfo brettfo commented May 21, 2026

A review of telemetry found an error in the wild where a repo specifies that it uses Central Package Versions but then has no Packages.props file. This is most likely a copy/paste error on the repo owner's part, but nevertheless a sentinel property UsingMicrosoftCentralPackageVersionsSdk is set to true which set off the process of finding the path to the expected Packages.props but when pulling out the next property CentralPackagesFile an empty string was returned.

The fix is to change the test is not null to !string.IsNullOrWhiteSpace since an empty file name isn't possible and a check a few lines down will then revert back to "Default" package management so normal updates can proceed.

This is such an edge case I don't even want to honor it with a test, so a detailed comment in the source will have to suffice.

@github-actions github-actions Bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label May 21, 2026
@brettfo brettfo marked this pull request as ready for review May 21, 2026 22:46
@brettfo brettfo requested a review from a team as a code owner May 21, 2026 22:46
Copilot AI review requested due to automatic review settings May 21, 2026 22:46
Copy link
Copy Markdown
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 updates NuGet SDK project discovery to treat an empty/whitespace package-management “special file” path the same as missing, allowing discovery to fall back to Default package management when a repo’s MSBuild properties indicate CPM/CPV but don’t provide a usable file path.

Changes:

  • Switches packageManagementFile checks from null-only to string.IsNullOrWhiteSpace.
  • Adds explanatory comments describing the incomplete-repo configuration scenario and the intended fallback behavior.
Show a summary per file
File Description
nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs Treats empty/whitespace package-management file paths as missing so discovery can fall back to default package management.

Copilot's findings

Comments suppressed due to low confidence (1)

nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs:684

  • When forcing packageManagementKind back to Default due to a missing/blank management file, packageManagementFile can remain as an empty/whitespace string. That value then gets emitted as PackageManagementSpecialFileRelativePath and can also win during merge (since it's non-null), which may reintroduce the downstream path issues you’re trying to avoid. Consider normalizing blank values to null (e.g., set packageManagementFile = null when falling back to Default).
            if (packageManagementKind != PackageManagementKind.Default && string.IsNullOrWhiteSpace(packageManagementFile))
            {
                logger.Warn($"Project [{projectRelativePath}] detected package management kind of {packageManagementKind} but no package management file found; forcing management kind to {PackageManagementKind.Default}.");
                packageManagementKind = PackageManagementKind.Default;
            }
  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@brettfo brettfo force-pushed the dev/brettfo/nuget-empty-path branch from c7fe85d to 61e09cd Compare May 22, 2026 19:07
@brettfo brettfo merged commit 7398f80 into main May 22, 2026
139 of 140 checks passed
@brettfo brettfo deleted the dev/brettfo/nuget-empty-path branch May 22, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: dotnet:nuget NuGet packages via nuget or dotnet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants