From 9717259b51aa50a475aff124961e4dbd79cfca88 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Thu, 7 Nov 2024 17:00:48 -0700 Subject: [PATCH 1/2] improve packages directory detection --- .../Update/PackagesConfigUpdaterTests.cs | 107 +++++++++++++++++- .../Updater/PackagesConfigUpdater.cs | 46 +++++++- 2 files changed, 145 insertions(+), 8 deletions(-) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs index 78e831da8c..811efa3b4c 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs @@ -6,18 +6,27 @@ public class PackagesConfigUpdaterTests : TestBase { [Theory] [MemberData(nameof(PackagesDirectoryPathTestData))] - public void PathToPackagesDirectoryCanBeDetermined(string projectContents, string dependencyName, string dependencyVersion, string expectedPackagesDirectoryPath) + public async Task PathToPackagesDirectoryCanBeDetermined(string projectContents, string? packagesConfigContents, string dependencyName, string dependencyVersion, string expectedPackagesDirectoryPath) { + using var tempDir = new TemporaryDirectory(); + string? packagesConfigPath = null; + if (packagesConfigContents is not null) + { + packagesConfigPath = Path.Join(tempDir.DirectoryPath, "packages.config"); + await File.WriteAllTextAsync(packagesConfigPath, packagesConfigContents); + } + var projectBuildFile = ProjectBuildFile.Parse("/", "project.csproj", projectContents); - var actualPackagesDirectorypath = PackagesConfigUpdater.GetPathToPackagesDirectory(projectBuildFile, dependencyName, dependencyVersion, "packages.config"); + var actualPackagesDirectorypath = PackagesConfigUpdater.GetPathToPackagesDirectory(projectBuildFile, dependencyName, dependencyVersion, packagesConfigPath); Assert.Equal(expectedPackagesDirectoryPath, actualPackagesDirectorypath); } - public static IEnumerable PackagesDirectoryPathTestData() + public static IEnumerable PackagesDirectoryPathTestData() { // project with namespace yield return [ + // project contents """ @@ -28,14 +37,20 @@ public static IEnumerable PackagesDirectoryPathTestData() """, + // packages.config contents + null, + // dependency name "Newtonsoft.Json", + // dependency version "7.0.1", + // expected packages directory path "../packages" ]; // project without namespace yield return [ + // project contents """ @@ -46,14 +61,20 @@ public static IEnumerable PackagesDirectoryPathTestData() """, + // packages.config contents + null, + // dependency name "Newtonsoft.Json", + // dependency version "7.0.1", + // expected packages directory path "../packages" ]; // project with non-standard packages path yield return [ + // project contents """ @@ -64,9 +85,89 @@ public static IEnumerable PackagesDirectoryPathTestData() """, + // packages.config contents + null, + // dependency name "Newtonsoft.Json", + // dependency version "7.0.1", + // expected packages directory path "../not-a-path-you-would-expect" ]; + + // project without expected packages path, but has others + yield return + [ + // project contents + """ + + + + ..\..\..\packages\Some.Other.Package.1.2.3\lib\net45\Some.Other.Package.dll + True + + + + """, + // packages.config contents + """ + + + + """, + // dependency name + "Newtonsoft.Json", + // dependency version + "7.0.1", + // expected packages directory path + "../../../packages" + ]; + + // project without expected package, but exists in packages.config, default is returned + yield return + [ + // project contents + """ + + + + + """, + // packages.config contents + """ + + + + """, + // dependency name + "Newtonsoft.Json", + // dependency version + "7.0.1", + // expected packages directory path + "../packages" + ]; + + // project without expected package and not in packages.config + yield return + [ + // project contents + """ + + + + + """, + // packages.config contents + """ + + + """, + // dependency name + "Newtonsoft.Json", + // dependency version + "7.0.1", + // expected packages directory path + null + ]; } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs index 4c0b129011..537e151e95 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs @@ -1,5 +1,6 @@ using System.Diagnostics; using System.Text; +using System.Text.RegularExpressions; using System.Xml.Linq; using System.Xml.XPath; @@ -213,8 +214,8 @@ private static Process[] GetLikelyNuGetSpawnedProcesses() var hintPathSubString = $"{dependencyName}.{dependencyVersion}"; string? partialPathMatch = null; - var hintPathNodes = projectBuildFile.Contents.Descendants().Where(e => e.IsHintPathNodeForDependency(dependencyName)); - foreach (var hintPathNode in hintPathNodes) + var specificHintPathNodes = projectBuildFile.Contents.Descendants().Where(e => e.IsHintPathNodeForDependency(dependencyName)).ToArray(); + foreach (var hintPathNode in specificHintPathNodes) { var hintPath = hintPathNode.GetContentValue(); var hintPathSubStringLocation = hintPath.IndexOf(hintPathSubString, StringComparison.OrdinalIgnoreCase); @@ -255,18 +256,53 @@ private static Process[] GetLikelyNuGetSpawnedProcesses() if (hasPackage) { // the dependency exists in the packages.config file, so it must be the second case - // the vast majority of projects found in the wild use this, and since we have nothing to look for, we'll just have to hope - partialPathMatch = "../packages"; + // at this point there's no good way to determine what the packages path is, but there's a really good chance that it looks + // something like one of these: + // ..\packages + // ..\..\packages + // etc. + // so first we'll see if we can find "packages\" somewhere in there + var genericHintPathNodes = projectBuildFile.Contents.Descendants().Where(IsHintPathNode).ToArray(); + if (genericHintPathNodes.Length > 0) + { + foreach (var hintPathNode in genericHintPathNodes) + { + // find the string "packages" followed by a directory separator... + var hintPath = hintPathNode.GetContentValue(); + var match = Regex.Match(hintPath, @"^(.*?packages[/\\])"); + if (match.Success) + { + // ...but remove the directory separator + partialPathMatch = match.Value[..^1]; + break; + } + } + } + else + { + // we know the dependency is used, but we have absolutely no idea where the packages path is, so we'll default to something reasonable + partialPathMatch = "../packages"; + } } } return partialPathMatch?.NormalizePathToUnix(); } - private static bool IsHintPathNodeForDependency(this IXmlElementSyntax element, string dependencyName) + private static bool IsHintPathNode(this IXmlElementSyntax element) { if (element.Name.Equals("HintPath", StringComparison.OrdinalIgnoreCase) && element.Parent.Name.Equals("Reference", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + return false; + } + + private static bool IsHintPathNodeForDependency(this IXmlElementSyntax element, string dependencyName) + { + if (element.IsHintPathNode()) { // the include attribute will look like one of the following: // From 31d077657d0b121d00f3717053cff85f170fe38b Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Fri, 8 Nov 2024 09:49:34 -0700 Subject: [PATCH 2/2] use better regex to find packages path --- .../Update/PackagesConfigUpdaterTests.cs | 4 ++-- .../Updater/PackagesConfigUpdater.cs | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs index 811efa3b4c..3697a1f2e9 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/PackagesConfigUpdaterTests.cs @@ -103,7 +103,7 @@ public async Task PathToPackagesDirectoryCanBeDetermined(string projectContents, - ..\..\..\packages\Some.Other.Package.1.2.3\lib\net45\Some.Other.Package.dll + ..\..\..\still-a-usable-path\Some.Other.Package.1.2.3\lib\net45\Some.Other.Package.dll True @@ -120,7 +120,7 @@ public async Task PathToPackagesDirectoryCanBeDetermined(string projectContents, // dependency version "7.0.1", // expected packages directory path - "../../../packages" + "../../../still-a-usable-path" ]; // project without expected package, but exists in packages.config, default is returned diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs index 537e151e95..87203c8466 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs @@ -256,24 +256,20 @@ private static Process[] GetLikelyNuGetSpawnedProcesses() if (hasPackage) { // the dependency exists in the packages.config file, so it must be the second case - // at this point there's no good way to determine what the packages path is, but there's a really good chance that it looks - // something like one of these: - // ..\packages - // ..\..\packages - // etc. - // so first we'll see if we can find "packages\" somewhere in there + // at this point there's no perfect way to determine what the packages path is, but there's a really good chance that + // for any given package it looks something like this: + // ..\..\packages\Package.Name.[version]\lib\Tfm\Package.Name.dll var genericHintPathNodes = projectBuildFile.Contents.Descendants().Where(IsHintPathNode).ToArray(); if (genericHintPathNodes.Length > 0) { foreach (var hintPathNode in genericHintPathNodes) { - // find the string "packages" followed by a directory separator... var hintPath = hintPathNode.GetContentValue(); - var match = Regex.Match(hintPath, @"^(.*?packages[/\\])"); + var match = Regex.Match(hintPath, @"^(?.*)[/\\](?[^/\\]+)[/\\]lib[/\\](?[^/\\]+)[/\\](?[^/\\]+)$"); + // e.g., ..\..\packages \ Some.Package.1.2.3 \ lib\ net45 \ Some.Package.dll if (match.Success) { - // ...but remove the directory separator - partialPathMatch = match.Value[..^1]; + partialPathMatch = match.Groups["PackagesPath"].Value; break; } }