From f7405059e0174b32c3fc3abc7fc19d15c31f42b3 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 8 Aug 2024 13:50:47 -0700 Subject: [PATCH] Reuse package information for unique packages --- .../{PackageReference.cs => PackageInfo.cs} | 30 ++----- .../PackageInfoFactory.cs | 25 ++++++ src/PackagesConfigConverter/PackageUsage.cs | 42 ++++++++++ .../ProjectConverter.cs | 81 ++++++++++--------- 4 files changed, 117 insertions(+), 61 deletions(-) rename src/PackagesConfigConverter/{PackageReference.cs => PackageInfo.cs} (62%) create mode 100644 src/PackagesConfigConverter/PackageInfoFactory.cs create mode 100644 src/PackagesConfigConverter/PackageUsage.cs diff --git a/src/PackagesConfigConverter/PackageReference.cs b/src/PackagesConfigConverter/PackageInfo.cs similarity index 62% rename from src/PackagesConfigConverter/PackageReference.cs rename to src/PackagesConfigConverter/PackageInfo.cs index 5dc4f2c..b95d38e 100644 --- a/src/PackagesConfigConverter/PackageReference.cs +++ b/src/PackagesConfigConverter/PackageInfo.cs @@ -2,11 +2,8 @@ // // Licensed under the MIT license. -using Microsoft.Build.Construction; -using NuGet.Frameworks; using NuGet.Packaging; using NuGet.Packaging.Core; -using NuGet.Versioning; using System; using System.Collections.Generic; using System.IO; @@ -14,41 +11,28 @@ namespace PackagesConfigConverter { - internal class PackageReference : NuGet.Packaging.PackageReference + internal sealed class PackageInfo { private IReadOnlyCollection _installedPackageFolderNames; - public PackageReference(PackageIdentity identity, NuGetFramework targetFramework, bool userInstalled, bool developmentDependency, bool requireReinstallation, VersionRange allowedVersions, PackagePathResolver packagePathResolver, VersionFolderPathResolver versionFolderPathResolver) - : base(identity, targetFramework, userInstalled, developmentDependency, requireReinstallation, allowedVersions) + public PackageInfo(PackageIdentity identity, PackagePathResolver packagePathResolver, VersionFolderPathResolver versionFolderPathResolver) { - InstalledPackageFilePath = PackagePathHelper.GetInstalledPackageFilePath(PackageIdentity, packagePathResolver ?? throw new ArgumentNullException(nameof(packagePathResolver))); + Identity = identity; + + InstalledPackageFilePath = PackagePathHelper.GetInstalledPackageFilePath(Identity, packagePathResolver ?? throw new ArgumentNullException(nameof(packagePathResolver))); RepositoryInstalledPath = Path.GetDirectoryName(InstalledPackageFilePath); GlobalInstalledPath = Path.GetFullPath(versionFolderPathResolver.GetInstallPath(identity.Id, identity.Version)); } - public IEnumerable AllElements => AnalyzerItems.Cast().Concat(AssemblyReferences).Concat(Imports); - - public List AnalyzerItems { get; } = new (); - - public List AssemblyReferences { get; } = new (); - - public bool GeneratePathProperty { get; set; } - public string GlobalInstalledPath { get; } - public List Imports { get; } = new (); - public string InstalledPackageFilePath { get; } public IReadOnlyCollection InstalledPackageFolderNames => _installedPackageFolderNames ??= GetPackageFolderNames(); - public bool IsMissingTransitiveDependency { get; set; } - - public string PackageId => PackageIdentity.Id; - - public NuGetVersion PackageVersion => PackageIdentity.Version; + public PackageIdentity Identity { get; } public string RepositoryInstalledPath { get; } @@ -91,4 +75,4 @@ static bool HasAnyRealFiles(string dir) } } } -} \ No newline at end of file +} diff --git a/src/PackagesConfigConverter/PackageInfoFactory.cs b/src/PackagesConfigConverter/PackageInfoFactory.cs new file mode 100644 index 0000000..d980d24 --- /dev/null +++ b/src/PackagesConfigConverter/PackageInfoFactory.cs @@ -0,0 +1,25 @@ +// Copyright (c) Jeff Kluge. All rights reserved. +// +// Licensed under the MIT license. + +using NuGet.Packaging; +using NuGet.Packaging.Core; +using System.Collections.Concurrent; + +namespace PackagesConfigConverter +{ + internal sealed class PackageInfoFactory + { + private readonly PackagePathResolver _packagePathResolver; + private readonly VersionFolderPathResolver _versionFolderPathResolver; + private readonly ConcurrentDictionary _packages = new (); + + public PackageInfoFactory(PackagePathResolver packagePathResolver, VersionFolderPathResolver versionFolderPathResolver) + { + _packagePathResolver = packagePathResolver; + _versionFolderPathResolver = versionFolderPathResolver; + } + + public PackageInfo GetPackageInfo(PackageIdentity identity) => _packages.GetOrAdd(identity, id => new PackageInfo(id, _packagePathResolver, _versionFolderPathResolver)); + } +} diff --git a/src/PackagesConfigConverter/PackageUsage.cs b/src/PackagesConfigConverter/PackageUsage.cs new file mode 100644 index 0000000..d6af0f2 --- /dev/null +++ b/src/PackagesConfigConverter/PackageUsage.cs @@ -0,0 +1,42 @@ +// Copyright (c) Jeff Kluge. All rights reserved. +// +// Licensed under the MIT license. + +using CommandLine; +using Microsoft.Build.Construction; +using NuGet.Packaging.Core; +using NuGet.Versioning; +using System.Collections.Generic; +using System.Linq; + +namespace PackagesConfigConverter +{ + internal sealed class PackageUsage + { + public PackageUsage(PackageIdentity identity, PackageInfo packageInfo, bool isDevelopmentDependency) + { + PackageInfo = packageInfo; + IsDevelopmentDependency = isDevelopmentDependency; + } + + public PackageInfo PackageInfo { get; } + + public bool IsDevelopmentDependency { get; } + + public IEnumerable AllElements => AnalyzerItems.Cast().Concat(AssemblyReferences).Concat(Imports); + + public List AnalyzerItems { get; } = new (); + + public List AssemblyReferences { get; } = new (); + + public List Imports { get; } = new (); + + public bool GeneratePathProperty { get; set; } + + public bool IsMissingTransitiveDependency { get; set; } + + public string PackageId => PackageInfo.Identity.Id; + + public NuGetVersion PackageVersion => PackageInfo.Identity.Version; + } +} diff --git a/src/PackagesConfigConverter/ProjectConverter.cs b/src/PackagesConfigConverter/ProjectConverter.cs index ffee454..506fd7d 100644 --- a/src/PackagesConfigConverter/ProjectConverter.cs +++ b/src/PackagesConfigConverter/ProjectConverter.cs @@ -40,6 +40,7 @@ internal sealed class ProjectConverter : IProjectConverter private readonly ISettings _nugetSettings; private readonly ProjectCollection _projectCollection = new (); private readonly string _repositoryPath; + private readonly PackageInfoFactory _packageInfoFactory; private readonly RestoreCommandProvidersCache _restoreCommandProvidersCache; private readonly RestoreArgs _restoreArgs; @@ -59,9 +60,9 @@ public ProjectConverter(ProjectConverterSettings converterSettings, ISettings nu _globalPackagesFolder = Path.GetFullPath(SettingsUtility.GetGlobalPackagesFolder(_nugetSettings)).Trim(Path.DirectorySeparatorChar); - PackagePathResolver = new PackagePathResolver(_repositoryPath); - - VersionFolderPathResolver = new VersionFolderPathResolver(_globalPackagesFolder); + var packagePathResolver = new PackagePathResolver(_repositoryPath); + var versionFolderPathResolver = new VersionFolderPathResolver(_globalPackagesFolder); + _packageInfoFactory = new PackageInfoFactory(packagePathResolver, versionFolderPathResolver); _restoreCommandProvidersCache = new RestoreCommandProvidersCache(); _restoreArgs = new RestoreArgs @@ -79,10 +80,6 @@ public ProjectConverter(ProjectConverterSettings converterSettings, ISettings nu public ILogger Log => _converterSettings.Log; - public PackagePathResolver PackagePathResolver { get; internal set; } - - public VersionFolderPathResolver VersionFolderPathResolver { get; internal set; } - public void ConvertRepository(CancellationToken cancellationToken) { bool success = true; @@ -146,25 +143,25 @@ private static ISettings GetNuGetSettings(ProjectConverterSettings converterSett return Settings.LoadDefaultSettings(converterSettings.RepositoryRoot, Settings.DefaultSettingsFileName, new XPlatMachineWideSetting()); } - private ProjectItemElement AddPackageReference(ProjectItemGroupElement itemGroupElement, PackageReference package) + private ProjectItemElement AddPackageReference(ProjectItemGroupElement itemGroupElement, PackageUsage package) { LibraryIncludeFlags includeAssets = LibraryIncludeFlags.All; LibraryIncludeFlags excludeAssets = LibraryIncludeFlags.None; LibraryIncludeFlags privateAssets = LibraryIncludeFlags.None; - if (package.HasFolder("build") && package.Imports.Count == 0) + if (package.PackageInfo.HasFolder("build") && package.Imports.Count == 0) { excludeAssets |= LibraryIncludeFlags.Build; } - if (package.HasFolder("lib") && package.AssemblyReferences.Count == 0) + if (package.PackageInfo.HasFolder("lib") && package.AssemblyReferences.Count == 0) { excludeAssets |= LibraryIncludeFlags.Compile; excludeAssets |= LibraryIncludeFlags.Runtime; } - if (package.HasFolder("analyzers") && package.AnalyzerItems.Count == 0) + if (package.PackageInfo.HasFolder("analyzers") && package.AnalyzerItems.Count == 0) { excludeAssets |= LibraryIncludeFlags.Analyzers; } @@ -181,9 +178,9 @@ private ProjectItemElement AddPackageReference(ProjectItemGroupElement itemGroup privateAssets = LibraryIncludeFlags.All; } - ProjectItemElement itemElement = itemGroupElement.AppendItem("PackageReference", package.PackageIdentity.Id); + ProjectItemElement itemElement = itemGroupElement.AppendItem("PackageReference", package.PackageInfo.Identity.Id); - itemElement.AddMetadataAsAttribute("Version", package.PackageVersion.ToNormalizedString()); + itemElement.AddMetadataAsAttribute("Version", package.PackageInfo.Identity.Version.ToNormalizedString()); if (includeAssets != LibraryIncludeFlags.All) { @@ -216,7 +213,14 @@ private bool ConvertProject(string projectPath, string packagesConfigPath) PackagesConfigReader packagesConfigReader = new PackagesConfigReader(XDocument.Load(packagesConfigPath)); - List packages = packagesConfigReader.GetPackages(allowDuplicatePackageIds: true).Select(i => new PackageReference(i.PackageIdentity, i.TargetFramework, i.IsUserInstalled, i.IsDevelopmentDependency, i.RequireReinstallation, i.AllowedVersions, PackagePathResolver, VersionFolderPathResolver)).ToList(); + List packages = packagesConfigReader + .GetPackages(allowDuplicatePackageIds: true) + .Select(i => + { + PackageInfo packageInfo = _packageInfoFactory.GetPackageInfo(i.PackageIdentity); + return new PackageUsage(i.PackageIdentity, packageInfo, i.IsDevelopmentDependency); + }) + .ToList(); List targetFrameworks = new List { @@ -246,9 +250,9 @@ private bool ConvertProject(string projectPath, string packagesConfigPath) Log.LogDebug(" Current package references:"); - foreach (PackageReference package in packages.Where(i => !i.IsMissingTransitiveDependency)) + foreach (PackageUsage package in packages.Where(i => !i.IsMissingTransitiveDependency)) { - Log.LogDebug($" {package.PackageIdentity}"); + Log.LogDebug($" {package.PackageInfo.Identity}"); } foreach (ProjectElement element in packages.Where(i => !i.IsMissingTransitiveDependency).SelectMany(i => i.AllElements)) @@ -264,7 +268,7 @@ private bool ConvertProject(string projectPath, string packagesConfigPath) Log.LogDebug(" Converted package references:"); - foreach (PackageReference package in packages) + foreach (PackageUsage package in packages) { ProjectItemElement packageReferenceItemElement = AddPackageReference(packageReferenceItemGroupElement, package); @@ -293,27 +297,28 @@ private bool ConvertProject(string projectPath, string packagesConfigPath) return true; } - private void DetectMissingTransitiveDependencies(List packages, string projectPath, RestoreTargetGraph restoreTargetGraph) + private void DetectMissingTransitiveDependencies(List packages, string projectPath, RestoreTargetGraph restoreTargetGraph) { IEnumerable> flatPackageDependencies = restoreTargetGraph.Flattened.Where(i => i.Key.Type == LibraryType.Package); foreach (GraphItem packageDependency in flatPackageDependencies) { (LibraryIdentity library, _) = (packageDependency.Key, packageDependency.Data); - PackageReference packageReference = packages.FirstOrDefault(i => i.PackageId.Equals(library.Name, StringComparison.OrdinalIgnoreCase)); + PackageUsage package = packages.FirstOrDefault(i => i.PackageId.Equals(library.Name, StringComparison.OrdinalIgnoreCase)); - if (packageReference == null) + if (package == null) { Log.LogWarning($"{projectPath}: The transitive package dependency \"{library.Name} {library.Version}\" was not in the packages.config. After converting to PackageReference, new dependencies will be pulled in transitively which could lead to restore or build errors"); - packages.Add(new PackageReference(new PackageIdentity(library.Name, library.Version), NuGetFramework.AnyFramework, true, false, true, new VersionRange(library.Version), PackagePathResolver, VersionFolderPathResolver) - { - IsMissingTransitiveDependency = true, - }); + PackageIdentity packageIdentity = new PackageIdentity(library.Name, library.Version); + PackageInfo packageInfo = _packageInfoFactory.GetPackageInfo(packageIdentity); + PackageUsage missingTransitiveDependency = new PackageUsage(packageIdentity, packageInfo, isDevelopmentDependency: false); + missingTransitiveDependency.IsMissingTransitiveDependency = true; + packages.Add(missingTransitiveDependency); } } } - private Match GetElementMatch(ElementPath elementPath, PackageReference package) + private Match GetElementMatch(ElementPath elementPath, PackageUsage package) { Match match = null; @@ -325,7 +330,7 @@ private Match GetElementMatch(ElementPath elementPath, PackageReference package) if (itemElement.ItemType.Equals("Analyzer", StringComparison.OrdinalIgnoreCase)) { - match = RegularExpressions.Analyzers[package.PackageIdentity].Match(elementPath.FullPath); + match = RegularExpressions.Analyzers[package.PackageInfo.Identity].Match(elementPath.FullPath); if (match.Success) { @@ -336,7 +341,7 @@ private Match GetElementMatch(ElementPath elementPath, PackageReference package) { if (File.Exists(elementPath.FullPath)) { - match = RegularExpressions.AssemblyReferences[package.PackageIdentity].Match(elementPath.FullPath); + match = RegularExpressions.AssemblyReferences[package.PackageInfo.Identity].Match(elementPath.FullPath); if (match.Success) { @@ -349,7 +354,7 @@ private Match GetElementMatch(ElementPath elementPath, PackageReference package) case ProjectImportElement importElement: - match = RegularExpressions.Imports[package.PackageIdentity].Match(elementPath.FullPath); + match = RegularExpressions.Imports[package.PackageInfo.Identity].Match(elementPath.FullPath); if (match.Success) { @@ -363,7 +368,7 @@ private Match GetElementMatch(ElementPath elementPath, PackageReference package) return match; } - private RestoreTargetGraph GetRestoreTargetGraph(List packages, string projectPath, List targetFrameworks) + private RestoreTargetGraph GetRestoreTargetGraph(List packages, string projectPath, List targetFrameworks) { // The package spec details what packages to restore PackageSpec packageSpec = new PackageSpec(targetFrameworks.Select(i => new TargetFrameworkInformation @@ -415,7 +420,7 @@ private bool IsPathRootedInRepositoryPath(string path) return path != null && path.StartsWith(_repositoryPath, StringComparison.OrdinalIgnoreCase); } - private void ProcessElement(ProjectElement element, List packages) + private void ProcessElement(ProjectElement element, List packages) { switch (element) { @@ -454,7 +459,7 @@ private void ProcessElement(ProjectElement element, List packa bool elementPathIsValid = false; - foreach (PackageReference package in packages.Where(i => !i.IsMissingTransitiveDependency)) + foreach (PackageUsage package in packages.Where(i => !i.IsMissingTransitiveDependency)) { Match match = GetElementMatch(elementPath, package); @@ -462,7 +467,7 @@ private void ProcessElement(ProjectElement element, List packa { elementPathIsValid = true; - if (!version.Equals(package.PackageIdentity.Version)) + if (!version.Equals(package.PackageInfo.Identity.Version)) { Log.LogWarning($" {element.Location}: The package version \"{version}\" specified in the \"{element.ElementName}\" element does not match the package version \"{package.PackageVersion}\". After conversion, the project will reference ONLY version \"{package.PackageVersion}\""); } @@ -471,7 +476,7 @@ private void ProcessElement(ProjectElement element, List packa if (!elementPathIsValid && IsPathRootedInRepositoryPath(elementPath.FullPath)) { - PackageReference package = packages.FirstOrDefault(i => !string.IsNullOrEmpty(i.RepositoryInstalledPath) && elementPath.FullPath.StartsWith(i.RepositoryInstalledPath)); + PackageUsage package = packages.FirstOrDefault(i => !string.IsNullOrEmpty(i.PackageInfo.RepositoryInstalledPath) && elementPath.FullPath.StartsWith(i.PackageInfo.RepositoryInstalledPath)); if (package == null) { @@ -479,7 +484,7 @@ private void ProcessElement(ProjectElement element, List packa } else { - string path = Path.Combine(package.GlobalInstalledPath, elementPath.FullPath.Substring(package.RepositoryInstalledPath.Length + 1)); + string path = Path.Combine(package.PackageInfo.GlobalInstalledPath, elementPath.FullPath.Substring(package.PackageInfo.RepositoryInstalledPath.Length + 1)); if (!File.Exists(path) && !Directory.Exists(path)) { @@ -507,11 +512,11 @@ private void ProcessElement(ProjectElement element, List packa } } - private void TrimPackages(List packages, string projectPath, ICollection> flatPackageDependencies) + private void TrimPackages(List packages, string projectPath, ICollection> flatPackageDependencies) { - IEnumerable GetPackageDependencies(PackageReference package) => flatPackageDependencies.First(p => p.Key.Name.Equals(package.PackageId, StringComparison.OrdinalIgnoreCase)).Data.Dependencies; + IEnumerable GetPackageDependencies(PackageUsage package) => flatPackageDependencies.First(p => p.Key.Name.Equals(package.PackageId, StringComparison.OrdinalIgnoreCase)).Data.Dependencies; - bool IsPackageInDependencySet(PackageReference packageReference, IEnumerable dependencies) => dependencies.Any(d => d.Name.Equals(packageReference.PackageId, StringComparison.OrdinalIgnoreCase)); + bool IsPackageInDependencySet(PackageUsage package, IEnumerable dependencies) => dependencies.Any(d => d.Name.Equals(package.PackageId, StringComparison.OrdinalIgnoreCase)); packages.RemoveAll(package => { @@ -519,7 +524,7 @@ private void TrimPackages(List packages, string projectPath, I if (willRemove) { - Log.LogWarning($"{projectPath}: The transitive package dependency {package.PackageId} {package.AllowedVersions} will be removed because it is referenced by another package in this dependency graph."); + Log.LogWarning($"{projectPath}: The transitive package dependency {package.PackageId} {package.PackageVersion} will be removed because it is referenced by another package in this dependency graph."); } return willRemove;