C#: Fix buildless fallback restore logic #19050
Conversation
When dotnet core projects are restored, the dependency manager precisely tracks the referenced package folders. The fallback restore logic ignored the precise usage list and instead considered all subfolders in the restore location to be referenced, even though not all subfolders were added to the dependency list. This meant that packages downloaded in partially successful restores were available on disk, but not added to the dependency list by the normal restore process, and skipped by the fallback restore process. This commit fixes this problem by ensuring that the fallback restore logic doesn't consider all subfolders in the restore location to be referenced, but only those that were added to the dependency list by the normal restore process.
|
DCA shows quite a big change in alert count. I'll do a traced-buildless comparison too. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the fallback restore logic in C# so that only dependency-listed packages are considered during fallback restore. It updates method signatures and parameter usage for downloading missing packages and refactors logging and return values in the package directory name method.
- Changed call signatures to pass a used package names list.
- Renamed and refactored the method that logs and returns package directories.
Comments suppressed due to low confidence (1)
csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs:430
- [nitpick] The method GetAllUsedPackageDirNames logs packages as unused while returning those that are used, indicating a mismatch between the method name and its internal logging. Consider adjusting the log messages or the returned values for consistency.
.ForEach(package => logger.LogDebug("Unused package: {package.package}"));
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
| { | ||
| // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. | ||
| var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds(explicitFeeds); | ||
| var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds([], explicitFeeds); |
There was a problem hiding this comment.
Passing an empty array for usedPackageNames on line 112 may bypass filtering based on the dependency list. Verify that this argument value correctly reflects the intended set of used package names.
| var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds([], explicitFeeds); | |
| var usedPackageNames = GetAllUsedPackageDirNames(dependencies); | |
| var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds(usedPackageNames, explicitFeeds); |
2 participants
When dotnet core projects are restored, the dependency manager precisely tracks the referenced package folders. The fallback restore logic ignored the precise usage list and instead considered all subfolders in the restore location to be referenced, even though not all subfolders were added to the dependency list. This meant that packages downloaded in partially successful restores were available on disk, but not added to the dependency list by the normal restore process, and skipped by the fallback restore process. This commit fixes this problem by ensuring that the fallback restore logic doesn't consider all subfolders in the restore location to be referenced, but only those that were added to the dependency list by the normal restore process.