Skip to content

Commit

Permalink
(chocolatey#3451) Use availablePackages in GetPackageDependencies
Browse files Browse the repository at this point in the history 
This updates the dependency handling of packages to use
availablePackages when a package from the same source satisfies the
dependency.  This improves packages that have dependencies with many
versions available, and allows less data being requested from servers
in the case we have already have available packages from the source.

Without this fix, the installation will be delayed with repeated
resolution of the same package dependencies.

Use parentPackage version when finding the parent package.
  • Loading branch information
@josh-cooley @corbob
josh-cooley authored and corbob committed Aug 6, 2024
1 parent 0487213 commit ba93945
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 10 deletions.
87 changes: 87 additions & 0 deletions src/chocolatey.tests/infrastructure.app/nuget/NugetCommonSpecs.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using System.Threading;
using Chocolatey.NuGet.Frameworks;
using chocolatey.infrastructure.app;
using chocolatey.infrastructure.app.configuration;
using chocolatey.infrastructure.app.nuget;
using chocolatey.infrastructure.filesystem;
using Moq;
using NuGet.Common;
using NuGet.Configuration;
using NuGet.Packaging;
using NuGet.Packaging.Core;
using NuGet.Protocol;
using NuGet.Protocol.Core.Types;
using NuGet.Versioning;
using FluentAssertions;

namespace chocolatey.tests.infrastructure.app.nuget
Expand Down Expand Up @@ -179,5 +185,86 @@ public void Should_set_user_agent_string()
UserAgent.UserAgentString.Should().StartWith(expectedUserAgentString);
}
}

private class When_gets_package_dependencies : TinySpec
{
private Func<Task> _because;
private readonly Mock<SourceCacheContext> _sourceCacheContext = new Mock<SourceCacheContext>();
private readonly Mock<ILogger> _nugetLogger = new Mock<ILogger>();
private readonly List<NuGetEndpointResources> _nuGetEndpointResources = new List<NuGetEndpointResources>();
private readonly HashSet<SourcePackageDependencyInfo> _sourcePackageDependencyInfos = new HashSet<SourcePackageDependencyInfo>();
private readonly HashSet<PackageDependency> _packageDependencies = new HashSet<PackageDependency>();
private readonly Mock<SourceRepository> _sourceRepository = new Mock<SourceRepository>();
private readonly Mock<DependencyInfoResource> _dependencyInfoResource = new Mock<DependencyInfoResource>();
private PackageSource _packageSource;
private ChocolateyConfiguration _configuration;

public override void Context()
{
_configuration = new ChocolateyConfiguration();
_sourceCacheContext.ResetCalls();
_nugetLogger.ResetCalls();
_sourceRepository.ResetCalls();
_nuGetEndpointResources.Clear();
_sourcePackageDependencyInfos.Clear();
_packageDependencies.Clear();
_sourceRepository.Setup(r => r.GetResource<DependencyInfoResource>(It.IsAny<SourceCacheContext>())).Returns(_dependencyInfoResource.Object);
_packageSource = new PackageSource("C:\\packages");
_sourceRepository.SetupGet(r => r.PackageSource).Returns(_packageSource);

var chocolateySourceCacheContext = new ChocolateySourceCacheContext(_configuration);
_nuGetEndpointResources.Add(NuGetEndpointResources.GetResourcesBySource(_sourceRepository.Object, chocolateySourceCacheContext));
}

public override void Because()
{
_because = () => NugetCommon.GetPackageDependencies(new PackageIdentity("a", new NuGetVersion(1, 0, 1000)), NuGetFramework.AnyFramework,
_sourceCacheContext.Object, _nugetLogger.Object, _nuGetEndpointResources, _sourcePackageDependencyInfos, _packageDependencies, _configuration);
}

[Fact]
public async Task Should_request_dependencies_once()
{
Context();

var adeps = new[] { new PackageDependency("b", new VersionRange(new NuGetVersion(1, 0, 1000), true, new NuGetVersion(2, 0, 0), false)), new PackageDependency("c", new VersionRange(new NuGetVersion(1, 0, 1), true, new NuGetVersion(2, 0, 0), false)) };
var adepInfo = new SourcePackageDependencyInfo("a", new NuGetVersion(1, 0, 1000), adeps, true, _sourceRepository.Object);
_dependencyInfoResource.Setup(r => r.ResolvePackage(It.Is<PackageIdentity>(pid => pid.Id == "a" && pid.Version == new NuGetVersion(1, 0, 1000)), It.IsAny<NuGetFramework>(), It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(adepInfo);
var bdeps = new[] { new PackageDependency("d", new VersionRange(new NuGetVersion(1, 0, 1000), true, new NuGetVersion(2, 0, 0), false)) };
var bdepInfo = new[] { new SourcePackageDependencyInfo("b", new NuGetVersion(1, 0, 1000), bdeps, true, _sourceRepository.Object) };
_dependencyInfoResource.Setup(r => r.ResolvePackages("b", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(bdepInfo);
var cdeps = new[] { new PackageDependency("d", new VersionRange(new NuGetVersion(1, 0, 1), true, new NuGetVersion(2, 0, 0), false)) };
var cdepInfo = new[] { new SourcePackageDependencyInfo("c", new NuGetVersion(1, 0, 1), cdeps, true, _sourceRepository.Object) };
_dependencyInfoResource.Setup(r => r.ResolvePackages("c", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(cdepInfo);
var ddepInfo = GetDependencies("d", "e", "1.0.0", 1001, "2.0.0");
_dependencyInfoResource.Setup(r => r.ResolvePackages("d", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(ddepInfo);
var edepInfo = GetDependencies("e", null, "1.0.0", 1001, null);
_dependencyInfoResource.Setup(r => r.ResolvePackages("e", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(edepInfo);

await _because();

_dependencyInfoResource.Verify(r => r.ResolvePackage(It.Is<PackageIdentity>(pid => pid.Id == "a" && pid.Version == new NuGetVersion(1, 0, 1000)), It.IsAny<NuGetFramework>(), It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
_dependencyInfoResource.Verify(r => r.ResolvePackages("b", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
_dependencyInfoResource.Verify(r => r.ResolvePackages("c", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
_dependencyInfoResource.Verify(r => r.ResolvePackages("d", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
_dependencyInfoResource.Verify(r => r.ResolvePackages("e", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
}

private IEnumerable<SourcePackageDependencyInfo> GetDependencies(string packageId, string dependencyId, string lowerRangeStart, int count, string upperRange)
{
var dependencyInfo = new List<SourcePackageDependencyInfo>();
var startVersion = NuGetVersion.Parse(lowerRangeStart);
var upperVersion = !string.IsNullOrEmpty(upperRange) ? NuGetVersion.Parse(upperRange) : null;
for (var i = startVersion.Patch; i < startVersion.Patch + count; i++)
{
var packageDependency = !string.IsNullOrEmpty(dependencyId)
? new[] { new PackageDependency(dependencyId, new VersionRange(new NuGetVersion(startVersion.Major, startVersion.Minor, i), true, upperVersion, false)) }
: Array.Empty<PackageDependency>();
dependencyInfo.Add(new SourcePackageDependencyInfo(packageId, new NuGetVersion(startVersion.Major, startVersion.Minor, i), packageDependency, true, _sourceRepository.Object));
}

return dependencyInfo;
}
}
}
}
23 changes: 17 additions & 6 deletions src/chocolatey/infrastructure.app/nuget/NugetCommon.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -451,16 +451,27 @@ public static async Task GetPackageDependencies(string packageId,

//if (availablePackages.Contains(packageID)) return;

var dependencyInfoResources = resources.DependencyInfoResources();

foreach (var dependencyInfoResource in dependencyInfoResources)
foreach (var resource in resources)
{
IEnumerable<SourcePackageDependencyInfo> dependencyInfos = Array.Empty<SourcePackageDependencyInfo>();
var dependencyInfoResource = resource.DependencyInfoResource;

if (dependencyInfoResource is null)
{
// We can't lookup any dependencies using this resource.
continue;
}

// check if we already have packages matching the constraints in the list
IEnumerable<SourcePackageDependencyInfo> dependencyInfos = availablePackages.Where(
p => string.Equals(p.Id, packageId, StringComparison.OrdinalIgnoreCase) && p.HasVersion && versionRange.Satisfies(p.Version) && p.Source == resource.Source).ToList();

try
{
dependencyInfos = await dependencyInfoResource.ResolvePackages(
packageId, configuration.Prerelease, framework, cacheContext, logger, CancellationToken.None);
if (!dependencyInfos.Any())
{
dependencyInfos = await dependencyInfoResource.ResolvePackages(
packageId, configuration.Prerelease, framework, cacheContext, logger, CancellationToken.None);
}
}
catch (AggregateException ex) when (!(ex.InnerException is null))
{
Expand Down
8 changes: 4 additions & 4 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1327,17 +1327,17 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
null));

// For an initial attempt at finding a package resolution solution, check for all parent packages (i.e. locally installed packages
// that take a dependency on the package that is currently being upgraded) and find the depdendencies associated with these packages.
// NOTE: All the latest availble package version, as well as the specifically requested package version (if applicable) will be
// that take a dependency on the package that is currently being upgraded) and find the dependencies associated with these packages.
// NOTE: All the latest available package version, as well as the specifically requested package version (if applicable) will be
// searched for. If this don't provide enough information to obtain a solution, then a follow up query in the catch block for this
// section of the code will be completed.
var parentInfos = new HashSet<SourcePackageDependencyInfo>(PackageIdentityComparer.Default);
NugetCommon.GetPackageParents(availablePackage.Identity.Id, parentInfos, localPackagesDependencyInfos).GetAwaiter().GetResult();
foreach (var parentPackage in parentInfos)
{
if (version != null)
if (parentPackage.HasVersion)
{
var requestedPackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, (SourceCacheContext)sourceCacheContext, remoteEndpoints, version);
var requestedPackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, (SourceCacheContext)sourceCacheContext, remoteEndpoints, parentPackage.Version);

if (requestedPackageDependency != null)
{
Expand Down

0 comments on commit ba93945

Please sign in to comment.