Skip to content

Commit

Permalink
Consolidate packages.config restore implementations by minimizing dis…
Browse files Browse the repository at this point in the history 
…crepancies (#5628)
  • Loading branch information
@nkolev92
nkolev92 authored Feb 12, 2024
1 parent ef5bce3 commit 2d388e3
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 50 deletions.
6 changes: 2 additions & 4 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,15 @@ internal void CalculateEffectivePackageSaveMode()
}
}

protected IEnumerable<Packaging.PackageReference> GetInstalledPackageReferences(
string projectConfigFilePath,
bool allowDuplicatePackageIds)
internal IEnumerable<PackageReference> GetInstalledPackageReferences(string projectConfigFilePath)
{
if (File.Exists(projectConfigFilePath))
{
try
{
XDocument xDocument = XmlUtility.Load(projectConfigFilePath);
var reader = new PackagesConfigReader(xDocument);
return reader.GetPackages(allowDuplicatePackageIds);
return reader.GetPackages(allowDuplicatePackageIds: true);
}
catch (XmlException ex)
{
Expand Down
4 changes: 1 addition & 3 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,7 @@ private async Task PerformV2RestoreAsync(string packagesConfigFilePath, string i
var sourceRepositoryProvider = GetSourceRepositoryProvider();
var nuGetPackageManager = new NuGetPackageManager(sourceRepositoryProvider, Settings, installPath, ExcludeVersion);

var installedPackageReferences = GetInstalledPackageReferences(
packagesConfigFilePath,
allowDuplicatePackageIds: true);
var installedPackageReferences = GetInstalledPackageReferences(packagesConfigFilePath);

var packageRestoreData = installedPackageReferences.Select(reference =>
new PackageRestoreData(
Expand Down
84 changes: 59 additions & 25 deletions src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,46 @@ private async Task<IReadOnlyList<RestoreSummary>> PerformNuGetV2RestoreAsync(Pac
var sourceRepositoryProvider = new CommandLineSourceRepositoryProvider(SourceProvider);
var nuGetPackageManager = new NuGetPackageManager(sourceRepositoryProvider, Settings, packagesFolderPath);

var installedPackageReferences = new HashSet<Packaging.PackageReference>(PackageReferenceComparer.Instance);
// EffectivePackageSaveMode is None when -PackageSaveMode is not provided by the user. None is treated as
// Defaultv3 for V3 restore and should be treated as Defaultv2 for V2 restore. This is the case in the
// actual V2 restore flow and should match in this preliminary missing packages check.
var packageSaveMode = EffectivePackageSaveMode == Packaging.PackageSaveMode.None ?
Packaging.PackageSaveMode.Defaultv2 :
EffectivePackageSaveMode;

List<PackageRestoreData> packageRestoreData = new();
bool areAnyPackagesMissing = false;

if (packageRestoreInputs.RestoringWithSolutionFile)
{
installedPackageReferences.AddRange(packageRestoreInputs
.PackagesConfigFiles
.SelectMany(file => GetInstalledPackageReferences(file, allowDuplicatePackageIds: true)));
Dictionary<string, string> configToProjectPath = GetPackagesConfigToProjectPath(packageRestoreInputs);
Dictionary<PackageReference, List<string>> packageReferenceToProjects = new(PackageReferenceComparer.Instance);

foreach (string configFile in packageRestoreInputs.PackagesConfigFiles)
{
foreach (PackageReference packageReference in GetInstalledPackageReferences(configFile))
{
if (configToProjectPath.TryGetValue(configFile, out string projectPath))
{
projectPath = configFile;
}

if (!packageReferenceToProjects.TryGetValue(packageReference, out List<string> value))
{
value ??= new();
packageReferenceToProjects.Add(packageReference, value);
}
value.Add(projectPath);
}
}

foreach (KeyValuePair<PackageReference, List<string>> package in packageReferenceToProjects)
{
var exists = nuGetPackageManager.PackageExistsInPackagesFolder(package.Key.PackageIdentity, packageSaveMode);
packageRestoreData.Add(new PackageRestoreData(package.Key, package.Value, !exists));
areAnyPackagesMissing |= !exists;
}

}
else if (packageRestoreInputs.PackagesConfigFiles.Count > 0)
{
Expand All @@ -291,21 +325,15 @@ private async Task<IReadOnlyList<RestoreSummary>> PerformNuGetV2RestoreAsync(Pac
throw new InvalidOperationException(message);
}

installedPackageReferences.AddRange(
GetInstalledPackageReferences(packageReferenceFile, allowDuplicatePackageIds: true));
foreach (PackageReference packageReference in GetInstalledPackageReferences(packageReferenceFile))
{
bool exists = nuGetPackageManager.PackageExistsInPackagesFolder(packageReference.PackageIdentity, packageSaveMode);
packageRestoreData.Add(new PackageRestoreData(packageReference, [packageReferenceFile], !exists));
areAnyPackagesMissing |= !exists;
}
}

// EffectivePackageSaveMode is None when -PackageSaveMode is not provided by the user. None is treated as
// Defaultv3 for V3 restore and should be treated as Defaultv2 for V2 restore. This is the case in the
// actual V2 restore flow and should match in this preliminary missing packages check.
var packageSaveMode = EffectivePackageSaveMode == Packaging.PackageSaveMode.None ?
Packaging.PackageSaveMode.Defaultv2 :
EffectivePackageSaveMode;

var missingPackageReferences = installedPackageReferences.Where(reference =>
!nuGetPackageManager.PackageExistsInPackagesFolder(reference.PackageIdentity, packageSaveMode)).ToArray();

if (missingPackageReferences.Length == 0)
if (!areAnyPackagesMissing)
{
var message = string.Format(
CultureInfo.CurrentCulture,
Expand All @@ -330,14 +358,6 @@ private async Task<IReadOnlyList<RestoreSummary>> PerformNuGetV2RestoreAsync(Pac
return restoreSummaries;
}

var packageRestoreData = missingPackageReferences.Select(reference =>
new PackageRestoreData(
reference,
new[] { packageRestoreInputs.RestoringWithSolutionFile
? packageRestoreInputs.DirectoryOfSolutionFile
: packageRestoreInputs.PackagesConfigFiles[0] },
isMissing: true));

var packageSources = GetPackageSources(Settings);

var repositories = packageSources
Expand Down Expand Up @@ -424,6 +444,20 @@ private async Task<IReadOnlyList<RestoreSummary>> PerformNuGetV2RestoreAsync(Pac
}
}

private static Dictionary<string, string> GetPackagesConfigToProjectPath(PackageRestoreInputs packageRestoreInputs)
{
Dictionary<string, string> configToProjectPath = new();
foreach (PackageSpec project in packageRestoreInputs.ProjectReferenceLookup.Projects)
{
if (project.RestoreMetadata?.ProjectStyle == ProjectStyle.PackagesConfig)
{
configToProjectPath.Add(((PackagesConfigProjectRestoreMetadata)project.RestoreMetadata).PackagesConfigPath, project.FilePath);
}
}

return configToProjectPath;
}

/// <summary>
/// Processes List of PackageRestoreFailedEventArgs into a List of RestoreLogMessages.
/// </summary>
Expand Down
1 change: 0 additions & 1 deletion src/NuGet.Clients/NuGet.CommandLine/GlobalSuppressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void Console.PrintJustified(int startIndex, string text, int maxWidth)', validate parameter 'text' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.Console.PrintJustified(System.Int32,System.String,System.Int32)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void Console.ReadSecureString(SecureString secureString)', validate parameter 'secureString' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.Console.ReadSecureString(System.Security.SecureString)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'void Console.WriteError(object value)', validate parameter 'value' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.Console.WriteError(System.Object)")]
[assembly: SuppressMessage("Build", "CA1822:Member GetInstalledPackageReferences does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.DownloadCommandBase.GetInstalledPackageReferences(System.String,System.Boolean)~System.Collections.Generic.IEnumerable{NuGet.Packaging.PackageReference}")]
[assembly: SuppressMessage("Build", "CA1801:Parameter token of method GetFolderPackagesAsync is never used. Remove the parameter or use it in the method body.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.InstallCommandProject.GetFolderPackagesAsync(System.Threading.CancellationToken)~System.Threading.Tasks.Task{System.Collections.Generic.IEnumerable{NuGet.Packaging.PackageReference}}")]
[assembly: SuppressMessage("Build", "CA1303:Method 'void MsBuildUtility.AddProperty(List<string> args, string property, string value)' passes a literal string as parameter 'message' of a call to 'ArgumentException.ArgumentException(string message)'. Retrieve the following string(s) from a resource table instead: \"value\".", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.MsBuildUtility.AddProperty(System.Collections.Generic.List{System.String},System.String,System.String)")]
[assembly: SuppressMessage("Build", "CA1062:In externally visible method 'IEnumerable<string> MsBuildUtility.GetAllProjectFileNames(string solutionFile, string pathToMsbuildDir)', validate parameter 'pathToMsbuildDir' is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.", Justification = "<Pending>", Scope = "member", Target = "~M:NuGet.CommandLine.MsBuildUtility.GetAllProjectFileNames(System.String,System.String)~System.Collections.Generic.IEnumerable{System.String}")]
Expand Down
39 changes: 22 additions & 17 deletions src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,11 @@ private static async Task<RestoreSummary> PerformNuGetV2RestoreAsync(Common.ILog
{
string globalPackageFolder = null;
string repositoryPath = null;
string firstPackagesConfigPath = null;
IList<PackageSource> packageSources = null;

var installedPackageReferences = new HashSet<Packaging.PackageReference>(PackageReferenceComparer.Instance);

ISettings settings = null;

Dictionary<PackageReference, List<string>> packageReferenceToProjects = new(PackageReferenceComparer.Instance);

foreach (PackageSpec packageSpec in dgFile.Projects.Where(i => i.RestoreMetadata.ProjectStyle == ProjectStyle.PackagesConfig))
{
var pcRestoreMetadata = (PackagesConfigProjectRestoreMetadata)packageSpec.RestoreMetadata;
Expand All @@ -449,9 +447,15 @@ private static async Task<RestoreSummary> PerformNuGetV2RestoreAsync(Common.ILog

string packagesConfigPath = GetPackagesConfigFilePath(pcRestoreMetadata.ProjectPath);

firstPackagesConfigPath = firstPackagesConfigPath ?? packagesConfigPath;

installedPackageReferences.AddRange(GetInstalledPackageReferences(packagesConfigPath, allowDuplicatePackageIds: true));
foreach (PackageReference packageReference in GetInstalledPackageReferences(packagesConfigPath))
{
if (!packageReferenceToProjects.TryGetValue(packageReference, out List<string> value))
{
value ??= new();
packageReferenceToProjects.Add(packageReference, value);
}
value.Add(packagesConfigPath);
}
}

if (string.IsNullOrEmpty(repositoryPath))
Expand All @@ -469,18 +473,19 @@ private static async Task<RestoreSummary> PerformNuGetV2RestoreAsync(Common.ILog
Packaging.PackageSaveMode.Defaultv2 :
effectivePackageSaveMode;

var missingPackageReferences = installedPackageReferences.Where(reference =>
!nuGetPackageManager.PackageExistsInPackagesFolder(reference.PackageIdentity, packageSaveMode)).ToArray();
List<PackageRestoreData> packageRestoreData = new(packageReferenceToProjects.Count);
bool areAnyPackagesMissing = false;
foreach (KeyValuePair<PackageReference, List<string>> package in packageReferenceToProjects)
{
var exists = nuGetPackageManager.PackageExistsInPackagesFolder(package.Key.PackageIdentity, packageSaveMode);
packageRestoreData.Add(new PackageRestoreData(package.Key, package.Value, !exists));
areAnyPackagesMissing |= !exists;
}

if (missingPackageReferences.Length == 0)
if (!areAnyPackagesMissing)
{
return new RestoreSummary(true);
}
var packageRestoreData = missingPackageReferences.Select(reference =>
new PackageRestoreData(
reference,
new[] { firstPackagesConfigPath },
isMissing: true));

var repositories = sourceRepositoryProvider.GetRepositories().ToArray();

Expand Down Expand Up @@ -591,15 +596,15 @@ internal static PackageSaveMode CalculateEffectivePackageSaveMode(ISettings sett
}


private static IEnumerable<PackageReference> GetInstalledPackageReferences(string projectConfigFilePath, bool allowDuplicatePackageIds)
private static IEnumerable<PackageReference> GetInstalledPackageReferences(string projectConfigFilePath)
{
if (File.Exists(projectConfigFilePath))
{
try
{
XDocument xDocument = Load(projectConfigFilePath);
var reader = new PackagesConfigReader(xDocument);
return reader.GetPackages(allowDuplicatePackageIds);
return reader.GetPackages(allowDuplicatePackageIds: true);
}
catch (XmlException ex)
{
Expand Down

0 comments on commit 2d388e3

Please sign in to comment.