Skip to content

Commit

Permalink
Bug fix for deduping dependencies in Find-PSResource (#1382)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alerickson
alerickson authored Aug 28, 2023
1 parent 27db036 commit e2b5aaa
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 27 deletions.
122 changes: 100 additions & 22 deletions src/code/FindHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal class FindHelper
private bool _includeDependencies = false;
private bool _repositoryNameContainsWildcard = true;
private NetworkCredential _networkCredential;
private Dictionary<string, List<string>> _packagesFound;

#endregion

Expand All @@ -46,6 +47,7 @@ public FindHelper(CancellationToken cancellationToken, PSCmdlet cmdletPassedIn,
_cancellationToken = cancellationToken;
_cmdletPassedIn = cmdletPassedIn;
_networkCredential = networkCredential;
_packagesFound = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
}

#endregion
Expand Down Expand Up @@ -404,7 +406,7 @@ public IEnumerable<PSCommandResourceInfo> FindByCommandOrDscResource(
{
_cmdletPassedIn.WriteVerbose(errRecord.Exception.Message);
}

continue;
}

Expand Down Expand Up @@ -632,7 +634,6 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R
{
ErrorRecord errRecord = null;
List<PSResourceInfo> parentPkgs = new List<PSResourceInfo>();
HashSet<string> pkgsFound = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
string tagsAsString = String.Empty;

_cmdletPassedIn.WriteDebug("In FindHelper::SearchByNames()");
Expand Down Expand Up @@ -677,8 +678,9 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R
if (foundPkg.Type == _type || _type == ResourceType.None)
{
parentPkgs.Add(foundPkg);
pkgsFound.Add(String.Format("{0}{1}", foundPkg.Name, foundPkg.Version.ToString()));
TryAddToPackagesFound(foundPkg);
_cmdletPassedIn.WriteDebug($"Found package '{foundPkg.Name}' version '{foundPkg.Version}'");

yield return foundPkg;
}
}
Expand Down Expand Up @@ -729,7 +731,8 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R

PSResourceInfo foundPkg = currentResult.returnedObject;
parentPkgs.Add(foundPkg);
pkgsFound.Add(String.Format("{0}{1}", foundPkg.Name, foundPkg.Version.ToString()));
TryAddToPackagesFound(foundPkg);

yield return foundPkg;
}
}
Expand Down Expand Up @@ -778,7 +781,8 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R

PSResourceInfo foundPkg = currentResult.returnedObject;
parentPkgs.Add(foundPkg);
pkgsFound.Add(String.Format("{0}{1}", foundPkg.Name, foundPkg.Version.ToString()));
TryAddToPackagesFound(foundPkg);

yield return foundPkg;
}
}
Expand Down Expand Up @@ -833,13 +837,14 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R
"FindVersionConvertToPSResourceFailure",
ErrorCategory.ObjectNotFound,
this));

continue;
}

PSResourceInfo foundPkg = currentResult.returnedObject;
parentPkgs.Add(foundPkg);
pkgsFound.Add(String.Format("{0}{1}", foundPkg.Name, foundPkg.Version.ToString()));
TryAddToPackagesFound(foundPkg);

yield return foundPkg;
}
}
Expand Down Expand Up @@ -915,7 +920,7 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R
&& _versionRange.Satisfies(version))
{
parentPkgs.Add(foundPkg);
pkgsFound.Add(String.Format("{0}{1}", foundPkg.Name, foundPkg.Version.ToString()));
TryAddToPackagesFound(foundPkg);

yield return foundPkg;
}
Expand All @@ -936,7 +941,7 @@ private IEnumerable<PSResourceInfo> SearchByNames(ServerApiCall currentServer, R
foreach (PSResourceInfo currentPkg in parentPkgs)
{
_cmdletPassedIn.WriteDebug($"Finding dependency packages for '{currentPkg.Name}'");
foreach (PSResourceInfo pkgDep in FindDependencyPackages(currentServer, currentResponseUtil, currentPkg, repository, pkgsFound))
foreach (PSResourceInfo pkgDep in FindDependencyPackages(currentServer, currentResponseUtil, currentPkg, repository))
{
yield return pkgDep;
}
Expand All @@ -962,6 +967,48 @@ private HashSet<string> GetPackageNamesPopulated(string[] pkgNames)
return pkgsToDiscover;
}


private bool TryAddToPackagesFound(PSResourceInfo foundPkg)
{
bool addedToHash = false;
string foundPkgName = foundPkg.Name;
string foundPkgVersion = Utils.GetNormalizedVersionString(foundPkg.Version.ToString(), foundPkg.Prerelease);

if (_packagesFound.ContainsKey(foundPkgName))
{
List<string> pkgVersions = _packagesFound[foundPkgName] as List<string>;

if (!pkgVersions.Contains(foundPkgVersion))
{
pkgVersions.Add(foundPkgVersion);
_packagesFound[foundPkgName] = pkgVersions;
addedToHash = true;
}
}
else
{
_packagesFound.Add(foundPkg.Name, new List<string> { foundPkgVersion });
addedToHash = true;
}

_cmdletPassedIn.WriteDebug($"Found package '{foundPkg.Name}' version '{foundPkg.Version}'");

return addedToHash;
}

private string FormatPkgVersionString(PSResourceInfo pkg)
{
string fullPkgVersion = pkg.Version.ToString();

if (!string.IsNullOrWhiteSpace(pkg.Prerelease))
{
fullPkgVersion += $"-{pkg.Prerelease}";
}
_cmdletPassedIn.WriteDebug($"Formatted full package version is: '{fullPkgVersion}'");

return fullPkgVersion;
}

#endregion

#region Internal Client Search Methods
Expand All @@ -970,8 +1017,7 @@ internal IEnumerable<PSResourceInfo> FindDependencyPackages(
ServerApiCall currentServer,
ResponseUtil currentResponseUtil,
PSResourceInfo currentPkg,
PSRepositoryInfo repository,
HashSet<string> foundPkgs)
PSRepositoryInfo repository)
{
if (currentPkg.Dependencies.Length > 0)
{
Expand Down Expand Up @@ -1009,15 +1055,26 @@ internal IEnumerable<PSResourceInfo> FindDependencyPackages(
}

depPkg = currentResult.returnedObject;
string pkgHashKey = String.Format("{0}{1}", depPkg.Name, depPkg.Version.ToString());

if (!foundPkgs.Contains(pkgHashKey))
if (!_packagesFound.ContainsKey(depPkg.Name))
{
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository, foundPkgs))
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository))
{
yield return depRes;
}
}
else
{
List<string> pkgVersions = _packagesFound[depPkg.Name] as List<string>;
// _packagesFound has depPkg.name in it, but the version is not the same
if (!pkgVersions.Contains(FormatPkgVersionString(depPkg)))
{
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository))
{
yield return depRes;
}
}
}
}
else
{
Expand Down Expand Up @@ -1066,7 +1123,7 @@ internal IEnumerable<PSResourceInfo> FindDependencyPackages(
if (foundDep.IsPrerelease) {
depVersionStr += $"-{foundDep.Prerelease}";
}

if (NuGetVersion.TryParse(depVersionStr, out NuGetVersion depVersion)
&& dep.VersionRange.Satisfies(depVersion))
{
Expand All @@ -1078,26 +1135,47 @@ internal IEnumerable<PSResourceInfo> FindDependencyPackages(
{
continue;
}

string pkgHashKey = String.Format("{0}{1}", depPkg.Name, depPkg.Version.ToString());

if (!foundPkgs.Contains(pkgHashKey))
if (!_packagesFound.ContainsKey(depPkg.Name))
{
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository, foundPkgs))
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository))
{
yield return depRes;
}
}
else {
List<string> pkgVersions = _packagesFound[depPkg.Name] as List<string>;
// _packagesFound has depPkg.name in it, but the version is not the same
if (!pkgVersions.Contains(FormatPkgVersionString(depPkg)))
{
foreach (PSResourceInfo depRes in FindDependencyPackages(currentServer, currentResponseUtil, depPkg, repository))
{
yield return depRes;
}
}
}
}
}
}

string currentPkgHashKey = String.Format("{0}{1}", currentPkg.Name, currentPkg.Version.ToString());

if (!foundPkgs.Contains(currentPkgHashKey))
if (!_packagesFound.ContainsKey(currentPkg.Name))
{
TryAddToPackagesFound(currentPkg);

yield return currentPkg;
}
else
{
List<string> pkgVersions = _packagesFound[currentPkg.Name] as List<string>;
// _packagesFound has currentPkg.name in it, but the version is not the same
if (!pkgVersions.Contains(FormatPkgVersionString(currentPkg)))
{
TryAddToPackagesFound(currentPkg);

yield return currentPkg;
}
}

}

#endregion
Expand Down
9 changes: 4 additions & 5 deletions src/code/InstallHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ private void MoveFilesIntoInstallPath(
}
}
else
{
{
var scriptXML = pkgInfo.Name + "_InstalledScriptInfo.xml";
if (!_savePkg)
{
Expand All @@ -509,8 +509,8 @@ private void MoveFilesIntoInstallPath(
File.Delete(Path.Combine(finalModuleVersionDir, pkgInfo.Name + PSScriptFileExt));
}
}
else {
_cmdletPassedIn.WriteVerbose(string.Format("Moving '{0}' to '{1}'", Path.Combine(dirNameVersion, scriptXML), Path.Combine(installPath, scriptXML)));
else {
_cmdletPassedIn.WriteVerbose(string.Format("Moving '{0}' to '{1}'", Path.Combine(dirNameVersion, scriptXML), Path.Combine(installPath, scriptXML)));
Utils.MoveFiles(Path.Combine(dirNameVersion, scriptXML), Path.Combine(installPath, scriptXML));
}

Expand Down Expand Up @@ -591,12 +591,11 @@ private List<PSResourceInfo> InstallPackages(
_cmdletPassedIn.WriteWarning("Installing dependencies is not currently supported for V3 server protocol repositories. The package will be installed without installing dependencies.");
}

HashSet<string> myHash = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
// Get the dependencies from the installed package.
if (parentPkgObj.Dependencies.Length > 0)
{
bool depFindFailed = false;
foreach (PSResourceInfo depPkg in findHelper.FindDependencyPackages(currentServer, currentResponseUtil, parentPkgObj, repository, myHash))
foreach (PSResourceInfo depPkg in findHelper.FindDependencyPackages(currentServer, currentResponseUtil, parentPkgObj, repository))
{
if (depPkg == null)
{
Expand Down
21 changes: 21 additions & 0 deletions test/FindPSResourceTests/FindPSResourceV2Server.Tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,4 +415,25 @@ Describe 'Test HTTP Find-PSResource for V2 Server Protocol' -tags 'ManualValidat

$res.Name | Should -Be $testModuleName
}

It "find should not duplicate dependencies found" {
$res = Find-PSResource -Name "Az" -IncludeDependencies -Repository $PSGalleryName
$res | Should -Not -BeNullOrEmpty

$foundPkgs = [System.Collections.Generic.HashSet[String]]::new()
$duplicatePkgsFound = $false
foreach ($item in $res)
{
if ($foundPkgs.Contains($item.Name))
{
write-host "this pkg already found"
$duplicatePkgsFound = $true
break
}

$foundPkgs.Add($item.Name)
}

$duplicatePkgsFound | Should -BeFalse
}
}

0 comments on commit e2b5aaa

Please sign in to comment.