From 2926273c94fdb91110cebe63644aa3a07f6c94d0 Mon Sep 17 00:00:00 2001 From: Leszek 'skolima' Ciesielski Date: Mon, 30 Apr 2018 16:14:00 +0200 Subject: [PATCH] Failure to update .NET AspNet WebApi projects (#242) * Patch WebApiProjects with a Condition on Import This allows the dotnet remove/add commands to work without trying to import a non-existent project * Follow project references when updating import conditions * Undo unnecessary change * Specify solution file explicitly in build This might help the random build errors, according to https://github.com/dotnet/sdk/issues/2076 --- .gitignore | 3 +- .travis.yml | 2 +- .../DotNetUpdatePackageCommandTests.cs | 8 +- .../UpdateProjectImportsCommandTests.cs | 85 +++++++++++++++++ NuKeeper/Engine/Packages/PackageUpdater.cs | 46 ++++----- .../Process/DotNetUpdatePackageCommand.cs | 2 +- NuKeeper/NuGet/Process/IFileRestoreCommand.cs | 2 +- ...tePackageCommand.cs => IPackageCommand.cs} | 2 +- .../NuGet/Process/NuGetFileRestoreCommand.cs | 7 ++ .../Process/NuGetUpdatePackageCommand.cs | 2 +- .../Process/UpdateProjectImportsCommand.cs | 94 +++++++++++++++++++ 11 files changed, 223 insertions(+), 30 deletions(-) create mode 100644 NuKeeper.Integration.Tests/NuGet/Process/UpdateProjectImportsCommandTests.cs rename NuKeeper/NuGet/Process/{IUpdatePackageCommand.cs => IPackageCommand.cs} (85%) create mode 100644 NuKeeper/NuGet/Process/UpdateProjectImportsCommand.cs diff --git a/.gitignore b/.gitignore index 1d573ce24..f9cb768f1 100644 --- a/.gitignore +++ b/.gitignore @@ -41,5 +41,4 @@ _ReSharper*/ # Custom NuKeeper/Properties/launchSettings.json - - +*.orig diff --git a/.travis.yml b/.travis.yml index bb8451bb0..2737c914b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ mono: none dist: trusty dotnet: 2.1.105 script: - - dotnet build -c Release -f netcoreapp2.0 + - dotnet build -c Release -f netcoreapp2.0 NuKeeper.sln /m:1 - dotnet test -c Release -f netcoreapp2.0 NuKeeper.Tests/NuKeeper.Tests.csproj --filter "TestCategory!=WindowsOnly" - dotnet test -c Release -f netcoreapp2.0 NuKeeper.Inspection.Tests/NuKeeper.Inspection.Tests.csproj --filter "TestCategory!=WindowsOnly" - dotnet test -c Release -f netcoreapp2.0 NuKeeper.Integration.Tests/NuKeeper.Integration.Tests.csproj --filter "TestCategory!=WindowsOnly" diff --git a/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs b/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs index e26f45425..d54225167 100644 --- a/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs +++ b/NuKeeper.Integration.Tests/NuGet/Process/DotNetUpdatePackageCommandTests.cs @@ -22,8 +22,13 @@ public class DotNetUpdatePackageCommandTests $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) v4.7 + + + + + - + "; private readonly string _testDotNetCoreProject = @@ -55,7 +60,6 @@ public class DotNetUpdatePackageCommandTests "; [Test] - [Ignore("Known failure, issue #239")] public async Task ShouldNotThrowOnWebProjectMixedStyleUpdates() { await ExecuteValidUpdateTest(_testWebApiProject); diff --git a/NuKeeper.Integration.Tests/NuGet/Process/UpdateProjectImportsCommandTests.cs b/NuKeeper.Integration.Tests/NuGet/Process/UpdateProjectImportsCommandTests.cs new file mode 100644 index 000000000..0dedfbe44 --- /dev/null +++ b/NuKeeper.Integration.Tests/NuGet/Process/UpdateProjectImportsCommandTests.cs @@ -0,0 +1,85 @@ +using System.IO; +using System.Threading.Tasks; +using NuKeeper.Inspection.RepositoryInspection; +using NuKeeper.NuGet.Process; +using NUnit.Framework; + +namespace NuKeeper.Integration.Tests.NuGet.Process +{ + [TestFixture] + public class UpdateProjectImportsCommandTests + { + private readonly string _testWebApiProject = + @" + + + + +"; + + private readonly string _projectWithReference = + @""; + + private readonly string _unpatchedImport = + @""; + + private readonly string _patchedImport = + @""; + + [Test] + public async Task ShouldUpdateConditionOnTaskImport() + { + var workDirectory = Path.Combine(TestContext.CurrentContext.WorkDirectory, + nameof(ShouldUpdateConditionOnTaskImport)); + + Directory.CreateDirectory(workDirectory); + var projectName = nameof(ShouldUpdateConditionOnTaskImport) + ".csproj"; + var projectPath = Path.Combine(workDirectory, projectName); + await File.WriteAllTextAsync(projectPath, _testWebApiProject); + + var subject = new UpdateProjectImportsCommand(); + + await subject.Invoke(null, null, + new PackageInProject("acme", "1", + new PackagePath(workDirectory, projectName, PackageReferenceType.ProjectFileOldStyle))); + + var updatedContents = await File.ReadAllTextAsync(projectPath); + + Assert.That(updatedContents, Does.Not.Contain(_unpatchedImport)); + Assert.That(updatedContents, Does.Contain(_patchedImport)); + } + + [Test] + public async Task ShouldFollowResolvableImports() + { + var workDirectory = Path.Combine(TestContext.CurrentContext.WorkDirectory, + nameof(ShouldFollowResolvableImports)); + + Directory.CreateDirectory(workDirectory); + + var projectName = nameof(ShouldFollowResolvableImports) + ".csproj"; + var projectPath = Path.Combine(workDirectory, projectName); + await File.WriteAllTextAsync(projectPath, _testWebApiProject); + + var intermediateProject = Path.Combine(workDirectory, "Intermediate.csproj"); + var intermediateContents = _projectWithReference.Replace("{importPath}", projectPath); + await File.WriteAllTextAsync(intermediateProject, intermediateContents); + + var rootProject = Path.Combine(workDirectory, "RootProject.csproj"); + var rootContets = _projectWithReference.Replace("{importPath}", + Path.Combine("..", nameof(ShouldFollowResolvableImports), "Intermediate.csproj")); + await File.WriteAllTextAsync(rootProject, rootContets); + + var subject = new UpdateProjectImportsCommand(); + + await subject.Invoke(null, null, + new PackageInProject("acme", "1", + new PackagePath(workDirectory, "RootProject.csproj", PackageReferenceType.ProjectFileOldStyle))); + + var updatedContents = await File.ReadAllTextAsync(projectPath); + + Assert.That(updatedContents, Does.Not.Contain(_unpatchedImport)); + Assert.That(updatedContents, Does.Contain(_patchedImport)); + } + } +} diff --git a/NuKeeper/Engine/Packages/PackageUpdater.cs b/NuKeeper/Engine/Packages/PackageUpdater.cs index e12deaef6..ad1164054 100644 --- a/NuKeeper/Engine/Packages/PackageUpdater.cs +++ b/NuKeeper/Engine/Packages/PackageUpdater.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; using NuKeeper.Configuration; using NuKeeper.Git; @@ -64,35 +65,38 @@ private async Task UpdateAllCurrentUsages(PackageUpdateSet updateSet) { foreach (var current in updateSet.CurrentPackages) { - var restoreCommand = GetRestoreCommand(current.Path.PackageReferenceType); - if (restoreCommand != null) + var updateCommands = GetUpdateCommands(current.Path.PackageReferenceType); + foreach (var updateCommand in updateCommands) { - await restoreCommand.Invoke(current.Path.Info); + await updateCommand.Invoke(updateSet.SelectedVersion, updateSet.Selected.Source, current); } - - var updateCommand = GetUpdateCommand(current.Path.PackageReferenceType); - await updateCommand.Invoke(updateSet.SelectedVersion, updateSet.Selected.Source, current); - } - } - - private IFileRestoreCommand GetRestoreCommand(PackageReferenceType packageReferenceType) - { - if (packageReferenceType != PackageReferenceType.ProjectFile) - { - return new NuGetFileRestoreCommand(_logger, _settings); } - - return null; } - private IUpdatePackageCommand GetUpdateCommand(PackageReferenceType packageReferenceType) + private IReadOnlyCollection GetUpdateCommands(PackageReferenceType packageReferenceType) { - if (packageReferenceType != PackageReferenceType.PackagesConfig) + switch (packageReferenceType) { - return new DotNetUpdatePackageCommand(_logger, _settings); + case PackageReferenceType.PackagesConfig: + return new IPackageCommand[] + { + new NuGetFileRestoreCommand(_logger, _settings), + new NuGetUpdatePackageCommand(_logger, _settings) + }; + + case PackageReferenceType.ProjectFileOldStyle: + return new IPackageCommand[] + { + new UpdateProjectImportsCommand(), + new NuGetFileRestoreCommand(_logger, _settings), + new DotNetUpdatePackageCommand(_logger, _settings) + }; + + case PackageReferenceType.ProjectFile: + return new[] {new DotNetUpdatePackageCommand(_logger, _settings)}; + + default: throw new ArgumentOutOfRangeException(nameof(packageReferenceType)); } - - return new NuGetUpdatePackageCommand(_logger, _settings); } private async Task MakeGitHubPullRequest( diff --git a/NuKeeper/NuGet/Process/DotNetUpdatePackageCommand.cs b/NuKeeper/NuGet/Process/DotNetUpdatePackageCommand.cs index 8668ddbaa..dc9db7f32 100644 --- a/NuKeeper/NuGet/Process/DotNetUpdatePackageCommand.cs +++ b/NuKeeper/NuGet/Process/DotNetUpdatePackageCommand.cs @@ -10,7 +10,7 @@ namespace NuKeeper.NuGet.Process { - public class DotNetUpdatePackageCommand : IUpdatePackageCommand + public class DotNetUpdatePackageCommand : IPackageCommand { private readonly IExternalProcess _externalProcess; private readonly INuKeeperLogger _logger; diff --git a/NuKeeper/NuGet/Process/IFileRestoreCommand.cs b/NuKeeper/NuGet/Process/IFileRestoreCommand.cs index fb8ab68bf..82c292ba2 100644 --- a/NuKeeper/NuGet/Process/IFileRestoreCommand.cs +++ b/NuKeeper/NuGet/Process/IFileRestoreCommand.cs @@ -3,7 +3,7 @@ namespace NuKeeper.NuGet.Process { - public interface IFileRestoreCommand + public interface IFileRestoreCommand : IPackageCommand { Task Invoke(FileInfo file); } diff --git a/NuKeeper/NuGet/Process/IUpdatePackageCommand.cs b/NuKeeper/NuGet/Process/IPackageCommand.cs similarity index 85% rename from NuKeeper/NuGet/Process/IUpdatePackageCommand.cs rename to NuKeeper/NuGet/Process/IPackageCommand.cs index 118710ef1..c470e4f2e 100644 --- a/NuKeeper/NuGet/Process/IUpdatePackageCommand.cs +++ b/NuKeeper/NuGet/Process/IPackageCommand.cs @@ -4,7 +4,7 @@ namespace NuKeeper.NuGet.Process { - public interface IUpdatePackageCommand + public interface IPackageCommand { Task Invoke(NuGetVersion newVersion, string packageSource, PackageInProject currentPackage); } diff --git a/NuKeeper/NuGet/Process/NuGetFileRestoreCommand.cs b/NuKeeper/NuGet/Process/NuGetFileRestoreCommand.cs index 719d397fa..de1fbbedf 100644 --- a/NuKeeper/NuGet/Process/NuGetFileRestoreCommand.cs +++ b/NuKeeper/NuGet/Process/NuGetFileRestoreCommand.cs @@ -3,9 +3,11 @@ using System.Linq; using System.Runtime.InteropServices; using System.Threading.Tasks; +using NuGet.Versioning; using NuKeeper.Configuration; using NuKeeper.Inspection.Formats; using NuKeeper.Inspection.Logging; +using NuKeeper.Inspection.RepositoryInspection; using NuKeeper.ProcessRunner; namespace NuKeeper.NuGet.Process @@ -61,6 +63,11 @@ public async Task Invoke(FileInfo file) } } + public async Task Invoke(NuGetVersion selectedVersion, string source, PackageInProject current) + { + await Invoke(current.Path.Info); + } + private static string GetSourcesCommandLine(IEnumerable sources) { return sources.Select(s => $"-Source {s}").JoinWithSeparator(" "); diff --git a/NuKeeper/NuGet/Process/NuGetUpdatePackageCommand.cs b/NuKeeper/NuGet/Process/NuGetUpdatePackageCommand.cs index deff4c5ef..6d2152a07 100644 --- a/NuKeeper/NuGet/Process/NuGetUpdatePackageCommand.cs +++ b/NuKeeper/NuGet/Process/NuGetUpdatePackageCommand.cs @@ -11,7 +11,7 @@ namespace NuKeeper.NuGet.Process { - public class NuGetUpdatePackageCommand : IUpdatePackageCommand + public class NuGetUpdatePackageCommand : IPackageCommand { private readonly IExternalProcess _externalProcess; private readonly INuKeeperLogger _logger; diff --git a/NuKeeper/NuGet/Process/UpdateProjectImportsCommand.cs b/NuKeeper/NuGet/Process/UpdateProjectImportsCommand.cs new file mode 100644 index 000000000..9296522d1 --- /dev/null +++ b/NuKeeper/NuGet/Process/UpdateProjectImportsCommand.cs @@ -0,0 +1,94 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; +using NuGet.Versioning; +using NuKeeper.Inspection.RepositoryInspection; + +namespace NuKeeper.NuGet.Process +{ + public class UpdateProjectImportsCommand : IPackageCommand + { + public async Task Invoke(NuGetVersion newVersion, string packageSource, PackageInProject currentPackage) + { + var projectsToUpdate = new Stack(); + projectsToUpdate.Push(currentPackage.Path.FullName); + + while (projectsToUpdate.TryPop(out var currentProject)) + { + using (var projectContents = File.Open(currentProject, FileMode.Open, FileAccess.ReadWrite)) + { + var projectsToCheck = await UpdateConditionsOnProjects(projectContents); + foreach (var potentialProject in projectsToCheck) + { + var fullPath = + Path.GetFullPath(Path.Combine(Path.GetDirectoryName(currentProject), potentialProject)); + if (File.Exists(fullPath)) + { + projectsToUpdate.Push(fullPath); + } + } + } + } + } + + private async Task> UpdateConditionsOnProjects(Stream fileContents) + { + var xml = XDocument.Load(fileContents); + var ns = xml.Root.GetDefaultNamespace(); + + var project = xml.Element(ns + "Project"); + + if (project == null) + { + return Enumerable.Empty(); + } + + var imports = project.Elements(ns + "Import"); + var importsWithToolsPath = imports + .Where(i => i.Attributes("Project").Any(a => a.Value.Contains("$(VSToolsPath)"))).ToList(); + var importsWithoutCondition = importsWithToolsPath.Where(i => !i.Attributes("Condition").Any()); + var importsWithBrokenVsToolsCondition = importsWithToolsPath.Where(i => + i.Attributes("Condition").Any(a => a.Value == "\'$(VSToolsPath)\' != \'\'")); + + var saveRequired = false; + foreach (var importToFix in importsWithBrokenVsToolsCondition.Concat(importsWithoutCondition)) + { + saveRequired = true; + UpdateImportNode(importToFix); + } + + if (saveRequired) + { + fileContents.Seek(0, SeekOrigin.Begin); + await xml.SaveAsync(fileContents, SaveOptions.None, CancellationToken.None); + } + + return FindProjectReferences(project, ns); + } + + private static IEnumerable FindProjectReferences(XElement project, XNamespace ns) + { + var itemGroups = project.Elements(ns + "ItemGroup"); + var projectReferences = itemGroups.SelectMany(ig => ig.Elements(ns + "ProjectReference")); + var includes = projectReferences.Attributes("Include").Select(a => a.Value); + return includes; + } + + private static void UpdateImportNode(XElement importToFix) + { + var importPath = importToFix.Attribute("Project").Value; + var condition = $"Exists('{importPath}')"; + if (!importToFix.Attributes("Condition").Any()) + { + importToFix.Add(new XAttribute("Condition", condition)); + } + else + { + importToFix.Attribute("Condition").Value = condition; + } + } + } +}