Exclude NuGet cache from project file search in ProjectLocator#15388
Exclude NuGet cache from project file search in ProjectLocator#15388MO2k4 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
When running `aspire update` from a broad directory (e.g. home), the ProjectLocator would descend into ~/.nuget/packages and incorrectly surface immutable template packages as updatable AppHost projects. Replace Directory.GetFiles with FileSystemEnumerable to skip the NuGet cache directory via ShouldRecursePredicate. The cache path is resolved from NUGET_PACKAGES env var or the platform default. Adds three tests covering cache exclusion, normal discovery, and custom cache paths. Fixes microsoft#14055
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15388Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15388" |
| } | ||
|
|
||
| [Fact] | ||
| public async Task FindAppHostProjectFilesAsync_RespectsCustomNuGetPackagesEnvVar() |
There was a problem hiding this comment.
Tests shouldn't set env variables like this, it affects the ability to run concurrently and can impact other tests.
There was a problem hiding this comment.
Thanks for the feedback, i've adjusted this and now switched to CliExecutionContext
…liExecutionContext
mitchdenny
left a comment
There was a problem hiding this comment.
Good fix for a real user-facing issue — the FileSystemEnumerable approach with ShouldRecursePredicate is the right call for skipping the NuGet cache early instead of filtering after the fact.
One bug to address: the StartsWith check in ShouldRecursePredicate lacks a trailing directory separator, which means it could false-positive on sibling directories whose names share a prefix with the cache path (e.g., .nuget/packages-extra). See inline comment for the suggested fix.
Tests are well-structured and cover the key scenarios.
| !entry.IsDirectory && FileSystemName.MatchesSimpleExpression(pattern, entry.FileName), | ||
| ShouldRecursePredicate = (ref FileSystemEntry entry) => | ||
| nugetCachePath is null || |
There was a problem hiding this comment.
Bug: StartsWith without a trailing directory separator can produce false positives. For example, if the NuGet cache path is /home/user/.nuget/packages, a sibling directory named /home/user/.nuget/packages-extra would also be incorrectly excluded because "/home/user/.nuget/packages-extra".StartsWith("/home/user/.nuget/packages") is true.
The codebase already handles this correctly in BundleService.cs (zip-slip protection) by appending Path.DirectorySeparatorChar before comparing. Suggested fix:
ShouldRecursePredicate = (ref FileSystemEntry entry) =>
{
if (nugetCachePath is null)
{
return true;
}
var dirPath = entry.ToFullPath();
return !dirPath.Equals(nugetCachePath, pathComparison)
&& !dirPath.StartsWith(nugetCachePath + Path.DirectorySeparatorChar, pathComparison);
}The extra Equals check handles the exact cache root directory itself (since the StartsWith with trailing separator won't match it).
Append Path.DirectorySeparatorChar to the NuGet cache path in ShouldRecursePredicate so sibling directories whose names share a prefix (e.g. packages-extra vs packages) are not incorrectly excluded. Add FindAppHostProjectFilesAsync_DoesNotExcludeSiblingDirectoriesOfNuGetCache test to verify the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move FileSystemEnumerable setup (pattern matching + NuGet cache exclusion) into a static FindMatchingFiles method, keeping FindAppHostProjectFilesAsync focused on the higher-level flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
aspire updateincorrectly surfacing immutable NuGet template packages (e.g.aspire.projecttemplates) as updatable AppHost projects when run from a broad directory like~Directory.GetFileswithFileSystemEnumerableusingShouldRecursePredicateto skip the NuGet cache directory before descending into itNUGET_PACKAGESenv var or the platform-appropriate default (~/.nuget/packages)Fixes #14055
Test plan
FindAppHostProjectFilesAsync_ExcludesProjectsInsideNuGetCache— verifies projects inside the NuGet cache are excludedFindAppHostProjectFilesAsync_FindsProjectsOutsideNuGetCache— verifies normal project discovery still worksFindAppHostProjectFilesAsync_RespectsCustomNuGetPackagesEnvVar— verifies customNUGET_PACKAGESpaths are honored