From 2878c7ffe8c3bd6d61fa5e3e66eb33a5d3d31d3d Mon Sep 17 00:00:00 2001
From: Ryan Nowak <nowakra@gmail.com>
Date: Sun, 19 Apr 2020 16:19:44 -0700
Subject: [PATCH] Address feedback about how we do evaluation

Fixes: #362

This change splits up our evaluation code-path into a few steps:

- Load project
- Do a restore
- Mark dirty
- Run targets and read properties

The key is that we need to do a restore separately from other targets
because restore will bring in nuget packages that contain targets. You
need to let MSBuild load those before continuing. The code here is
pretty similar to what dotnet does.

Additionally blocking the NBGV scenario, there are some additional
assemblies that need to be provided by MSBuild. NBGV also loads
Microsoft.DotNet.Platform.Abstractions which is part of the SDK. Since
we had an allow-list of just nuget assemblies that wasn't resolved.
---
 src/Microsoft.Tye.Core/ProjectReader.cs | 62 ++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 12 deletions(-)

diff --git a/src/Microsoft.Tye.Core/ProjectReader.cs b/src/Microsoft.Tye.Core/ProjectReader.cs
index 093bf3a3..538bf448 100644
--- a/src/Microsoft.Tye.Core/ProjectReader.cs
+++ b/src/Microsoft.Tye.Core/ProjectReader.cs
@@ -145,16 +145,24 @@ namespace Microsoft.Tye
         {
             var sw = Stopwatch.StartNew();
 
+            // Currently we only log at debug level.
+            var logger = new ConsoleLogger(
+                verbosity: LoggerVerbosity.Normal,
+                write: message => output.WriteDebug(message),
+                colorSet: null,
+                colorReset: null);
+
             // We need to isolate projects from each other for testing. MSBuild does not support
             // loading the same project twice in the same collection.
             var projectCollection = new ProjectCollection();
 
             ProjectInstance projectInstance;
+            Microsoft.Build.Evaluation.Project msbuildProject;
 
             try
             {
                 output.WriteDebugLine($"Loading project '{project.ProjectFile.FullName}'.");
-                var msbuildProject = Microsoft.Build.Evaluation.Project.FromFile(project.ProjectFile.FullName, new ProjectOptions()
+                msbuildProject = Microsoft.Build.Evaluation.Project.FromFile(project.ProjectFile.FullName, new ProjectOptions()
                 {
                     ProjectCollection = projectCollection,
                 });
@@ -166,18 +174,45 @@ namespace Microsoft.Tye
                 throw new CommandException($"Failed to load project: '{project.ProjectFile.FullName}'.", ex);
             }
 
-            // Currently we only log at debug level.
-            var logger = new ConsoleLogger(
-                verbosity: LoggerVerbosity.Normal,
-                write: message => output.WriteDebug(message),
-                colorSet: null,
-                colorReset: null);
-
             try
             {
                 AssemblyLoadContext.Default.Resolving += ResolveAssembly;
+
+                output.WriteDebugLine($"Restoring project '{project.ProjectFile.FullName}'.");
+
+                // Similar to what MSBuild does for restore:
+                // https://github.com/microsoft/msbuild/blob/3453beee039fb6f5ccc54ac783ebeced31fec472/src/MSBuild/XMake.cs#L1417
+                //
+                // We need to do restore as a separate operation
+                var restoreRequest = new BuildRequestData(
+                    projectInstance,
+                    targetsToBuild: new[] { "Restore" },
+                    hostServices: null,
+                    flags: BuildRequestDataFlags.ClearCachesAfterBuild | BuildRequestDataFlags.SkipNonexistentTargets | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports);
+
+                var parameters = new BuildParameters(projectCollection)
+                {
+                    Loggers = new[] { logger, },
+                };
+
+                // We don't really look at the result, because it's not clear we should halt totally
+                // if restore fails.
+                var restoreResult = BuildManager.DefaultBuildManager.Build(parameters, restoreRequest);
+                output.WriteDebugLine($"Restored project '{project.ProjectFile.FullName}'.");
+
+                msbuildProject.MarkDirty();
+                projectInstance = msbuildProject.CreateProjectInstance();
+
+                var targets = new List<string>()
+                {
+                    "ResolveReferences",
+                    "ResolvePackageDependenciesDesignTime",
+                    "PrepareResources",
+                    "GetAssemblyAttributes",
+                };
+
                 var result = projectInstance.Build(
-                    targets: new[] { "Restore", "ResolveReferences", "ResolvePackageDependenciesDesignTime", "PrepareResources", "GetAssemblyAttributes", },
+                    targets: targets.ToArray(),
                     loggers: new[] { logger, });
 
                 // If the build fails, we're not really blocked from doing our work.
@@ -189,8 +224,11 @@ namespace Microsoft.Tye
                 AssemblyLoadContext.Default.Resolving -= ResolveAssembly;
             }
 
-            // Reading both InformationalVersion and Version is more resilient in the face of build failures.
-            var version = projectInstance.GetProperty("InformationalVersion")?.EvaluatedValue ?? projectInstance.GetProperty("Version").EvaluatedValue;
+            // Reading a few different version properties to be more resilient.
+            var version =
+                projectInstance.GetProperty("AssemblyInformationalVersion")?.EvaluatedValue ??
+                projectInstance.GetProperty("InformationalVersion")?.EvaluatedValue ??
+                projectInstance.GetProperty("Version").EvaluatedValue;
             project.Version = version;
             output.WriteDebugLine($"Found application version: {version}");
 
@@ -254,7 +292,7 @@ namespace Microsoft.Tye
             // See: https://github.com/microsoft/MSBuildLocator/issues/86
             Assembly? ResolveAssembly(AssemblyLoadContext context, AssemblyName assemblyName)
             {
-                if (assemblyName.Name is object && assemblyName.Name.StartsWith("NuGet."))
+                if (assemblyName.Name is object)
                 {
                     var msbuildDirectory = Environment.GetEnvironmentVariable("MSBuildExtensionsPath")!;
                     var assemblyFilePath = Path.Combine(msbuildDirectory, assemblyName.Name + ".dll");