Skip to content

[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.

Sign up for GitHub

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

Merged
Nigusu-Allehu merged 7 commits into from
Oct 31, 2025
Merged

[Feature] Add package source mapping to package download #6885

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4bd63e6
Add package source mapping
Nigusu-Allehu Oct 24, 2025
9f2deed
clean up
Nigusu-Allehu Oct 29, 2025
fa0cfb6
Simplify
Nigusu-Allehu Oct 30, 2025
a0a2be5
cealnup
Nigusu-Allehu Oct 30, 2025
b713d79
handle missing mapping
Nigusu-Allehu Oct 30, 2025
253adf9
perf
Nigusu-Allehu Oct 30, 2025
192c72e
linear
Nigusu-Allehu Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 93 additions & 13 deletions src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/Package/Download/PackageDownloadRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable enable

using System;
using System.Collections.Generic;
using System.Globalization;
Expand Down Expand Up @@ -48,28 +50,45 @@ public static async Task<int> RunAsync(PackageDownloadArgs args, CancellationTok

public static async Task<int> RunAsync(PackageDownloadArgs args, ILoggerWithColor logger, IReadOnlyList<PackageSource> packageSources, ISettings settings, CancellationToken token)
{
// Check for insecure sources
if (DetectAndReportInsecureSources(args.AllowInsecureConnections, packageSources, logger))
var packageSourceMapping = PackageSourceMapping.GetPackageSourceMapping(settings);
var hasSourcesArg = args.Sources != null && args.Sources.Count > 0;
var mappingDisabled = (packageSourceMapping != null && !packageSourceMapping.IsEnabled) || packageSourceMapping == null;
if ((mappingDisabled || hasSourcesArg) && DetectAndReportInsecureSources(args.AllowInsecureConnections, packageSources, logger))
{
return ExitCodeError;
}

string outputDirectory = args.OutputDirectory ?? Directory.GetCurrentDirectory();
var cache = new SourceCacheContext();
IReadOnlyList<SourceRepository> sourceRepositories = GetSourceRepositories(packageSources);

IReadOnlyDictionary<string, SourceRepository> sourceRepositoriesMap = GetSourceRepositories(packageSources);

bool downloadedAllSuccessfully = true;

foreach (var package in args.Packages)
foreach (var package in args.Packages ?? [])
{
logger.LogMinimal(string.Format(
CultureInfo.CurrentCulture,
Strings.PackageDownloadCommand_Starting,
package.Id,
string.IsNullOrEmpty(package.NuGetVersion?.ToNormalizedString()) ? Strings.PackageDownloadCommand_LatestVersion : package.NuGetVersion.ToNormalizedString()));

// Resolve which repositories to use for this package
if (!TryGetRepositoriesForPackage(
package.Id,
args,
packageSources,
packageSourceMapping,
sourceRepositoriesMap,
logger,
out List<SourceRepository> sourceRepositories))
{
return ExitCodeError;
}

try
{
(NuGetVersion version, SourceRepository downloadRepository) =
(NuGetVersion? version, SourceRepository? downloadRepository) =
await ResolvePackageDownloadVersion(
package,
sourceRepositories,
Expand All @@ -88,7 +107,7 @@ await ResolvePackageDownloadVersion(
bool success = await DownloadPackageAsync(
package.Id,
version,
downloadRepository,
downloadRepository!,
cache,
settings,
outputDirectory,
Expand Down Expand Up @@ -127,16 +146,16 @@ await ResolvePackageDownloadVersion(
return downloadedAllSuccessfully ? ExitCodeSuccess : ExitCodeError;
}

internal static async Task<(NuGetVersion, SourceRepository)> ResolvePackageDownloadVersion(
internal static async Task<(NuGetVersion?, SourceRepository?)> ResolvePackageDownloadVersion(
PackageWithNuGetVersion packageWithNuGetVersion,
IEnumerable<SourceRepository> sourceRepositories,
SourceCacheContext cache,
ILoggerWithColor logger,
bool includePrerelease,
CancellationToken token)
{
NuGetVersion versionToDownload = null;
SourceRepository downloadSourceRepository = null;
NuGetVersion? versionToDownload = null;
SourceRepository? downloadSourceRepository = null;
bool versionSpecified = packageWithNuGetVersion.NuGetVersion != null;

foreach (var repo in sourceRepositories)
Expand Down Expand Up @@ -188,6 +207,67 @@ await ResolvePackageDownloadVersion(
return (versionToDownload, downloadSourceRepository);
}

/// <summary>
/// Builds the set of SourceRepository objects to use for a given package,
/// applying package source mapping (when --source is not provided) and
/// validating HTTP usage only on the *effective* sources.
/// </summary>
private static bool TryGetRepositoriesForPackage(
Copy link
Contributor

@aortiz-msft aortiz-msft Nov 3, 2025

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.

string packageId,
PackageDownloadArgs args,
IReadOnlyList<PackageSource> allPackageSources,
PackageSourceMapping? packageSourceMapping,
IReadOnlyDictionary<string, SourceRepository> sourceRepositoriesMap,
ILoggerWithColor logger,
out List<SourceRepository> repositories)
{
List<PackageSource> effectiveSources;

var sourceExplicitlyProvided = args.Sources?.Count > 0;
if (sourceExplicitlyProvided || (packageSourceMapping != null && !packageSourceMapping.IsEnabled))
{
// --source given OR mapping disabled: use all provided sources as-is
effectiveSources = [.. allPackageSources];
}
else
{
// Mapping enabled, no --source: try mapped names first
var mappedNames = packageSourceMapping == null ? [] : packageSourceMapping.GetConfiguredPackageSources(packageId);

// Build effective sources in the same order as mappedNames
var mapped = mappedNames
.Select(n => allPackageSources.FirstOrDefault(ps =>
string.Equals(ps.Name, n, StringComparison.OrdinalIgnoreCase)))
.ToList();

// Only validate insecure sources when mapping produced something
if (mapped.Count > 0)
{
if (DetectAndReportInsecureSources(args.AllowInsecureConnections, mapped!, logger))
{
repositories = [];
return false;
}

effectiveSources = mapped!;
}
else
{
// No mapping for this package: fall back to all sources
effectiveSources = [.. allPackageSources];
}
}

// Convert effective sources to repositories
repositories = new List<SourceRepository>(effectiveSources.Count);
foreach (var src in effectiveSources)
{
repositories.Add(sourceRepositoriesMap[src.Name]);
}

return true;
}

private static async Task<bool> DownloadPackageAsync(
string id,
NuGetVersion version,
Expand Down Expand Up @@ -239,7 +319,7 @@ private static async Task<bool> DownloadPackageAsync(
return success;
}

private static IReadOnlyList<PackageSource> GetPackageSources(IList<string> sources, IPackageSourceProvider sourceProvider)
private static IReadOnlyList<PackageSource> GetPackageSources(IList<string>? sources, IPackageSourceProvider sourceProvider)
{
IEnumerable<PackageSource> configuredSources = sourceProvider.LoadPackageSources()
.Where(s => s.IsEnabled);
Expand Down Expand Up @@ -271,13 +351,13 @@ private static bool DetectAndReportInsecureSources(
return false;
}

private static IReadOnlyList<SourceRepository> GetSourceRepositories(IReadOnlyList<PackageSource> packageSources)
private static IReadOnlyDictionary<string, SourceRepository> GetSourceRepositories(IReadOnlyList<PackageSource> packageSources)
{
IEnumerable<Lazy<INuGetResourceProvider>> providers = Repository.Provider.GetCoreV3();
List<SourceRepository> sourceRepositories = [];
Dictionary<string, SourceRepository> sourceRepositories = [];
foreach (var source in packageSources)
{
sourceRepositories.Add(Repository.CreateSource(providers, source, FeedType.Undefined));
sourceRepositories[source.Name] = Repository.CreateSource(providers, source, FeedType.Undefined);
}

return sourceRepositories;
Expand Down
175 changes: 175 additions & 0 deletions .../NuGet.Core.FuncTests/NuGet.XPlat.FuncTest/Package/Download/PackageDownloadRunnerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
using Test.Utility;
using Moq;
using NuGet.CommandLine.XPlat;
using NuGet.CommandLine.XPlat.Commands.Package;
Expand Down Expand Up @@ -426,4 +427,178 @@ [new PackageSource(context.PackageSource)],
File.Exists(Path.Combine(context.WorkingDirectory, $"{id.ToLowerInvariant()}.{v}.nupkg"))
.Should().BeFalse("Package does not exist in sources");
}

public static IEnumerable<object[]> Cases()
{
// Parameters:
// A-packages, B-packages, sourceMappings, sourcesArgs, downloadId, downloadVersion,
// allowInsecureConnections, expectSuccess, expectedInstalled

// --source specified, mapping ignored, package only in A -> success
yield return new object[]
{
new List<(string,string)> { ("Contoso.Lib", "1.0.0") }, // A
new List<(string,string)>(), // B
new List<(string,string)> { ("B", "Contoso.*") }, // mapping ignored
new List<string> { "A" }, // --source A
"Contoso.Lib", "1.0.0", // downloadId, downloadVersion
true, // allow insecure
true, // expect success
("Contoso.Lib", "1.0.0") // expectedInstalled
};

// no --source, mapping -> B, package only in B -> success
yield return new object[]
{
new List<(string,string)>(), // A
new List<(string,string)> { ("Contoso.Mapped", "2.0.0") }, // B
new List<(string,string)> { ("B", "Contoso.*") }, // mapping -> B
null, // no --source
"Contoso.Mapped", "2.0.0", // downloadId, downloadVersion
true, // allow insecure
true, // expect success
("Contoso.Mapped", "2.0.0") // expectedInstalled
};

// no --source, mapping -> A, package only in B -> fail
yield return new object[]
{
new List<(string,string)>(), // A
new List<(string,string)> { ("Contoso.Mapped", "2.0.0") },
new List<(string,string)> { ("A", "Contoso.*") }, // mapped to A
null,
"Contoso.Mapped", "2.0.0",
true,
false,
null!
};

// --source specified, no source mapping with an insecure source
yield return new object[]
{
new List<(string,string)> { ("Contoso.Lib", "1.0.0") }, // A
new List<(string,string)>(),
new List<(string,string)> { ("A", "Contoso.*") },
new List<string> { "A" }, // --source
"Contoso.Lib", "1.0.0",
false, // allow insecure connections false / not set to true
false,
null!
};

// no --source, mapping -> B, allow insecure not enabled -> fail
Copy link
Contributor

@aortiz-msft aortiz-msft Nov 3, 2025

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.

yield return new object[]
{
new List<(string,string)>(), // A
new List<(string,string)> { ("Contoso.Mapped", "1.0.0") },
new List<(string,string)> { ("B", "Contoso.*") },
null,
"Contoso.Mapped", "1.0.0",
false, // allow insecure connections false / not set to true
false,
null!
};
}

[Theory]
[MemberData(nameof(Cases))]
public async Task RunAsync_WithSourceMapping_ListDriven_UsingCleanSetup(
IReadOnlyList<(string id, string version)> sourceAPackages,
IReadOnlyList<(string id, string version)> sourceBPackages,
IReadOnlyList<(string source, string pattern)> sourceMappings,
IReadOnlyList<string> sourcesArgs,
string downloadId,
string downloadVersion,
bool allowInsecureConnections,
bool expectSuccess,
(string id, string version)? expectedInstalled)
{
// Arrange
using var context = new SimpleTestPathContext();
string srcADirectory = Path.Combine(context.PackageSource, "SourceA");
string srcBDirectory = Path.Combine(context.PackageSource, "SourceB");

using var serverA = new FileSystemBackedV3MockServer(srcADirectory);
using var serverB = new FileSystemBackedV3MockServer(srcBDirectory);

foreach (var (id, ver) in sourceAPackages)
{
await SimpleTestPackageUtility.CreateFullPackageAsync(srcADirectory, id, ver);
}

foreach (var (id, ver) in sourceBPackages)
{
await SimpleTestPackageUtility.CreateFullPackageAsync(srcBDirectory, id, ver);
}

serverA.Start();
serverB.Start();

// sources
context.Settings.AddSource("A", serverA.ServiceIndexUri);
context.Settings.AddSource("B", serverB.ServiceIndexUri);

// mapping
foreach (var (src, pattern) in sourceMappings)
{
context.Settings.AddPackageSourceMapping(src, pattern);
}

var settings = Settings.LoadSettingsGivenConfigPaths([context.Settings.ConfigPath]);

var packageSources = new List<PackageSource>
{
new(serverA.ServiceIndexUri, "A"),
new(serverB.ServiceIndexUri, "B")
};

// args
var args = new PackageDownloadArgs
{
Packages =
[
new PackageWithNuGetVersion
{
Id = downloadId,
NuGetVersion = downloadVersion is null ? null : NuGetVersion.Parse(downloadVersion)
}
],
OutputDirectory = context.WorkingDirectory,
AllowInsecureConnections = allowInsecureConnections,
Sources = sourcesArgs == null ? [] : sourcesArgs.ToList()
};

string capturedLogs = string.Empty;
var logger = new Mock<ILoggerWithColor>(MockBehavior.Loose);
logger
.Setup(l => l.LogError(It.IsAny<string>()))
.Callback<string>(msg => capturedLogs += msg + Environment.NewLine);

// Act
var exit = await PackageDownloadRunner.RunAsync(
args,
logger.Object,
packageSources,
settings,
CancellationToken.None);

serverA.Stop();
serverB.Stop();

// Assert
if (expectSuccess)
{
exit.Should().Be(PackageDownloadRunner.ExitCodeSuccess, because: capturedLogs);
expectedInstalled.Should().NotBeNull();

var (expId, expVer) = expectedInstalled!.Value;
var installDir = Path.Combine(context.WorkingDirectory, expId.ToLowerInvariant(), expVer);
Directory.Exists(installDir).Should().BeTrue();
File.Exists(Path.Combine(installDir, $"{expId.ToLowerInvariant()}.{expVer}.nupkg")).Should().BeTrue();
}
else
{
exit.Should().Be(PackageDownloadRunner.ExitCodeError);
}
}
}
1 change: 0 additions & 1 deletion ...sts/NuGet.CommandLine.Xplat.Tests/Commands/Package/Download/PackageDownloadRunnerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,4 @@ public async Task ResolvePackageDownloadVersion_UnlistedPackage_BehavesAsExpecte
foundRepo.Should().BeNull();
}
}

}
16 changes: 14 additions & 2 deletions test/TestUtilities/Test.Utility/FileSystemBackedV3MockServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,21 @@ private Action<HttpListenerResponse> ServerHandlerV3(HttpListenerRequest request
}
else if (path.StartsWith("/flat/") && path.EndsWith(".nupkg"))
{
var file = new FileInfo(Path.Combine(_packageDirectory, parts.Last()));
var requestedFileName = parts.Last();
var directory = new DirectoryInfo(_packageDirectory);
FileInfo file = null;

if (file.Exists)
// Scan for file and ignore case to make sure this works in linux too
foreach (var candidate in directory.EnumerateFiles("*.nupkg", SearchOption.TopDirectoryOnly))
{
if (string.Equals(candidate.Name, requestedFileName, StringComparison.OrdinalIgnoreCase))
{
file = candidate;
break;
}
}

if (file != null && file.Exists)
{
return new Action<HttpListenerResponse>(response =>
{
Expand Down
Loading