From 2d388e30d1166a95603242954281b626585b9de0 Mon Sep 17 00:00:00 2001 From: Nikolche Kolev Date: Mon, 12 Feb 2024 14:05:44 -0800 Subject: [PATCH] Consolidate packages.config restore implementations by minimizing discrepancies (#5628) --- .../Commands/DownloadCommandBase.cs | 6 +- .../Commands/InstallCommand.cs | 4 +- .../Commands/RestoreCommand.cs | 84 +++++++++++++------ .../NuGet.CommandLine/GlobalSuppressions.cs | 1 - .../NuGet.Build.Tasks/BuildTasksUtility.cs | 39 +++++---- 5 files changed, 84 insertions(+), 50 deletions(-) diff --git a/src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs b/src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs index b0fa23e1ed0..dff04231b8a 100644 --- a/src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs +++ b/src/NuGet.Clients/NuGet.CommandLine/Commands/DownloadCommandBase.cs @@ -95,9 +95,7 @@ internal void CalculateEffectivePackageSaveMode() } } - protected IEnumerable GetInstalledPackageReferences( - string projectConfigFilePath, - bool allowDuplicatePackageIds) + internal IEnumerable GetInstalledPackageReferences(string projectConfigFilePath) { if (File.Exists(projectConfigFilePath)) { @@ -105,7 +103,7 @@ internal void CalculateEffectivePackageSaveMode() { XDocument xDocument = XmlUtility.Load(projectConfigFilePath); var reader = new PackagesConfigReader(xDocument); - return reader.GetPackages(allowDuplicatePackageIds); + return reader.GetPackages(allowDuplicatePackageIds: true); } catch (XmlException ex) { diff --git a/src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs b/src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs index ba7822b137b..96a706779e7 100644 --- a/src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs +++ b/src/NuGet.Clients/NuGet.CommandLine/Commands/InstallCommand.cs @@ -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( diff --git a/src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs b/src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs index 51c9cd1c6ed..462cbdc6e11 100644 --- a/src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs +++ b/src/NuGet.Clients/NuGet.CommandLine/Commands/RestoreCommand.cs @@ -264,12 +264,46 @@ private async Task> PerformNuGetV2RestoreAsync(Pac var sourceRepositoryProvider = new CommandLineSourceRepositoryProvider(SourceProvider); var nuGetPackageManager = new NuGetPackageManager(sourceRepositoryProvider, Settings, packagesFolderPath); - var installedPackageReferences = new HashSet(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 = new(); + bool areAnyPackagesMissing = false; + if (packageRestoreInputs.RestoringWithSolutionFile) { - installedPackageReferences.AddRange(packageRestoreInputs - .PackagesConfigFiles - .SelectMany(file => GetInstalledPackageReferences(file, allowDuplicatePackageIds: true))); + Dictionary configToProjectPath = GetPackagesConfigToProjectPath(packageRestoreInputs); + Dictionary> 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 value)) + { + value ??= new(); + packageReferenceToProjects.Add(packageReference, value); + } + value.Add(projectPath); + } + } + + foreach (KeyValuePair> 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) { @@ -291,21 +325,15 @@ private async Task> 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, @@ -330,14 +358,6 @@ private async Task> 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 @@ -424,6 +444,20 @@ private async Task> PerformNuGetV2RestoreAsync(Pac } } + private static Dictionary GetPackagesConfigToProjectPath(PackageRestoreInputs packageRestoreInputs) + { + Dictionary 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; + } + /// /// Processes List of PackageRestoreFailedEventArgs into a List of RestoreLogMessages. /// diff --git a/src/NuGet.Clients/NuGet.CommandLine/GlobalSuppressions.cs b/src/NuGet.Clients/NuGet.CommandLine/GlobalSuppressions.cs index e278ca9da14..897c1d7fac6 100644 --- a/src/NuGet.Clients/NuGet.CommandLine/GlobalSuppressions.cs +++ b/src/NuGet.Clients/NuGet.CommandLine/GlobalSuppressions.cs @@ -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 = "", 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 = "", 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 = "", 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 = "", 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 = "", 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 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 = "", 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 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 = "", Scope = "member", Target = "~M:NuGet.CommandLine.MsBuildUtility.GetAllProjectFileNames(System.String,System.String)~System.Collections.Generic.IEnumerable{System.String}")] diff --git a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs index 02e2d7175d6..45d623a6db5 100644 --- a/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs +++ b/src/NuGet.Core/NuGet.Build.Tasks/BuildTasksUtility.cs @@ -418,13 +418,11 @@ private static async Task PerformNuGetV2RestoreAsync(Common.ILog { string globalPackageFolder = null; string repositoryPath = null; - string firstPackagesConfigPath = null; IList packageSources = null; - - var installedPackageReferences = new HashSet(PackageReferenceComparer.Instance); - ISettings settings = null; + Dictionary> packageReferenceToProjects = new(PackageReferenceComparer.Instance); + foreach (PackageSpec packageSpec in dgFile.Projects.Where(i => i.RestoreMetadata.ProjectStyle == ProjectStyle.PackagesConfig)) { var pcRestoreMetadata = (PackagesConfigProjectRestoreMetadata)packageSpec.RestoreMetadata; @@ -449,9 +447,15 @@ private static async Task 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 value)) + { + value ??= new(); + packageReferenceToProjects.Add(packageReference, value); + } + value.Add(packagesConfigPath); + } } if (string.IsNullOrEmpty(repositoryPath)) @@ -469,18 +473,19 @@ private static async Task PerformNuGetV2RestoreAsync(Common.ILog Packaging.PackageSaveMode.Defaultv2 : effectivePackageSaveMode; - var missingPackageReferences = installedPackageReferences.Where(reference => - !nuGetPackageManager.PackageExistsInPackagesFolder(reference.PackageIdentity, packageSaveMode)).ToArray(); + List packageRestoreData = new(packageReferenceToProjects.Count); + bool areAnyPackagesMissing = false; + foreach (KeyValuePair> 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(); @@ -591,7 +596,7 @@ internal static PackageSaveMode CalculateEffectivePackageSaveMode(ISettings sett } - private static IEnumerable GetInstalledPackageReferences(string projectConfigFilePath, bool allowDuplicatePackageIds) + private static IEnumerable GetInstalledPackageReferences(string projectConfigFilePath) { if (File.Exists(projectConfigFilePath)) { @@ -599,7 +604,7 @@ private static IEnumerable GetInstalledPackageReferences(strin { XDocument xDocument = Load(projectConfigFilePath); var reader = new PackagesConfigReader(xDocument); - return reader.GetPackages(allowDuplicatePackageIds); + return reader.GetPackages(allowDuplicatePackageIds: true); } catch (XmlException ex) {