Merge pull request #5226 from livarcocc/migrate_full_framework

Migration will now pick win7-x86 as the RuntimeIdentifier to be used by Full Framework TFMs

TestAssets/TestProjects/TestAppMultipleFrameworksNoRuntimes/Program.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Xml;
+
+namespace ConsoleApplication
+{
+    public class Program
+    {
+        public static void Main()
+        {
+            Console.WriteLine("Hello World!");
+#if NET20 || NET35 || NET45 || NET461
+            // Force XmlDocument to be used
+            var doc = new XmlDocument();
+#endif
+        }
+    }
+}

TestAssets/TestProjects/TestAppMultipleFrameworksNoRuntimes/project.json

@@ -0,0 +1,22 @@
+{
+  "version": "1.0.0-*",
+  "buildOptions": {
+    "emitEntryPoint": true
+  },
+  "dependencies": {},
+  "frameworks": {
+    "net461": {
+      "frameworkAssemblies": {
+        "System.Xml": {}
+      }
+    },
+    "netcoreapp1.0": {
+      "dependencies": {
+        "Microsoft.NetCore.App": {
+          "version": "1.0.3",
+          "type": "platform"
+        }
+      }
+    }
+  }
+}

TestAssets/TestProjects/TestAppWithMultipleFrameworksAndRuntimes/project.json

@@ -27,7 +27,7 @@
     },
     "netcoreapp1.0": {
       "dependencies": {
-        "Microsoft.NetCore.App": "1.0.1"
+        "Microsoft.NetCore.App": "1.0.3"
       }
     }
   },

TestAssets/TestProjects/TestAppWithMultipleFullFrameworksOnly/Program.cs

@@ -0,0 +1,17 @@
+using System;
+using System.Xml;
+
+namespace ConsoleApplication
+{
+    public class Program
+    {
+        public static void Main()
+        {
+            Console.WriteLine("Hello World!");
+#if NET20 || NET35 || NET45 || NET461
+            // Force XmlDocument to be used
+            var doc = new XmlDocument();
+#endif
+        }
+    }
+}

TestAssets/TestProjects/TestAppWithMultipleFullFrameworksOnly/project.json

@@ -0,0 +1,29 @@
+{
+  "version": "1.0.0-*",
+  "buildOptions": {
+    "emitEntryPoint": true
+  },
+  "dependencies": {},
+  "frameworks": {
+    "net20": {
+      "frameworkAssemblies": {
+        "System.Xml": {}
+      }
+    },
+    "net35": {
+      "frameworkAssemblies": {
+        "System.Xml": {}
+      }
+    },
+    "net40": {
+      "frameworkAssemblies": {
+        "System.Xml": {}
+      }
+    },
+    "net461": {
+      "frameworkAssemblies": {
+        "System.Xml": {}
+      }
+    }
+  }
+}

src/Microsoft.DotNet.ProjectJsonMigration/ProjectContextExtensions.cs

@@ -17,5 +17,30 @@ namespace Microsoft.DotNet.ProjectJsonMigration
             // _ here is just an arbitrary configuration value so we can obtain the output name
             return Path.GetFileNameWithoutExtension(projectContext.GetOutputPaths("_").CompilationFiles.Assembly);
         }
+
+        public static bool HasRuntimes(this IEnumerable<ProjectContext> projectContexts)
+        {
+            return projectContexts.Any(p => p.ProjectFile.Runtimes.Any());
+        }
+
+        public static bool HasBothCoreAndFullFrameworkTFMs(this IEnumerable<ProjectContext> projectContexts)
+        {
+            return projectContexts.HasCoreTFM() && projectContexts.HasFullFrameworkTFM();
+        }
+
+        public static bool HasCoreTFM(this IEnumerable<ProjectContext> projectContexts)
+        {
+            return projectContexts.Any(p => !p.IsFullFramework());
+        }
+
+        public static bool HasFullFrameworkTFM(this IEnumerable<ProjectContext> projectContexts)
+        {
+            return projectContexts.Any(p => p.IsFullFramework());
+        }
+
+        public static bool IsFullFramework(this ProjectContext projectContext)
+        {
+            return !projectContext.TargetFramework.IsPackageBased;
+        }
     }
 }

src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateTFMRule.cs

@@ -41,16 +41,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                         migrationRuleInputs.ProjectContexts.Select(p => p.TargetFramework)),
                     propertyGroup,
                     mergeExisting: true);
-
-
-                var runtimes = string.Join(",", migrationRuleInputs.ProjectContexts.Select(p => p.RuntimeIdentifier));
-                Console.WriteLine($"Runtimes = {runtimes}");
-
-                _transformApplicator.Execute(
-                    FrameworksRuntimeIdentifiersTransform.Transform(
-                        migrationRuleInputs.ProjectContexts),
-                    propertyGroup,
-                    mergeExisting: true);
             }
             else
             {
@@ -59,12 +49,16 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                         migrationRuleInputs.ProjectContexts.Single().TargetFramework),
                     propertyGroup,
                     mergeExisting: true);
-                _transformApplicator.Execute(
+            }
-                    FrameworkRuntimeIdentifiersTransform.Transform(
+
-                        migrationRuleInputs.ProjectContexts.Single()),
+            _transformApplicator.Execute(
+                    RuntimeIdentifiersTransform.Transform(migrationRuleInputs.ProjectContexts),
+                    propertyGroup,
+                    mergeExisting: true);
+            _transformApplicator.Execute(
+                    RuntimeIdentifierTransform.Transform(migrationRuleInputs.ProjectContexts),
                     propertyGroup,
                     mergeExisting: true);
-            }
         }
 
         private void CleanExistingProperties(ProjectRootElement csproj)
@@ -128,24 +122,38 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                 frameworks => string.Join(";", frameworks.Select(f => f.GetShortFolderName())),
                 frameworks => true);
 
-        private AddPropertyTransform<IEnumerable<ProjectContext>> FrameworksRuntimeIdentifiersTransform =>
-            new AddPropertyTransform<IEnumerable<ProjectContext>>(
-                "RuntimeIdentifiers",
-                projectContexts => RuntimeIdentifiers,
-                projectContexts => projectContexts.All(p => !p.ProjectFile.Runtimes.Any()) &&
-                                   projectContexts.Any(p => !p.TargetFramework.IsPackageBased));
-
         private AddPropertyTransform<NuGetFramework> FrameworkTransform =>
             new AddPropertyTransform<NuGetFramework>(
                 "TargetFramework",
                 framework => framework.GetShortFolderName(),
                 framework => true);
 
-        private AddPropertyTransform<ProjectContext> FrameworkRuntimeIdentifiersTransform =>
+        private AddPropertyTransform<IEnumerable<ProjectContext>> RuntimeIdentifiersTransform =>
-            new AddPropertyTransform<ProjectContext>(
+            new AddPropertyTransform<IEnumerable<ProjectContext>>(
                 "RuntimeIdentifiers",
-                projectContext => RuntimeIdentifiers,
+                projectContexts => RuntimeIdentifiers,
-                projectContext => !projectContext.ProjectFile.Runtimes.Any() &&
+                projectContexts => !projectContexts.HasRuntimes() &&
-                                  !projectContext.TargetFramework.IsPackageBased);
+                                    projectContexts.HasBothCoreAndFullFrameworkTFMs());
+
+        private AddPropertyTransform<IEnumerable<ProjectContext>> RuntimeIdentifierTransform =>
+            new AddPropertyTransform<IEnumerable<ProjectContext>>(
+                "RuntimeIdentifier",
+                projectContexts => "win7-x86",
+                projectContexts => !projectContexts.HasRuntimes() && projectContexts.HasFullFrameworkTFM())
+            .WithMSBuildCondition(projectContexts =>
+                {
+                    string msBuildCondition = null;
+                    if (projectContexts.HasBothCoreAndFullFrameworkTFMs())
+                    {
+                        msBuildCondition = string.Join(
+                            " OR ",
+                            projectContexts.Where(p => p.IsFullFramework()).Select(
+                                p => $"'$(TargetFramework)' == '{p.TargetFramework.GetShortFolderName()}'"));
+
+                        msBuildCondition = $" {msBuildCondition} ";
+                    }
+
+                    return msBuildCondition;
+                });
     }
 }

src/Microsoft.DotNet.ProjectJsonMigration/transforms/AddPropertyTransform.cs

@@ -14,7 +14,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
         private readonly string _propertyValue;
         private readonly Func<T,string> _propertyValueFunc;
 
-        private string _msbuildCondition = null;
+        private Func<T, string> _msbuildConditionFunc = null;
 
         public AddPropertyTransform(string propertyName, string propertyValue, Func<T,bool> condition)
             : base(condition)
@@ -32,7 +32,13 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
 
         public AddPropertyTransform<T> WithMSBuildCondition(string condition)
         {
-            _msbuildCondition = condition;
+            _msbuildConditionFunc = source => condition;
+            return this;
+        }
+
+        public AddPropertyTransform<T> WithMSBuildCondition(Func<T, string> conditionFunc)
+        {
+            _msbuildConditionFunc = conditionFunc;
             return this;
         }
 
@@ -43,9 +49,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Transforms
             var property = _propertyObjectGenerator.CreatePropertyElement(PropertyName);
             property.Value = propertyValue;
             
-            if (!string.IsNullOrEmpty(_msbuildCondition))
+            if (_msbuildConditionFunc != null)
             {
-                property.Condition = _msbuildCondition;
+                property.Condition = _msbuildConditionFunc(source);
             }
             
             return property;

test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateTFMs.cs

@@ -69,7 +69,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void MigratingDesktopTFMsAddsAllRuntimeIdentifiersIfTheProjectDoesNothaveAnyAlready()
+        public void MigratingCoreAndDesktopTFMsAddsAllRuntimeIdentifiersIfTheProjectDoesNothaveAnyAlready()
         {
             var testDirectory = Temp.CreateDirectory().Path;
             var testPJ = new ProjectJsonBuilder(TestAssetsManager)
@@ -93,6 +93,32 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
                 .Value.Should().Be("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");
         }
 
+        [Fact]
+        public void MigratingCoreAndDesktopTFMsAddsRuntimeIdentifierWithWin7x86ConditionOnAllFullFrameworksWhenNoRuntimesExistAlready()
+        {
+            var testDirectory = Temp.CreateDirectory().Path;
+            var testPJ = new ProjectJsonBuilder(TestAssetsManager)
+                .FromTestAssetBase("TestLibraryWithMultipleFrameworks")
+                .SaveToDisk(testDirectory);
+
+            var projectContexts = ProjectContext.CreateContextForEachFramework(testDirectory);
+            var mockProj = ProjectRootElement.Create();
+
+            var migrationSettings = MigrationSettings.CreateMigrationSettingsTestHook(testDirectory, testDirectory, mockProj);
+            var migrationInputs = new MigrationRuleInputs(
+                projectContexts, 
+                mockProj, 
+                mockProj.AddItemGroup(), 
+                mockProj.AddPropertyGroup());
+
+            new MigrateTFMRule().Apply(migrationSettings, migrationInputs);
+
+            mockProj.Properties.Count(p => p.Name == "RuntimeIdentifier").Should().Be(1);
+            var runtimeIdentifier = mockProj.Properties.First(p => p.Name == "RuntimeIdentifier");
+            runtimeIdentifier.Value.Should().Be("win7-x86");
+            runtimeIdentifier.Condition.Should().Be(" '$(TargetFramework)' == 'net20' OR '$(TargetFramework)' == 'net35' OR '$(TargetFramework)' == 'net40' OR '$(TargetFramework)' == 'net461' ");
+        }
+
         [Fact]
         public void MigrateTFMRuleDoesNotAddRuntimesWhenMigratingDesktopTFMsWithRuntimesAlready()
         {
@@ -117,6 +143,32 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
             mockProj.Properties.Count(p => p.Name == "RuntimeIdentifiers").Should().Be(0);
         }
 
+        [Fact]
+        public void MigratingProjectWithFullFrameworkTFMsOnlyAddsARuntimeIdentifierWin7x86WhenNoRuntimesExistAlready()
+        {
+            var testDirectory = Temp.CreateDirectory().Path;
+            var testPJ = new ProjectJsonBuilder(TestAssetsManager)
+                .FromTestAssetBase("TestAppWithMultipleFullFrameworksOnly")
+                .SaveToDisk(testDirectory);
+
+            var projectContexts = ProjectContext.CreateContextForEachFramework(testDirectory);
+            var mockProj = ProjectRootElement.Create();
+
+            var migrationSettings =
+                MigrationSettings.CreateMigrationSettingsTestHook(testDirectory, testDirectory, mockProj);
+            var migrationInputs = new MigrationRuleInputs(
+                projectContexts, 
+                mockProj, 
+                mockProj.AddItemGroup(), 
+                mockProj.AddPropertyGroup());
+
+            new MigrateTFMRule().Apply(migrationSettings, migrationInputs);
+
+            mockProj.Properties.Count(p => p.Name == "RuntimeIdentifiers").Should().Be(0);
+            mockProj.Properties.Where(p => p.Name == "RuntimeIdentifier").Should().HaveCount(1);
+            mockProj.Properties.Single(p => p.Name == "RuntimeIdentifier").Value.Should().Be("win7-x86");
+        }
+
         [Fact]
         public void MigratingSingleTFMProjectPopulatesTargetFramework()
         {

test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/BuildCommand.cs

@@ -14,7 +14,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities
 
         private string _configuration;
 
-        private NuGetFramework _framework;
+        private string _framework;
 
         private string _runtime;
 
@@ -72,6 +72,13 @@ namespace Microsoft.DotNet.Tools.Test.Utilities
         }
 
         public BuildCommand WithFramework(NuGetFramework framework)
+        {
+            _framework = framework.GetShortFolderName();
+
+            return this;
+        }
+
+        public BuildCommand WithFramework(string framework)
         {
             _framework = framework;
 
@@ -137,7 +144,7 @@ namespace Microsoft.DotNet.Tools.Test.Utilities
                 return null;
             }
 
-            return $"--framework {_framework.GetShortFolderName()}";
+            return $"--framework {_framework}";
         }
 
         private string GetRuntime()

test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs

@@ -53,6 +53,24 @@ namespace Microsoft.DotNet.Migration.Tests
             csproj.EndsWith("\n").Should().Be(true);
         }
 
+        [WindowsOnlyTheory]
+        [InlineData("TestAppMultipleFrameworksNoRuntimes", null)]
+        [InlineData("TestAppWithMultipleFullFrameworksOnly", "net461")]
+        public void ItMigratesAppsWithFullFramework(string projectName, string framework)
+        {
+            var projectDirectory = TestAssetsManager.CreateTestInstance(
+                projectName,
+                identifier: projectName).WithLockFiles().Path;
+
+            CleanBinObj(projectDirectory);
+
+            MigrateProject(new [] { projectDirectory });
+
+            Restore(projectDirectory);
+
+            BuildMSBuild(projectDirectory, projectName, framework: framework);
+        }
+
         [Fact]
         public void ItMigratesSignedApps()
         {
@@ -695,7 +713,8 @@ namespace Microsoft.DotNet.Migration.Tests
             string projectDirectory,
             string projectName,
             string configuration="Debug",
-            string runtime=null)
+            string runtime=null,
+            string framework=null)
         {
             if (projectName != null && !Path.HasExtension(projectName))
             {
@@ -707,6 +726,7 @@ namespace Microsoft.DotNet.Migration.Tests
             var result = new BuildCommand()
                 .WithWorkingDirectory(projectDirectory)
                 .WithRuntime(runtime)
+                .WithFramework(framework)
                 .ExecuteWithCapturedOutput($"{projectName} /p:Configuration={configuration}");
 
             result