Skip to content

Commit

Permalink
Address feedback about how we do evaluation
Browse files Browse the repository at this point in the history
Fixes: dotnet#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.
  • Loading branch information
rynowak authored and kishanAnem committed May 15, 2020
1 parent 9553bd4 commit 63b4d1a
Showing 1 changed file with 50 additions and 12 deletions.
62 changes: 50 additions & 12 deletions src/Microsoft.Tye.Core/ProjectReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,24 @@ private static void EvaluateProject(OutputContext output, ProjectServiceBuilder
{
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,
});
Expand All @@ -166,18 +174,45 @@ private static void EvaluateProject(OutputContext output, ProjectServiceBuilder
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.
Expand All @@ -189,8 +224,11 @@ private static void EvaluateProject(OutputContext output, ProjectServiceBuilder
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}");

Expand Down Expand Up @@ -254,7 +292,7 @@ bool PropertyIsTrue(string property)
// 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");
Expand Down

0 comments on commit 63b4d1a

Please sign in to comment.