Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -576,5 +576,112 @@ private void TestSkipIsolationConstraints(string glob, string referencePath, boo

configuration.ShouldSkipIsolationConstraintsForReference(referencePath).ShouldBe(expectedOutput);
}

[Fact]
public void TestProjectEvaluationIdPreservedAcrossTranslation()
{
string projectBody = """
<Project ToolsVersion='msbuilddefaulttoolsversion' xmlns='msbuildnamespace'>
<Target Name='Build' />
</Project>
""".Cleanup();

using var collection = new ProjectCollection();
using ProjectFromString projectFromString = new(
projectBody,
new Dictionary<string, string>(),
ObjectModelHelpers.MSBuildDefaultToolsVersion,
collection);
Project project = projectFromString.Project;
project.FullPath = "foo";
ProjectInstance instance = project.CreateProjectInstance();

BuildRequestConfiguration configuration = new(
new BuildRequestData(instance, [], null, BuildRequestDataFlags.None, propertiesToTransfer: []), "2.0")
{
ConfigurationId = 1,
};

// The evaluation ID should be set from the project instance.
int expectedEvalId = instance.EvaluationId;
configuration.ProjectEvaluationId.ShouldBe(expectedEvalId);
expectedEvalId.ShouldNotBe(BuildEventContext.InvalidEvaluationId);

((ITranslatable)configuration).Translate(TranslationHelpers.GetWriteTranslator());
INodePacket packet = BuildRequestConfiguration.FactoryForDeserialization(TranslationHelpers.GetReadTranslator());

BuildRequestConfiguration deserializedConfig = packet as BuildRequestConfiguration;
deserializedConfig.ShouldNotBeNull();
deserializedConfig.ProjectEvaluationId.ShouldBe(expectedEvalId);
}

[Fact]
public void TestProjectEvaluationIdPreservedInShallowClone()
{
string projectBody = """
<Project ToolsVersion='msbuilddefaulttoolsversion' xmlns='msbuildnamespace'>
<Target Name='Build' />
</Project>
""".Cleanup();

using var collection = new ProjectCollection();
using ProjectFromString projectFromString = new(
projectBody,
new Dictionary<string, string>(),
ObjectModelHelpers.MSBuildDefaultToolsVersion,
collection);
Project project = projectFromString.Project;
project.FullPath = "foo";
ProjectInstance instance = project.CreateProjectInstance();

BuildRequestConfiguration original = new(new BuildRequestData(instance, [], null), "2.0")
{
ConfigurationId = 1,
};

int expectedEvalId = instance.EvaluationId;
original.ProjectEvaluationId.ShouldBe(expectedEvalId);

BuildRequestConfiguration clone = original.ShallowCloneWithNewId(2);
clone.ProjectEvaluationId.ShouldBe(expectedEvalId);
}


[Fact]
public void TestProjectEvaluationIdPreservedAcrossTranslateForFutureUse()
{
string projectBody = """
<Project ToolsVersion='msbuilddefaulttoolsversion' xmlns='msbuildnamespace'>
<Target Name='Build' />
</Project>
""".Cleanup();

using var collection = new ProjectCollection();
using ProjectFromString projectFromString = new(
projectBody,
new Dictionary<string, string>(),
ObjectModelHelpers.MSBuildDefaultToolsVersion,
collection);
Project project = projectFromString.Project;
project.FullPath = "foo";
ProjectInstance instance = project.CreateProjectInstance();

BuildRequestConfiguration configuration = new(new BuildRequestData(instance, [], null), "2.0")
{
ConfigurationId = 1,
};

int expectedEvalId = instance.EvaluationId;
configuration.ProjectEvaluationId.ShouldBe(expectedEvalId);

// TranslateForFutureUse uses a different serialization path.
configuration.TranslateForFutureUse(TranslationHelpers.GetWriteTranslator());
ITranslator reader = TranslationHelpers.GetReadTranslator();

BuildRequestConfiguration deserialized = new();
deserialized.TranslateForFutureUse(reader);

deserialized.ProjectEvaluationId.ShouldBe(expectedEvalId);
}
}
}
17 changes: 17 additions & 0 deletions src/Build.UnitTests/BackEnd/BuildResult_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.Build.Execution;
using Microsoft.Build.Framework;
using Microsoft.Build.Unittest;
using Shouldly;
using Xunit;
using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;

Expand Down Expand Up @@ -343,6 +344,22 @@ public void TestTranslation()
Assert.True(TranslationHelpers.CompareCollections(result["omega"].Items, deserializedResult["omega"].Items, TaskItemComparer.Instance));
}

[Fact]
public void TestTranslationPreservesEvaluationId()
{
BuildRequest request = new(1, 1, 2, ["Build"], null, new BuildEventContext(1, 1, 2, 3, 4, 5), null);
BuildResult result = new(request, new BuildAbortedException())
{
EvaluationId = 42,
};

((ITranslatable)result).Translate(TranslationHelpers.GetWriteTranslator());
INodePacket packet = BuildResult.FactoryForDeserialization(TranslationHelpers.GetReadTranslator());
BuildResult deserializedResult = (packet as BuildResult)!;

deserializedResult.EvaluationId.ShouldBe(42);
}

private BuildRequest CreateNewBuildRequest(int configurationId, string[] targets)
{
return new BuildRequest(1 /* submissionId */, _nodeRequestId++, configurationId, targets, null, BuildEventContext.Invalid, null);
Expand Down
41 changes: 33 additions & 8 deletions src/Build.UnitTests/TerminalLogger_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal sealed class MockBuildEventSink(int nodeNumber) : IBuildEventSink, IEve
public bool HaveLoggedBuildFinishedEvent { get; set; }

void IBuildEventSink.Consume(BuildEventArgs buildEvent, int sinkId) => (this as IBuildEventSink).Consume(buildEvent);

void IBuildEventSink.Consume(BuildEventArgs buildEvent)
{
// map the incoming build event to the appropriate event handler
Expand Down Expand Up @@ -169,7 +169,7 @@ public TerminalLogger_Tests(ITestOutputHelper outputHelper)
{
_outputHelper = outputHelper;
_mockTerminal = new Terminal(_outputWriter);

_terminallogger = new TerminalLogger(_mockTerminal);
_terminallogger.Initialize(_centralNodeEventSource, _nodeCount);
_terminallogger._createStopwatch = () => new MockStopwatch();
Expand Down Expand Up @@ -916,11 +916,11 @@ public void TestTerminalLoggerTogetherWithOtherLoggers()
string logFileWithoutTL = env.ExpectFile(".binlog").Path;

// Execute MSBuild with binary, file and terminal loggers
RunnerUtilities.ExecMSBuild($"{projectFile.Path} /bl:{logFileWithTL} -flp:logfile={Path.Combine(logFolder.Path, "logFileWithTL.log")};verbosity=diagnostic -tl:on", out bool success, outputHelper: _outputHelper);
RunnerUtilities.ExecMSBuild($"{projectFile.Path} /bl:{logFileWithTL} -flp:logfile={Path.Combine(logFolder.Path, "logFileWithTL.log")};verbosity=diagnostic -tl:on", out bool success, outputHelper: _outputHelper);
success.ShouldBeTrue();

// Execute MSBuild with binary and file loggers
RunnerUtilities.ExecMSBuild($"{projectFile.Path} /bl:{logFileWithoutTL} -flp:logfile={Path.Combine(logFolder.Path, "logFileWithoutTL.log")};verbosity=diagnostic", out success, outputHelper: _outputHelper);
RunnerUtilities.ExecMSBuild($"{projectFile.Path} /bl:{logFileWithoutTL} -flp:logfile={Path.Combine(logFolder.Path, "logFileWithoutTL.log")};verbosity=diagnostic", out success, outputHelper: _outputHelper);
success.ShouldBeTrue();

// Read the binary log and replay into mockLogger
Expand Down Expand Up @@ -1029,29 +1029,29 @@ public void ReplayBinaryLogWithFewerNodesThanOriginalBuild()
{
// Create multiple projects that will build in parallel
TransientTestFolder logFolder = env.CreateFolder(createFolder: true);

// Create three simple projects
TransientTestFile project1 = env.CreateFile(logFolder, "project1.proj", @"
<Project>
<Target Name='Build'>
<Message Text='Building project1' Importance='High' />
</Target>
</Project>");

TransientTestFile project2 = env.CreateFile(logFolder, "project2.proj", @"
<Project>
<Target Name='Build'>
<Message Text='Building project2' Importance='High' />
</Target>
</Project>");

TransientTestFile project3 = env.CreateFile(logFolder, "project3.proj", @"
<Project>
<Target Name='Build'>
<Message Text='Building project3' Importance='High' />
</Target>
</Project>");

// Create a solution file that builds all projects in parallel
string solutionContents = $@"
<Project>
Expand Down Expand Up @@ -1136,5 +1136,30 @@ public async Task DisplayNodesRestoresStatusAfterMSBuildTaskYields_TestProject(b

await Verify(_outputWriter.ToString(), _settings).UniqueForOSPlatform().UseParameters(runOnCentralNode);
}

[Fact]
public void MetaprojProjectStartedDoesNotCrash()
{
#if DEBUG
// Metaproj files (generated for solution multi-targeting builds) are never evaluated,
// so they have no matching ProjectEvaluationFinished event. TerminalLogger should
// handle ProjectStarted for metaproj files without hitting the Debug.Assert that
// checks for prior evaluation info. In Release mode this test is a no-op because
// Debug.Assert is compiled out.
string metaprojFile = NativeMethods.IsUnixLike ? "/src/solution.sln.metaproj" : @"C:\src\solution.sln.metaproj";

BuildEventContext buildContext = MakeBuildEventContext(evalId: -1, projectContextId: 10);

_centralNodeEventSource.InvokeBuildStarted(MakeBuildStartedEventArgs());

Should.NotThrow(() =>
{
_centralNodeEventSource.InvokeProjectStarted(MakeProjectStartedEventArgs(metaprojFile, "Build", buildEventContext: buildContext));
_centralNodeEventSource.InvokeProjectFinished(MakeProjectFinishedEventArgs(metaprojFile, true, buildEventContext: buildContext));
});

_centralNodeEventSource.InvokeBuildFinished(MakeBuildFinishedEventArgs(true));
#endif
}
}
}
7 changes: 7 additions & 0 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2689,6 +2689,13 @@ private void HandleResult(int node, BuildResult result)
configuration.ProjectTargets ??= result.ProjectTargets;
}

// Update the evaluation ID if it's valid - this propagates the eval ID
// from worker nodes to the central node for cached result scenarios.
if (result.EvaluationId != BuildEventContext.InvalidEvaluationId)
{
configuration.ProjectEvaluationId = result.EvaluationId;
}

// Only report results to the project cache services if it's the result for a build submission.
// Note that graph builds create a submission for each node in the graph, so each node in the graph will be
// handled here. This intentionally mirrors the behavior for cache requests, as it doesn't make sense to
Expand Down
6 changes: 2 additions & 4 deletions src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,8 @@ internal ProjectLoggingContext LogProjectStarted(BuildRequest request, BuildRequ
{
ErrorUtilities.VerifyThrow(this.IsValid, "Build not started.");

// If we can retrieve the evaluationId from the project, do so. Don't if it's not available or
// if we'd have to retrieve it from the cache in order to access it.
// Order is important here because the Project getter will throw if IsCached.
int evaluationId = (configuration != null && !configuration.IsCached && configuration.Project != null) ? configuration.Project.EvaluationId : BuildEventContext.InvalidEvaluationId;
// Use the persisted ProjectEvaluationId which remains available even when the project is cached.
int evaluationId = configuration?.ProjectEvaluationId ?? BuildEventContext.InvalidEvaluationId;

return new ProjectLoggingContext(this, request, configuration.ProjectFullPath, configuration.ToolsVersion, evaluationId);
}
Expand Down
14 changes: 10 additions & 4 deletions src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,8 @@ private async Task RequestThreadProc(bool setThreadParameters)
{
ErrorUtilities.VerifyThrow(result == null, "Result already set when exception was thrown.");
result = new BuildResult(_requestEntry.Request, thrownException);
// Populate the evaluation ID from the configuration for sending to the central node.
result.EvaluationId = _requestEntry.RequestConfiguration.ProjectEvaluationId;
}

ReportResultAndCleanUp(result);
Expand Down Expand Up @@ -1021,7 +1023,10 @@ private BuildResult[] GetResultsForContinuation(FullyQualifiedBuildRequest[] req
results = new Dictionary<int, BuildResult>();
for (int i = 0; i < requests.Length; i++)
{
results[i] = new BuildResult(new BuildRequest(), new BuildAbortedException());
var abortResult = new BuildResult(new BuildRequest(), new BuildAbortedException());
// Populate the evaluation ID from the configuration for sending to the central node.
abortResult.EvaluationId = _requestEntry.RequestConfiguration.ProjectEvaluationId;
results[i] = abortResult;
}
}

Expand Down Expand Up @@ -1244,6 +1249,9 @@ private async Task<BuildResult> BuildProject()
BuildResult result = await _targetBuilder.BuildTargets(_projectLoggingContext, _requestEntry, this,
allTargets, _requestEntry.RequestConfiguration.BaseLookup, _cancellationTokenSource.Token);

// Populate the evaluation ID from the configuration for sending to the central node.
result.EvaluationId = _requestEntry.RequestConfiguration.ProjectEvaluationId;

UpdateStatisticsPostBuild();

result = _requestEntry.Request.ProxyTargets == null
Expand Down Expand Up @@ -1334,7 +1342,7 @@ private void UpdateStatisticsPostBuild()

bool isFromNuget, isMetaprojTarget, isCustom;

if (IsMetaprojTargetPath(projectTargetInstance.Value.FullPath))
if (FileUtilities.IsMetaprojectFilename(projectTargetInstance.Value.FullPath))
{
isMetaprojTarget = true;
isFromNuget = false;
Expand Down Expand Up @@ -1388,8 +1396,6 @@ void CollectTasksStats(TaskRegistry taskRegistry)
}
}

private static bool IsMetaprojTargetPath(string targetPath) => targetPath.EndsWith(".metaproj", StringComparison.OrdinalIgnoreCase);

/// <summary>
/// Saves the current operating environment (working directory and environment variables)
/// from the request's <see cref="TaskEnvironment"/> to the configuration for later restoration.
Expand Down
20 changes: 20 additions & 0 deletions src/Build/BackEnd/Shared/BuildRequestConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ internal class BuildRequestConfiguration : IEquatable<BuildRequestConfiguration>
/// </summary>
private string _savedCurrentDirectory;

/// <summary>
/// Saves the evaluation ID for the project so that it's accessible even when the underlying Project becomes cached.
/// </summary>
private int _projectEvaluationId = BuildEventContext.InvalidEvaluationId;

#endregion

/// <summary>
Expand Down Expand Up @@ -186,6 +191,7 @@ internal BuildRequestConfiguration(int configId, BuildRequestData data, string d
_projectInitialTargets = data.ProjectInstance.InitialTargets;
_projectDefaultTargets = data.ProjectInstance.DefaultTargets;
_projectTargets = GetProjectTargets(data.ProjectInstance.Targets);
_projectEvaluationId = data.ProjectInstance.EvaluationId;
if (data.PropertiesToTransfer != null)
{
_transferredProperties = new List<ProjectPropertyInstance>();
Expand Down Expand Up @@ -223,6 +229,7 @@ internal BuildRequestConfiguration(int configId, ProjectInstance instance)
_projectInitialTargets = instance.InitialTargets;
_projectDefaultTargets = instance.DefaultTargets;
_projectTargets = GetProjectTargets(instance.Targets);
_projectEvaluationId = instance.EvaluationId;
IsCacheable = false;
}

Expand All @@ -247,6 +254,7 @@ private BuildRequestConfiguration(int configId, BuildRequestConfiguration other)
IsCacheable = other.IsCacheable;
_configId = configId;
RequestedTargets = other.RequestedTargets;
_projectEvaluationId = other._projectEvaluationId;
}

/// <summary>
Expand Down Expand Up @@ -289,6 +297,15 @@ internal BuildRequestConfiguration()
/// </summary>
public bool IsCached { get; private set; }

/// <summary>
/// The evaluation ID for this project, persisted so it remains available even when the project is cached.
/// </summary>
public int ProjectEvaluationId
{
get => _projectEvaluationId;
internal set => _projectEvaluationId = value;
}

/// <summary>
/// Flag indicating if this configuration represents a traversal project. Traversal projects
/// are projects which typically do little or no work themselves, but have references to other
Expand Down Expand Up @@ -424,6 +441,7 @@ private void SetProjectBasedState(ProjectInstance project)
_projectInitialTargets = null;
_projectTargets = null;

_projectEvaluationId = _project.EvaluationId;
ProjectDefaultTargets = _project.DefaultTargets;
ProjectInitialTargets = _project.InitialTargets;
ProjectTargets = GetProjectTargets(_project.Targets);
Expand Down Expand Up @@ -941,6 +959,7 @@ public void Translate(ITranslator translator)
translator.Translate(ref _resultsNodeId);
translator.Translate(ref _savedCurrentDirectory);
translator.TranslateDictionary(ref _savedEnvironmentVariables, CommunicationsUtilities.EnvironmentVariableComparer);
translator.Translate(ref _projectEvaluationId);

// if the entire state is translated, then the transferred state represents the full evaluation data
if (translator.Mode == TranslationDirection.ReadFromStream && _transferredState?.TranslateEntireState == true)
Expand All @@ -959,6 +978,7 @@ internal void TranslateForFutureUse(ITranslator translator)
translator.Translate(ref _projectInitialTargets);
translator.Translate(ref _projectTargets);
translator.TranslateDictionary(ref _globalProperties, ProjectPropertyInstance.FactoryForDeserialization);
translator.Translate(ref _projectEvaluationId);
}

/// <summary>
Expand Down
Loading
Loading