-
Notifications
You must be signed in to change notification settings - Fork 737
[Feature] Add package source mapping to package download
#6885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Add package source mapping to package download
#6885
Conversation
|
I will mark this PR ready for review, once the following design spec NuGet/Home#14495 gets approved. Or will update the PR if there is design changes. |
package downloadpackage download
|
Also waiting on #6804 to merge first |
52f9b05 to
9ae9541
Compare
cb7c906 to
00a03c1
Compare
e160332 to
4bd63e6
Compare
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
Outdated
Show resolved
Hide resolved
2d404a2
into
dev-feature-package-download
| null! | ||
| }; | ||
|
|
||
| // no --source, mapping -> B, allow insecure not enabled -> fail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are missing a test with no --source, mapping -> A,B, B has latest -> retrieves the latest package from B.
| /// applying package source mapping | ||
| /// validating HTTP usage only on the *effective* sources. | ||
| /// </summary> | ||
| private static bool TryGetRepositoriesForPackage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm finding this method really hard to read/maintain. There's too many nested control statements and three different code paths for a return statement. One suggestion would be to factor the foreach statement out into its own helper function. Ideally, there should be a single return statement.
Bug
Fixes: NuGet/Home#14607
Description
This PR
package downloadcommandTLDR
--sourceis specified, the command now prioritizes the provided source(s) and skips package source mapping--sourceis not specified, the command reads sources fromnuget.configand applies package source mapping to determine which source to use for each package.PR Checklist