Merge pull request #5393 from dotnet/dev/jgoshi/issue5343

Migration: do not add a csproj to the solution if it already exists

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;
+        }
+    }
+}

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

TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestApp/TestApp.xproj

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0.23107" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0.23107</VisualStudioVersion>
+    <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
+  </PropertyGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.Props" Condition="'$(VSToolsPath)' != ''" />
+  <PropertyGroup Label="Globals">
+    <ProjectGuid>0138cb8f-4aa9-4029-a21e-c07c30f425ba</ProjectGuid>
+    <RootNamespace>TestAppWithContents</RootNamespace>
+    <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
+    <OutputPath Condition="'$(OutputPath)'=='' ">..\..\..\artifacts\</OutputPath>
+  </PropertyGroup>
+  <PropertyGroup>
+    <SchemaVersion>2.0</SchemaVersion>
+  </PropertyGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" />
+</Project>

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": {}
+  }
+}

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!";
+        }
+    }
+}

TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.csproj

@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>netstandard1.5</TargetFramework>
+    <AssemblyName>TestLibrary</AssemblyName>
+    <PackageId>TestLibrary</PackageId>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="NETStandard.Library" Version="1.6.0" />
+  </ItemGroup>
+
+</Project>

TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/TestLibrary.xproj

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0.23107" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <PropertyGroup>
+    <VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0.23107</VisualStudioVersion>
+    <VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>
+  </PropertyGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.Props" Condition="'$(VSToolsPath)' != ''" />
+  <PropertyGroup Label="Globals">
+    <ProjectGuid>dc0b35d0-8a36-4b52-8a11-b86739f055d2</ProjectGuid>
+    <RootNamespace>TestAppWithContents</RootNamespace>
+    <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
+    <OutputPath Condition="'$(OutputPath)'=='' ">..\..\..\artifacts\</OutputPath>
+  </PropertyGroup>
+  <PropertyGroup>
+    <SchemaVersion>2.0</SchemaVersion>
+  </PropertyGroup>
+  <Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" />
+</Project>

TestAssets/NonRestoredTestProjects/PJAppWithSlnAndOneAlreadyMigratedCsproj/TestLibrary/project.json

@@ -0,0 +1,9 @@
+{
+  "version": "1.0.0-*",
+  "dependencies": {
+    "NETStandard.Library": "1.6.0"
+  },
+  "frameworks": {
+    "netstandard1.5": {}
+  }
+}

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(
+                    Path.Combine(ProjectBackupDirectory.FullName, fileName));
-                        ProjectBackupDirectory.FullName, file.Name));
             }
         }
+
+        public static void RenameCsprojFromMigrationOutputNameToTempName(string outputProject)
+        {
+            var backupFileName = $"{outputProject}{TempCsprojExtention}";
+            if (File.Exists(backupFileName))
+            {
+                File.Delete(backupFileName);
+            }
+            File.Move(outputProject, backupFileName);
+        }
     }
 }

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<string> _migratedProjects = new HashSet<string>();
 
         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));
+                    if (_migratedProjects.Contains(outputProject))
-                    return new ProjectMigrationReport(migrationSettings.ProjectDirectory, projectName, skipped: true);
+                    {
+                        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);
-        }
-
     }
 }

src/dotnet/commands/dotnet-migrate/MigrateCommand.cs

@@ -113,22 +113,23 @@ namespace Microsoft.DotNet.Tools.Migrate
                 return;
             }
 
-            List<string> csprojFilesToAdd = new List<string>();
+            var csprojFilesToAdd = new HashSet<string>();
 
             var slnPathWithTrailingSlash = PathUtility.EnsureTrailingSlash(_slnFile.BaseDirectory);
             foreach (var report in migrationReport.ProjectMigrationReports)
             {
                 var reportPathWithTrailingSlash = PathUtility.EnsureTrailingSlash(report.ProjectDirectory);
-                var reportRelPath = Path.Combine(
+                var relativeReportPath = PathUtility.GetRelativePath(
-                    PathUtility.GetRelativePath(slnPathWithTrailingSlash, reportPathWithTrailingSlash),
+                    slnPathWithTrailingSlash,
-                    report.ProjectName + ".xproj");
+                    reportPathWithTrailingSlash);
 
-                var projects = _slnFile.Projects.Where(p => p.FilePath == reportRelPath);
+                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);
@@ -136,7 +137,15 @@ namespace Microsoft.DotNet.Tools.Migrate
                 }
                 else
                 {
-                    csprojFilesToAdd.Add(Path.Combine(report.ProjectDirectory, migratedProjectName));
+                    var csprojPath = Path.Combine(relativeReportPath, migratedProjectName);
+                    var solutionContainsCsprojPriorToMigration = _slnFile.Projects
+                        .Where(p => p.FilePath == csprojPath)
+                        .Any();
+
+                    if (!solutionContainsCsprojPriorToMigration)
+                    {
+                        csprojFilesToAdd.Add(Path.Combine(report.ProjectDirectory, migratedProjectName));
+                    }
                 }
 
                 foreach (var preExisting in report.PreExistingCsprojDependencies)
@@ -292,7 +301,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()))

test/dotnet-migrate.Tests/GivenThatIWantToMigrateSolutions.cs

@@ -96,6 +96,48 @@ 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()
+        {
+            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