Skip to content

Commit

Permalink
(#3237) Reduce number of queries for dependencies
Browse files Browse the repository at this point in the history 
When attempting to figure out what packages need to be verified/checked
during a package upgrade operation, we were previously fetching
information about all package versions for all "parent packages" (i.e.
packages that include a dependency on the package being acted on). This
causes a problem when doing something like `choco upgrade chocolatey`
when you have the chromium package installed.  When in this situation,
due to the high number of package versions for chromium and the fact
that it takes a dependency on Chocolatey, the number of queries that
are emitted for Chocolatey CLI is very large.  This makes it appear as
though the operation has stalled, when in actual fact it is just
working its way through the queries (this can be verified by looking at
the requests through fiddler when this command is running).

This commit attempt to address this problem by changing to only query
for the latest package version (and/or the requested package version)
when attempting the look up for the parent package dependencies.  This
greatly reduces the number of outgoing queries, but it also means that
it is possible for things to not work correctly due to not all package
information being available.  To guard against this, a fallback has
been put in place such that if the initial attempt at resolving the
solution fails, it goes back and fetches information about all package
versions for the parent packages, and attempt to gather the resolve the
solution again.

This change has been made in a way that there is some code duplication,
however, based on some discussion within the team, the way that this
method is written needs to be overhauled, and now is not the right time
to do this.  Instead, we are accepting the duplication, until some
effort can be put in to overhaul this method completely.
  • Loading branch information
@gep13
gep13 committed Jul 11, 2023
1 parent 2f79fb6 commit f4f25f6
Showing 1 changed file with 83 additions and 19 deletions.
102 changes: 83 additions & 19 deletions src/chocolatey/infrastructure.app/services/NugetService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,14 +1207,27 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
null,
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
// 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)
{
foreach (var packageVersion in NugetList.FindAllPackageVersions(parentPackage.Id, config, _nugetLogger, sourceCacheContext, remoteEndpoints))
if (version != null)
{
NugetCommon.GetPackageDependencies(packageVersion.Identity, NuGetFramework.AnyFramework, sourceCacheContext, _nugetLogger, remoteEndpoints, sourcePackageDependencyInfos, sourceDependencyCache, config).GetAwaiter().GetResult();
var requestedPackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, sourceCacheContext, remoteEndpoints, version);

if (requestedPackageDependency != null)
{
NugetCommon.GetPackageDependencies(requestedPackageDependency.Identity, NuGetFramework.AnyFramework, sourceCacheContext, _nugetLogger, remoteEndpoints, sourcePackageDependencyInfos, sourceDependencyCache, config).GetAwaiter().GetResult();
}
}

var availablePackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, sourceCacheContext, remoteEndpoints);
NugetCommon.GetPackageDependencies(availablePackageDependency.Identity, NuGetFramework.AnyFramework, sourceCacheContext, _nugetLogger, remoteEndpoints, sourcePackageDependencyInfos, sourceDependencyCache, config).GetAwaiter().GetResult();
}

var removedSources = RemovePinnedSourceDependencies(sourcePackageDependencyInfos, allLocalPackages);
Expand Down Expand Up @@ -1251,17 +1264,6 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
//var allPackagesIdentities = allLocalPackages.Select(p => p.SearchMetadata.Identity).ToList();
var allPackagesReferences = allPackagesIdentities.Select(p => new PackageReference(p, NuGetFramework.AnyFramework));

var resolverContext = new PackageResolverContext(
dependencyBehavior: DependencyBehavior.Highest,
targetIds: targetIdsToInstall,
requiredPackageIds: allPackagesIdentities.Select(p => p.Id),
packagesConfig: allPackagesReferences,
preferredVersions: allPackagesIdentities,
availablePackages: sourcePackageDependencyInfos,
packageSources: remoteRepositories.Select(s => s.PackageSource),
log: _nugetLogger
);

IEnumerable<SourcePackageDependencyInfo> resolvedPackages = new List<SourcePackageDependencyInfo>();
if (config.IgnoreDependencies)
{
Expand Down Expand Up @@ -1291,6 +1293,17 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
}
else
{
var resolverContext = new PackageResolverContext(
dependencyBehavior: DependencyBehavior.Highest,
targetIds: targetIdsToInstall,
requiredPackageIds: allPackagesIdentities.Select(p => p.Id),
packagesConfig: allPackagesReferences,
preferredVersions: allPackagesIdentities,
availablePackages: sourcePackageDependencyInfos,
packageSources: remoteRepositories.Select(s => s.PackageSource),
log: _nugetLogger
);

try
{
resolvedPackages = dependencyResolver.Resolve(resolverContext, CancellationToken.None)
Expand All @@ -1306,15 +1319,66 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
packagesToUninstall.AddRange(allLocalPackages.Where(p => resolvedPackages.Select(x => x.Id).Contains(p.Name, StringComparer.OrdinalIgnoreCase)));
}
}
catch (NuGetResolverConstraintException ex)
catch (NuGetResolverConstraintException)
{
var logMessage = GetDependencyResolutionErrorMessage(ex);
this.Log().Error(ChocolateyLoggers.Important, logMessage);
this.Log().Warn("Re-attempting package dependency resolution using additional available package information...");

try
{
// If for some reason, it hasn't been possible to find a solution from the resolverContext, it could be that
// we haven't provided enough information about the available package versions in the sourcePackageDependencyInfos
// object. If we get here, assume that this is the case and re-attempt the upgrade, by pulling in ALL the
// dependency information, rather than only the latest package version, and specified package version.

// NOTE: There is duplication of work here, compared to what is done above, but further refactoing of this
// entire method would need to be done in order to make it more usable/maintable going forward. In the
// interim, the duplication is "acceptable" as it is hoped that the need to find ALL package dependencies
// will be the edge case, and not the rule.
foreach (var parentPackage in parentInfos)
{
foreach (var packageVersion in NugetList.FindAllPackageVersions(parentPackage.Id, config, _nugetLogger, sourceCacheContext, remoteEndpoints))
{
NugetCommon.GetPackageDependencies(packageVersion.Identity, NuGetFramework.AnyFramework, sourceCacheContext, _nugetLogger, remoteEndpoints, sourcePackageDependencyInfos, sourceDependencyCache, config).GetAwaiter().GetResult();
}
}

resolverContext = new PackageResolverContext(
dependencyBehavior: DependencyBehavior.Highest,
targetIds: targetIdsToInstall,
requiredPackageIds: allPackagesIdentities.Select(p => p.Id),
packagesConfig: allPackagesReferences,
preferredVersions: allPackagesIdentities,
availablePackages: sourcePackageDependencyInfos,
packageSources: remoteRepositories.Select(s => s.PackageSource),
log: _nugetLogger
);

resolvedPackages = dependencyResolver.Resolve(resolverContext, CancellationToken.None)
.Select(p => sourcePackageDependencyInfos.SingleOrDefault(x => PackageIdentityComparer.Default.Equals(x, p)));

if (!config.ForceDependencies)
{
var identitiesToUninstall = packagesToUninstall.Select(x => x.Identity);
resolvedPackages = resolvedPackages.Where(p => !(localPackagesDependencyInfos.Contains(p) && !identitiesToUninstall.Contains(p)));

foreach (var pkgMetadata in packagesToInstall)
// If forcing dependencies, then dependencies already added to packages to remove.
// If not forcing dependencies, then package needs to be removed so it can be upgraded to the new version required by the parent
packagesToUninstall.AddRange(allLocalPackages.Where(p => resolvedPackages.Select(x => x.Id).Contains(p.Name, StringComparer.OrdinalIgnoreCase)));
}
}
catch (NuGetResolverConstraintException nestedEx)
{
var errorResult = packageResultsToReturn.GetOrAdd(pkgMetadata.Identity.Id, new PackageResult(pkgMetadata, pathResolver.GetInstallPath(pkgMetadata.Identity)));
errorResult.Messages.Add(new ResultMessage(ResultType.Error, logMessage));
// If we get here, both the inital attempt to resolve a solution didn't work, as well as a second
// attempt using all available package versions didn't work, so this time around we hard fail, and
// provide information to the user about the conflicts for the package resolution.
var logMessage = GetDependencyResolutionErrorMessage(nestedEx);
this.Log().Error(ChocolateyLoggers.Important, logMessage);

foreach (var pkgMetadata in packagesToInstall)
{
var errorResult = packageResultsToReturn.GetOrAdd(pkgMetadata.Identity.Id, new PackageResult(pkgMetadata, pathResolver.GetInstallPath(pkgMetadata.Identity)));
errorResult.Messages.Add(new ResultMessage(ResultType.Error, logMessage));
}
}
}
catch (Exception ex)
Expand Down

0 comments on commit f4f25f6

Please sign in to comment.