diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/BindingRedirectsTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/BindingRedirectsTests.cs new file mode 100644 index 00000000000..6914ff07685 --- /dev/null +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/BindingRedirectsTests.cs @@ -0,0 +1,225 @@ +using Xunit; + +namespace NuGetUpdater.Core.Test.Update; + +public class BindingRedirectsTests +{ + [Fact] + public async Task SimpleBindingRedirectIsPerformed() + { + await VerifyBindingRedirectsAsync( + projectContents: """ + + + + v4.5 + + + + + + + packages\Some.Package.2.0.0\lib\net45\Some.Package.dll + True + + + + + """, + configContents: """ + + + + + + + + + + + """, + updatedPackageName: "Some.Package", + updatedPackageVersion: "2.0.0", + expectedConfigContents: """ + + + + + + + + + + + """ + ); + } + + [Fact] + public async Task ConfigFileIndentationIsPreserved() + { + await VerifyBindingRedirectsAsync( + projectContents: """ + + + + v4.5 + + + + + + + packages\Some.Package.2.0.0\lib\net45\Some.Package.dll + True + + + + + """, + configContents: """ + + + + + + + + + + + """, + updatedPackageName: "Some.Package", + updatedPackageVersion: "2.0.0", + expectedConfigContents: """ + + + + + + + + + + + """ + ); + } + + [Fact] + public async Task NoExtraBindingsAreAdded() + { + await VerifyBindingRedirectsAsync( + projectContents: """ + + + + v4.5 + + + + + + + packages\Some.Package.2.0.0\lib\net45\Some.Package.dll + True + + + packages\Some.Unrelated.Package.3.0.0\lib\net45\Some.Package.dll + True + + + + + """, + configContents: """ + + + + + + + + + + + """, + updatedPackageName: "Some.Package", + updatedPackageVersion: "2.0.0", + expectedConfigContents: """ + + + + + + + + + + + """ + ); + } + + [Fact] + public async Task NewBindingIsAdded() + { + await VerifyBindingRedirectsAsync( + projectContents: """ + + + + v4.5 + + + + + + + packages\Some.Package.2.0.0\lib\net45\Some.Package.dll + True + + + + + """, + configContents: """ + + + + """, + updatedPackageName: "Some.Package", + updatedPackageVersion: "2.0.0", + expectedConfigContents: """ + + + + + + + + + + + """ + ); + } + + private static async Task VerifyBindingRedirectsAsync(string projectContents, string configContents, string expectedConfigContents, string updatedPackageName, string updatedPackageVersion, string configFileName = "app.config") + { + using var tempDir = new TemporaryDirectory(); + var projectFileName = "project.csproj"; + var projectFilePath = Path.Combine(tempDir.DirectoryPath, projectFileName); + var configFilePath = Path.Combine(tempDir.DirectoryPath, configFileName); + + await File.WriteAllTextAsync(projectFilePath, projectContents); + await File.WriteAllTextAsync(configFilePath, configContents); + + var projectBuildFile = ProjectBuildFile.Open(tempDir.DirectoryPath, projectFilePath); + await BindingRedirectManager.UpdateBindingRedirectsAsync(projectBuildFile, updatedPackageName, updatedPackageVersion); + + var actualConfigContents = (await File.ReadAllTextAsync(configFilePath)).Replace("\r", ""); + expectedConfigContents = expectedConfigContents.Replace("\r", ""); + Assert.Equal(expectedConfigContents, actualConfigContents); + } +} diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs index e02bb35fcab..6f5117ab8f6 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.PackagesConfig.cs @@ -594,6 +594,7 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", [ MockNuGetPackage.CreatePackageWithAssembly("Some.Package", "7.0.1", "net45", "7.0.0.0"), MockNuGetPackage.CreatePackageWithAssembly("Some.Package", "13.0.1", "net45", "13.0.0.0"), + MockNuGetPackage.CreatePackageWithAssembly("Unrelated.Package", "1.2.3", "net45","1.2.0.0"), ], projectContents: """ @@ -612,6 +613,10 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", packages\Some.Package.7.0.1\lib\net45\Some.Package.dll True + + packages\Unrelated.Package.1.2.3\lib\net45\Unrelated.Package.dll + True + @@ -653,6 +658,107 @@ await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", packages\Some.Package.13.0.1\lib\net45\Some.Package.dll True + + packages\Unrelated.Package.1.2.3\lib\net45\Unrelated.Package.dll + True + + + + + """, + expectedPackagesConfigContents: """ + + + + + """, + additionalFilesExpected: + [ + ("app.config", """ + + + + + + + + + + + """) + ] + ); + } + + [Fact] + public async Task BindingRedirectIsAddedForUpdatedPackage() + { + await TestUpdateForProject("Some.Package", "7.0.1", "13.0.1", + packages: + [ + MockNuGetPackage.CreatePackageWithAssembly("Some.Package", "7.0.1", "net45", "7.0.0.0"), + MockNuGetPackage.CreatePackageWithAssembly("Some.Package", "13.0.1", "net45", "13.0.0.0"), + MockNuGetPackage.CreatePackageWithAssembly("Unrelated.Package", "1.2.3", "net45","1.2.0.0"), + ], + projectContents: """ + + + + v4.5 + + + + + + + + + + packages\Some.Package.7.0.1\lib\net45\Some.Package.dll + True + + + packages\Unrelated.Package.1.2.3\lib\net45\Unrelated.Package.dll + True + + + + + """, + packagesConfigContents: """ + + + + """, + additionalFiles: + [ + ("app.config", """ + + + + """) + ], + expectedProjectContents: """ + + + + v4.5 + + + + + + + + + + packages\Some.Package.13.0.1\lib\net45\Some.Package.dll + True + + + packages\Unrelated.Package.1.2.3\lib\net45\Unrelated.Package.dll + True + diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs index be0cf0ed8f7..c27c93cfac0 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/BindingRedirectManager.cs @@ -28,7 +28,9 @@ internal static class BindingRedirectManager /// https://learn.microsoft.com/en-us/nuget/resources/check-project-format /// /// The project build file (*.xproj) to be updated - public static async ValueTask UpdateBindingRedirectsAsync(ProjectBuildFile projectBuildFile) + /// The name of the package that was updated + /// The version of the package that was updated + public static async ValueTask UpdateBindingRedirectsAsync(ProjectBuildFile projectBuildFile, string updatedPackageName, string updatedPackageVersion) { var configFile = await TryGetRuntimeConfigurationFile(projectBuildFile); if (configFile is null) @@ -47,7 +49,20 @@ public static async ValueTask UpdateBindingRedirectsAsync(ProjectBuildFile proje return; } - var fileContent = AddBindingRedirects(configFile, bindings); + // we need to detect what assembly references come from the newly updated package; the `HintPath` will look like + // ..\packages\Some.Package.1.2.3\lib\net45\Some.Package.dll + // so we first pull out the packages sub-path, e.g., `..\packages` + // then we add the updated package name, version, and a trailing directory separator and ensure it's a unix-style path + // e.g., ../packages/Some.Package/1.2.3/ + // at this point any assembly in that directory is from the updated package and will need a binding redirect + // finally we pull out the assembly `HintPath` values for _all_ references relative to the project file in a unix-style value + // e.g., ../packages/Some.Other.Package/4.5.6/lib/net45/Some.Other.Package.dll + // all of that is passed to `AddBindingRedirects()` so we can ensure binding redirects for the relevant assemblies + var packagesDirectory = PackagesConfigUpdater.GetPathToPackagesDirectory(projectBuildFile, updatedPackageName, updatedPackageVersion, packagesConfigPath: null)!; + var assemblyPathPrefix = Path.Combine(packagesDirectory, $"{updatedPackageName}.{updatedPackageVersion}").NormalizePathToUnix().EnsureSuffix("/"); + var assemblyPaths = references.Select(static x => x.HintPath).Select(x => Path.GetRelativePath(Path.GetDirectoryName(projectBuildFile.Path)!, x).NormalizePathToUnix()).ToList(); + var bindingsAndAssemblyPaths = bindings.Zip(assemblyPaths); + var fileContent = AddBindingRedirects(configFile, bindingsAndAssemblyPaths, assemblyPathPrefix); configFile = configFile with { Content = fileContent }; await File.WriteAllTextAsync(configFile.Path, configFile.Content); @@ -161,10 +176,10 @@ static bool IsConfigFile(IXmlElementSyntax element) } } - private static string AddBindingRedirects(ConfigurationFile configFile, IEnumerable bindingRedirects) + private static string AddBindingRedirects(ConfigurationFile configFile, IEnumerable<(Runtime_AssemblyBinding Binding, string AssemblyPath)> bindingRedirectsAndAssemblyPaths, string assemblyPathPrefix) { // Do nothing if there are no binding redirects to add, bail out - if (!bindingRedirects.Any()) + if (!bindingRedirectsAndAssemblyPaths.Any()) { return configFile.Content; } @@ -185,7 +200,7 @@ private static string AddBindingRedirects(ConfigurationFile configFile, IEnumera // Get all of the current bindings in config var currentBindings = GetAssemblyBindings(runtime); - foreach (var bindingRedirect in bindingRedirects) + foreach (var (bindingRedirect, assemblyPath) in bindingRedirectsAndAssemblyPaths) { // If the binding redirect already exists in config, update it. Otherwise, add it. var bindingAssemblyIdentity = new AssemblyIdentity(bindingRedirect.Name, bindingRedirect.PublicKeyToken); @@ -208,11 +223,17 @@ private static string AddBindingRedirects(ConfigurationFile configFile, IEnumera } else { - // Get an assembly binding element to use - var assemblyBindingElement = GetAssemblyBindingElement(runtime); + // only add a previously missing binding redirect if it's related to the package that caused the whole update + // this isn't strictly necessary, but can be helpful to the end user and it's easy for them to revert if they + // don't like this particular change + if (assemblyPath.StartsWith(assemblyPathPrefix, StringComparison.OrdinalIgnoreCase)) + { + // Get an assembly binding element to use + var assemblyBindingElement = GetAssemblyBindingElement(runtime); - // Add the binding to that element - assemblyBindingElement.AddIndented(bindingRedirect.ToXElement()); + // Add the binding to that element + assemblyBindingElement.AddIndented(bindingRedirect.ToXElement()); + } } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs index 98720390d53..4c0b1290115 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/PackagesConfigUpdater.cs @@ -93,7 +93,7 @@ ILogger logger projectBuildFile.NormalizeDirectorySeparatorsInProject(); // Update binding redirects - await BindingRedirectManager.UpdateBindingRedirectsAsync(projectBuildFile); + await BindingRedirectManager.UpdateBindingRedirectsAsync(projectBuildFile, dependencyName, newDependencyVersion); logger.Log(" Writing project file back to disk"); await projectBuildFile.SaveAsync(); @@ -196,7 +196,7 @@ private static Process[] GetLikelyNuGetSpawnedProcesses() return processes; } - internal static string? GetPathToPackagesDirectory(ProjectBuildFile projectBuildFile, string dependencyName, string dependencyVersion, string packagesConfigPath) + internal static string? GetPathToPackagesDirectory(ProjectBuildFile projectBuildFile, string dependencyName, string dependencyVersion, string? packagesConfigPath) { // the packages directory can be found from the hint path of the matching dependency, e.g., when given "Newtonsoft.Json", "7.0.1", and a project like this: // @@ -242,7 +242,7 @@ private static Process[] GetLikelyNuGetSpawnedProcesses() } } - if (partialPathMatch is null) + if (partialPathMatch is null && packagesConfigPath is not null) { // if we got this far, we couldn't find the packages directory for the specified dependency and there are 2 possibilities: // 1. the dependency doesn't actually exist in this project diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs index 8d680cb9913..59ff5fa5e95 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/PathHelper.cs @@ -27,6 +27,8 @@ public static string JoinPath(string? path1, string path2) public static string EnsurePrefix(this string s, string prefix) => s.StartsWith(prefix) ? s : prefix + s; + public static string EnsureSuffix(this string s, string suffix) => s.EndsWith(suffix) ? s : s + suffix; + public static string NormalizePathToUnix(this string path) => path.Replace("\\", "/"); public static string NormalizeUnixPathParts(this string path)