C#: Use the feed manager in the NugetExeWrapper.#22033
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
DCA looks good. |
…istries configuration and provide sources via the command line instead of creating a file.
michaelnebel
force-pushed
the
csharp/usefeedmanager
branch
from
03d6585 to
e30e0a9
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the C# dependency fetching pipeline so packages.config restores performed via nuget.exe use the existing FeedManager (consistent with dotnet restore) and can pass multiple package sources via repeated -Source arguments, aligning with NuGet CLI behavior.
Changes:
- Refactors
packages.configrestore to useFeedManagerfor determining restore sources instead of editingnuget.config. - Introduces shared feed-selection helpers (
FeedsToUse, generalizedFeedsToRestoreArgument) and updatesdotnet restoreto use the renamed source-argument builder. - Simplifies
IPackagesConfigRestoreby removingIDisposableand associated config backup/restore logic.
Show a summary per file
| File | Description |
|---|---|
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/PackagesConfigRestorer.cs | Switch nuget.exe-based packages.config restore to compute -Source args from FeedManager rather than mutating nuget.config. |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs | Wires the new FeedManager-driven packages.config restore and updates dotnet restore source-argument method usage. |
| csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FeedManager.cs | Adds reusable feed selection (FeedsToUse) and generalizes source-argument generation across restore tools. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 4
| var feedsToUse = feedManager.FeedsToUse(packagesConfig, reachableFeeds).ToList(); | ||
| var useDefaultFeed = feedsToUse.Count == 0 && IsDefaultFeedReachable; |
There was a problem hiding this comment.
I think we should also use the default feed for windows (if it is reachable and we don't find any other feeds). The PR where this was originally introduced states, that this case was implemented specifically for MacOS 14, because there is no user/machine specific feed on that image (which can explain why Windows was excluded). Most likely our windows images will have user/machine defaults, but I don't think we should assume that in our implementation.
michaelnebel
force-pushed
the
csharp/usefeedmanager
branch
from
d8fa611 to
9bce72a
Compare
|
Running DCA again. It might be worth running a QA experiment for this PR as well. |
Just like for
dotnet restoreis it possible to specify the NuGet sources directly when using thenuget.exeCLI. The documentation for the NuGet CLI can be seen here and it states that it is possible to add (multiple) sources by providing (multiple)-Sourcearguments.In this PR we simplify and streamline the use nuget sources when downloading dependencies using
[mono] nuget.exe. That is, wenuget.configfiles in the checked out repo. Instead we supply the sources with the-Source.It is worth noting that we start to use
dotnet nugetto detect the sources needed for the restore.