From 5bbac55736ab0246b00fe2d8933b0d29b7a42800 Mon Sep 17 00:00:00 2001 From: Justin Goshi Date: Thu, 19 Jan 2017 11:23:01 -0800 Subject: [PATCH 1/2] Migration: do not add a csproj to the solution if it already exists --- .../TestApp/Program.cs | 16 ++++++++++ .../TestApp/TestApp.sln | 25 ++++++++++++++++ .../TestApp/TestApp.xproj | 18 +++++++++++ .../TestApp/project.json | 30 +++++++++++++++++++ .../TestLibrary/Helper.cs | 15 ++++++++++ .../TestLibrary/TestLibrary.csproj | 13 ++++++++ .../TestLibrary/TestLibrary.xproj | 18 +++++++++++ .../TestLibrary/project.json | 9 ++++++ .../commands/dotnet-migrate/MigrateCommand.cs | 21 +++++++++---- .../GivenThatIWantToMigrateSolutions.cs | 18 +++++++++++ 10 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/Program.cs create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.sln create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.xproj create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/project.json create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/Helper.cs create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.csproj create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.xproj create mode 100644 TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/project.json diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/Program.cs b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/Program.cs new file mode 100644 index 000000000..7937a9ba0 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/Program.cs @@ -0,0 +1,16 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace TestApp +{ + public class Program + { + public static int Main(string[] args) + { + Console.WriteLine(TestLibrary.Helper.GetMessage()); + return 0; + } + } +} diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.sln b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.sln new file mode 100644 index 000000000..0935de81a --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.sln @@ -0,0 +1,25 @@ + +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio 14 +VisualStudioVersion = 14.0.25420.1 +MinimumVisualStudioVersion = 10.0.40219.1 +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "TestApp", "TestApp.xproj", "{0138CB8F-4AA9-4029-A21E-C07C30F425BA}" +EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "TestLibrary", "..\TestLibrary\TestLibrary.csproj", "{DC0B35D0-8A36-4B52-8A11-B86739F055D2}" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {0138CB8F-4AA9-4029-A21E-C07C30F425BA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {0138CB8F-4AA9-4029-A21E-C07C30F425BA}.Debug|Any CPU.Build.0 = Debug|Any CPU + {0138CB8F-4AA9-4029-A21E-C07C30F425BA}.Release|Any CPU.ActiveCfg = Release|Any CPU + {0138CB8F-4AA9-4029-A21E-C07C30F425BA}.Release|Any CPU.Build.0 = Release|Any CPU + {DC0B35D0-8A36-4B52-8A11-B86739F055D2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {DC0B35D0-8A36-4B52-8A11-B86739F055D2}.Debug|Any CPU.Build.0 = Debug|Any CPU + {DC0B35D0-8A36-4B52-8A11-B86739F055D2}.Release|Any CPU.ActiveCfg = Release|Any CPU + {DC0B35D0-8A36-4B52-8A11-B86739F055D2}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection +EndGlobal diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.xproj b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.xproj new file mode 100644 index 000000000..d18702195 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.xproj @@ -0,0 +1,18 @@ + + + + 14.0.23107 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + 0138cb8f-4aa9-4029-a21e-c07c30f425ba + TestAppWithContents + ..\..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\..\artifacts\ + + + 2.0 + + + diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/project.json b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/project.json new file mode 100644 index 000000000..666d644b9 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/project.json @@ -0,0 +1,30 @@ +{ + "version": "1.0.0-*", + "buildOptions": { + "emitEntryPoint": true, + "preserveCompilationContext": true + }, + "dependencies": { + "TestLibrary": { + "target": "project", + "version": "1.0.0-*" + }, + "Microsoft.NETCore.App": "1.0.1" + }, + "frameworks": { + "netcoreapp1.0": {} + }, + "runtimes": { + "win7-x64": {}, + "win7-x86": {}, + "osx.10.10-x64": {}, + "osx.10.11-x64": {}, + "ubuntu.14.04-x64": {}, + "ubuntu.16.04-x64": {}, + "centos.7-x64": {}, + "rhel.7.2-x64": {}, + "debian.8-x64": {}, + "fedora.23-x64": {}, + "opensuse.13.2-x64": {} + } +} diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/Helper.cs b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/Helper.cs new file mode 100644 index 000000000..6c3bc0e53 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/Helper.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; + +namespace TestLibrary +{ + public static class Helper + { + public static string GetMessage() + { + return "This string came from the test library!"; + } + } +} diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.csproj b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.csproj new file mode 100644 index 000000000..2debc348d --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.csproj @@ -0,0 +1,13 @@ + + + + netstandard1.5 + TestLibrary + TestLibrary + + + + + + + diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.xproj b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.xproj new file mode 100644 index 000000000..dc8eb7060 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.xproj @@ -0,0 +1,18 @@ + + + + 14.0.23107 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + dc0b35d0-8a36-4b52-8a11-b86739f055d2 + TestAppWithContents + ..\..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\..\artifacts\ + + + 2.0 + + + diff --git a/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/project.json b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/project.json new file mode 100644 index 000000000..032754312 --- /dev/null +++ b/TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/project.json @@ -0,0 +1,9 @@ +{ + "version": "1.0.0-*", + "dependencies": { + "NETStandard.Library": "1.6.0" + }, + "frameworks": { + "netstandard1.5": {} + } +} diff --git a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs index 9a61d3a3e..54f9a56a2 100644 --- a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs +++ b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs @@ -109,17 +109,18 @@ namespace Microsoft.DotNet.Tools.Migrate return; } - List csprojFilesToAdd = new List(); + var csprojFilesToAdd = new HashSet(); var slnPathWithTrailingSlash = PathUtility.EnsureTrailingSlash(_slnFile.BaseDirectory); foreach (var report in migrationReport.ProjectMigrationReports) { var reportPathWithTrailingSlash = PathUtility.EnsureTrailingSlash(report.ProjectDirectory); - var reportRelPath = Path.Combine( - PathUtility.GetRelativePath(slnPathWithTrailingSlash, reportPathWithTrailingSlash), - report.ProjectName + ".xproj"); + var relReportPath = PathUtility.GetRelativePath( + slnPathWithTrailingSlash, + reportPathWithTrailingSlash); - var projects = _slnFile.Projects.Where(p => p.FilePath == reportRelPath); + var xprojPath = Path.Combine(relReportPath, report.ProjectName + ".xproj"); + var projects = _slnFile.Projects.Where(p => p.FilePath == xprojPath); var migratedProjectName = report.ProjectName + ".csproj"; if (projects.Count() == 1) @@ -132,7 +133,15 @@ namespace Microsoft.DotNet.Tools.Migrate } else { - csprojFilesToAdd.Add(Path.Combine(report.ProjectDirectory, migratedProjectName)); + var csprojPath = Path.Combine(relReportPath, migratedProjectName); + var slnAlreadyContainsMigratedCsproj = _slnFile.Projects + .Where(p => p.FilePath == csprojPath) + .Any(); + + if (!slnAlreadyContainsMigratedCsproj) + { + csprojFilesToAdd.Add(Path.Combine(report.ProjectDirectory, migratedProjectName)); + } } foreach (var preExisting in report.PreExistingCsprojDependencies) diff --git a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs index 17ee1299a..2111a9ac7 100644 --- a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs +++ b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs @@ -38,6 +38,24 @@ namespace Microsoft.DotNet.Migration.Tests "PJAppWithSlnAndXprojRefThatRefsCsprojWhereSlnDoesNotRefCsproj"); } + [Fact] + public void WhenSolutionContainsACsprojFileItDoesNotTryToAddItAgain() + { + var projectDirectory = TestAssets + .Get("NonRestoredTestProjects", "PJAppWithSlnAndOneAlreadyMigratedCsproj") + .CreateInstance() + .WithSourceFiles() + .Root; + + var solutionRelPath = Path.Combine("TestApp", "TestApp.sln"); + var cmd = new DotnetCommand() + .WithWorkingDirectory(projectDirectory) + .ExecuteWithCapturedOutput($"migrate \"{solutionRelPath}\""); + cmd.Should().Pass(); + cmd.StdOut.Should().NotContain("already contains project"); + cmd.StdErr.Should().BeEmpty(); + } + private void MigrateAndBuild(string groupName, string projectName, [CallerMemberName] string callingMethod = "", string identifier = "") { var projectDirectory = TestAssets From 717d0a45fa722897f7a418a679c08a2e1df6fc8e Mon Sep 17 00:00:00 2001 From: Justin Goshi Date: Fri, 20 Jan 2017 12:21:04 -0800 Subject: [PATCH 2/2] Moving existing csproj files with same name as migration output to backup --- .../MigrationBackupPlan.cs | 22 ++++++++++-- .../ProjectMigrator.cs | 35 +++++++++++-------- .../commands/dotnet-migrate/MigrateCommand.cs | 17 +++++---- .../GivenThatIWantToMigrateSolutions.cs | 24 +++++++++++++ 4 files changed, 72 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/MigrationBackupPlan.cs b/src/Microsoft.DotNet.ProjectJsonMigration/MigrationBackupPlan.cs index c0423293f..43325ea77 100644 --- a/src/Microsoft.DotNet.ProjectJsonMigration/MigrationBackupPlan.cs +++ b/src/Microsoft.DotNet.ProjectJsonMigration/MigrationBackupPlan.cs @@ -11,6 +11,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration { internal class MigrationBackupPlan { + private const string TempCsprojExtention = ".migration_in_place_backup"; + private readonly FileInfo globalJson; public MigrationBackupPlan( @@ -57,7 +59,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration .Where(f => f.Name == "project.json" || f.Extension == ".xproj" || f.FullName.EndsWith(".xproj.user") - || f.FullName.EndsWith(".lock.json")); + || f.FullName.EndsWith(".lock.json") + || f.FullName.EndsWith(TempCsprojExtention)); } public DirectoryInfo ProjectBackupDirectory { get; } @@ -81,10 +84,23 @@ namespace Microsoft.DotNet.ProjectJsonMigration foreach (var file in FilesToMove) { + var fileName = file.Name.EndsWith(TempCsprojExtention) + ? Path.GetFileNameWithoutExtension(file.Name) + : file.Name; + file.MoveTo( - Path.Combine( - ProjectBackupDirectory.FullName, file.Name)); + Path.Combine(ProjectBackupDirectory.FullName, fileName)); } } + + public static void RenameCsprojFromMigrationOutputNameToTempName(string outputProject) + { + var backupFileName = $"{outputProject}{TempCsprojExtention}"; + if (File.Exists(backupFileName)) + { + File.Delete(backupFileName); + } + File.Move(outputProject, backupFileName); + } } } diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs b/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs index 3aaeebda5..2596d8f2f 100644 --- a/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs +++ b/src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs @@ -20,6 +20,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration { private readonly IMigrationRule _ruleSet; private readonly ProjectDependencyFinder _projectDependencyFinder = new ProjectDependencyFinder(); + private HashSet _migratedProjects = new HashSet(); public ProjectMigrator() : this(new DefaultMigrationRuleSet()) { } @@ -76,7 +77,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration var settings = new MigrationSettings(projectDir, projectDir, rootSettings.MSBuildProjectTemplatePath); - MigrateProject(settings); projectMigrationReports.Add(MigrateProject(settings)); } @@ -141,13 +141,28 @@ namespace Microsoft.DotNet.ProjectJsonMigration { var migrationRuleInputs = ComputeMigrationRuleInputs(migrationSettings); var projectName = migrationRuleInputs.DefaultProjectContext.GetProjectName(); + var outputProject = Path.Combine(migrationSettings.OutputDirectory, projectName + ".csproj"); try { - if (IsMigrated(migrationSettings, migrationRuleInputs)) + if (File.Exists(outputProject)) { - MigrationTrace.Instance.WriteLine(String.Format(LocalizableStrings.SkipMigrationAlreadyMigrated, nameof(ProjectMigrator), migrationSettings.ProjectDirectory)); - return new ProjectMigrationReport(migrationSettings.ProjectDirectory, projectName, skipped: true); + if (_migratedProjects.Contains(outputProject)) + { + MigrationTrace.Instance.WriteLine(String.Format( + LocalizableStrings.SkipMigrationAlreadyMigrated, + nameof(ProjectMigrator), + migrationSettings.ProjectDirectory)); + + return new ProjectMigrationReport( + migrationSettings.ProjectDirectory, + projectName, + skipped: true); + } + else + { + MigrationBackupPlan.RenameCsprojFromMigrationOutputNameToTempName(outputProject); + } } VerifyInputs(migrationRuleInputs, migrationSettings); @@ -185,7 +200,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration } } - var outputProject = Path.Combine(migrationSettings.OutputDirectory, projectName + ".csproj"); + _migratedProjects.Add(outputProject); + return new ProjectMigrationReport( migrationSettings.ProjectDirectory, projectName, @@ -286,14 +302,5 @@ namespace Microsoft.DotNet.ProjectJsonMigration File.Copy(sourceFilePath, destinationFilePath); } } - - public bool IsMigrated(MigrationSettings migrationSettings, MigrationRuleInputs migrationRuleInputs) - { - var outputName = migrationRuleInputs.DefaultProjectContext.GetProjectName(); - - var outputProject = Path.Combine(migrationSettings.OutputDirectory, outputName + ".csproj"); - return File.Exists(outputProject); - } - } } diff --git a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs index 9f6dc5444..f1b97846a 100644 --- a/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs +++ b/src/dotnet/commands/dotnet-migrate/MigrateCommand.cs @@ -119,17 +119,17 @@ namespace Microsoft.DotNet.Tools.Migrate foreach (var report in migrationReport.ProjectMigrationReports) { var reportPathWithTrailingSlash = PathUtility.EnsureTrailingSlash(report.ProjectDirectory); - var relReportPath = PathUtility.GetRelativePath( + var relativeReportPath = PathUtility.GetRelativePath( slnPathWithTrailingSlash, reportPathWithTrailingSlash); - var xprojPath = Path.Combine(relReportPath, report.ProjectName + ".xproj"); - var projects = _slnFile.Projects.Where(p => p.FilePath == xprojPath); + var xprojPath = Path.Combine(relativeReportPath, report.ProjectName + ".xproj"); + var xprojProjectsReferencedBySolution = _slnFile.Projects.Where(p => p.FilePath == xprojPath); var migratedProjectName = report.ProjectName + ".csproj"; - if (projects.Count() == 1) + if (xprojProjectsReferencedBySolution.Count() == 1) { - var slnProject = projects.Single(); + var slnProject = xprojProjectsReferencedBySolution.Single(); slnProject.FilePath = Path.Combine( Path.GetDirectoryName(slnProject.FilePath), migratedProjectName); @@ -137,12 +137,12 @@ namespace Microsoft.DotNet.Tools.Migrate } else { - var csprojPath = Path.Combine(relReportPath, migratedProjectName); - var slnAlreadyContainsMigratedCsproj = _slnFile.Projects + var csprojPath = Path.Combine(relativeReportPath, migratedProjectName); + var solutionContainsCsprojPriorToMigration = _slnFile.Projects .Where(p => p.FilePath == csprojPath) .Any(); - if (!slnAlreadyContainsMigratedCsproj) + if (!solutionContainsCsprojPriorToMigration) { csprojFilesToAdd.Add(Path.Combine(report.ProjectDirectory, migratedProjectName)); } @@ -299,7 +299,6 @@ namespace Microsoft.DotNet.Tools.Migrate if (projectMigrationReport.Errors.Any()) { - sb.AppendLine(RedIfColored($"Project {projectMigrationReport.ProjectName} migration failed ({projectMigrationReport.ProjectDirectory})")); foreach (var error in projectMigrationReport.Errors.Select(e => e.GetFormattedErrorMessage())) diff --git a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs index 9f43a25dc..cb4bbe619 100644 --- a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs +++ b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs @@ -67,6 +67,30 @@ namespace Microsoft.DotNet.Migration.Tests "PJAppWithSlnAndXprojRefThatRefsCsprojWhereSlnDoesNotRefCsproj"); } + [Fact] + public void WhenSolutionContainsACsprojFileItGetsMovedToBackup() + { + var projectDirectory = TestAssets + .Get("NonRestoredTestProjects", "PJAppWithSlnAndOneAlreadyMigratedCsproj") + .CreateInstance() + .WithSourceFiles() + .Root + .FullName; + + var solutionRelPath = Path.Combine("TestApp", "TestApp.sln"); + var cmd = new DotnetCommand() + .WithWorkingDirectory(projectDirectory) + .ExecuteWithCapturedOutput($"migrate \"{solutionRelPath}\""); + cmd.Should().Pass(); + + File.Exists(Path.Combine(projectDirectory, "TestLibrary", "TestLibrary.csproj")) + .Should().BeTrue(); + File.Exists(Path.Combine(projectDirectory, "TestLibrary", "TestLibrary.csproj.migration_in_place_backup")) + .Should().BeFalse(); + File.Exists(Path.Combine(projectDirectory, "backup", "TestLibrary", "TestLibrary.csproj")) + .Should().BeTrue(); + } + [Fact] public void WhenSolutionContainsACsprojFileItDoesNotTryToAddItAgain() {