Skip to content

Commit

Permalink
Live logger properly report errors during restore. (#8707)
Browse files Browse the repository at this point in the history 
Fixes #8704

Context
Errors during restore was not reported when using /tl (live logger) leaving user without actionable information.

Changes Made
When restore success without error or warn - same as before
When restore success with warnings - report success with warnins and list warnings bellow that report
When restore fails - report errors and warnings THEN report Restore failed in 1.5s and do not report Build summary
  • Loading branch information
@rokonec
rokonec authored May 4, 2023
1 parent 6882ab9 commit b313662
Show file tree
Hide file tree
Showing 19 changed files with 242 additions and 96 deletions.
21 changes: 17 additions & 4 deletions src/MSBuild.UnitTests/LiveLogger_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Net.NetworkInformation;
using System.Text.RegularExpressions;
using Microsoft.Build.Framework;
using Microsoft.Build.Logging.LiveLogger;
using Shouldly;
Expand Down Expand Up @@ -182,7 +184,7 @@ private void InvokeLoggerCallbacksForSimpleProject(bool succeeded, Action additi
public void PrintsBuildSummary_Succeeded()
{
InvokeLoggerCallbacksForSimpleProject(succeeded: true, () => { });
_mockTerminal.GetLastLine().ShouldBe("Build succeeded in 5.0s");
_mockTerminal.GetLastLine().WithoutAnsiCodes().ShouldBe("Build succeeded in 5.0s");
}

[Fact]
Expand All @@ -192,14 +194,14 @@ public void PrintBuildSummary_SucceededWithWarnings()
{
WarningRaised?.Invoke(_eventSender, MakeWarningEventArgs("Warning!"));
});
_mockTerminal.GetLastLine().ShouldBe("Build succeeded with warnings in 5.0s");
_mockTerminal.GetLastLine().WithoutAnsiCodes().ShouldBe("Build succeeded with warnings in 5.0s");
}

[Fact]
public void PrintBuildSummary_Failed()
{
InvokeLoggerCallbacksForSimpleProject(succeeded: false, () => { });
_mockTerminal.GetLastLine().ShouldBe("Build failed in 5.0s");
_mockTerminal.GetLastLine().WithoutAnsiCodes().ShouldBe("Build failed in 5.0s");
}

[Fact]
Expand All @@ -209,9 +211,20 @@ public void PrintBuildSummary_FailedWithErrors()
{
ErrorRaised?.Invoke(_eventSender, MakeErrorEventArgs("Error!"));
});
_mockTerminal.GetLastLine().ShouldBe("Build failed with errors in 5.0s");
_mockTerminal.GetLastLine().WithoutAnsiCodes().ShouldBe("Build failed with errors in 5.0s");
}

#endregion

}

internal static class StringVT100Extensions
{
private static Regex s_removeAnsiCodes = new Regex("\\x1b\\[[0-9;]*[mGKHF]");

public static string WithoutAnsiCodes(this string text)
{
return s_removeAnsiCodes.Replace(text, string.Empty);
}
}
}
1 change: 0 additions & 1 deletion src/MSBuild.UnitTests/MockTerminal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ public void EndUpdate()
public void Write(ReadOnlySpan<char> text) { AddOutput(text.ToString()); }
public void WriteColor(TerminalColor color, string text) => AddOutput(text);
public void WriteColorLine(TerminalColor color, string text) { AddOutput(text); AddOutput("\n"); }
public string RenderColor(TerminalColor color, string text) => text;

public void WriteLine(string text) { AddOutput(text); AddOutput("\n"); }
public void WriteLineFitToWidth(ReadOnlySpan<char> text)
Expand Down
5 changes: 0 additions & 5 deletions src/MSBuild/LiveLogger/ITerminal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,4 @@ internal interface ITerminal : IDisposable
/// Writes a string to the output using the given color. Or buffers it if <see cref="BeginUpdate"/> was called.
/// </summary>
void WriteColorLine(TerminalColor color, string text);

/// <summary>
/// Return string representing text wrapped in VT100 color codes.
/// </summary>
string RenderColor(TerminalColor color, string text);
}
178 changes: 99 additions & 79 deletions src/MSBuild/LiveLogger/LiveLogger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ public override string ToString()
/// </summary>
private bool _buildHasWarnings;

/// <summary>
/// True if restore failed and this failure has already been reported.
/// </summary>
private bool _restoreFailed;

/// <summary>
/// The project build context corresponding to the <c>Restore</c> initial target, or null if the build is currently
/// bot restoring.
Expand Down Expand Up @@ -235,12 +240,22 @@ private void BuildFinished(object sender, BuildFinishedEventArgs e)
Terminal.BeginUpdate();
try
{
double duration = (e.Timestamp - _buildStartTime).TotalSeconds;
string duration = (e.Timestamp - _buildStartTime).TotalSeconds.ToString("F1");
string buildResult = RenderBuildResult(e.Succeeded, _buildHasErrors, _buildHasWarnings);

Terminal.WriteLine("");
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("BuildFinished",
RenderBuildResult(e.Succeeded, _buildHasErrors, _buildHasWarnings),
duration.ToString("F1")));
if (_restoreFailed)
{
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("RestoreCompleteWithMessage",
buildResult,
duration));
}
else
{
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("BuildFinished",
buildResult,
duration));
}
}
finally
{
Expand All @@ -249,6 +264,7 @@ private void BuildFinished(object sender, BuildFinishedEventArgs e)

_buildHasErrors = false;
_buildHasWarnings = false;
_restoreFailed = false;
}

/// <summary>
Expand Down Expand Up @@ -302,35 +318,7 @@ private void ProjectFinished(object sender, ProjectFinishedEventArgs e)

ProjectContext c = new(buildEventContext);

// First check if we're done restoring.
if (_restoreContext is ProjectContext restoreContext && c == restoreContext)
{
lock (_lock)
{
_restoreContext = null;

Stopwatch projectStopwatch = _projects[restoreContext].Stopwatch;
double duration = projectStopwatch.Elapsed.TotalSeconds;
projectStopwatch.Stop();

Terminal.BeginUpdate();
try
{
EraseNodes();
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("RestoreComplete",
duration.ToString("F1")));
DisplayNodes();
}
finally
{
Terminal.EndUpdate();
}
return;
}
}

// If this was a notable project build, we print it as completed only if it's produced an output or warnings/error.
if (_projects.TryGetValue(c, out Project? project) && (project.OutputPath is not null || project.BuildMessages is not null))
if (_projects.TryGetValue(c, out Project? project))
{
lock (_lock)
{
Expand All @@ -350,65 +338,97 @@ private void ProjectFinished(object sender, ProjectFinishedEventArgs e)
// reported during build.
bool haveErrors = project.BuildMessages?.Exists(m => m.Severity == MessageSeverity.Error) == true;
bool haveWarnings = project.BuildMessages?.Exists(m => m.Severity == MessageSeverity.Warning) == true;

string buildResult = RenderBuildResult(e.Succeeded, haveErrors, haveWarnings);

if (string.IsNullOrEmpty(project.TargetFramework))
// Check if we're done restoring.
if (c == _restoreContext)
{
Terminal.Write(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_NoTF",
Indentation,
projectFile,
buildResult,
duration));
if (e.Succeeded)
{
if (haveErrors || haveWarnings)
{
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("RestoreCompleteWithMessage",
buildResult,
duration));
}
else
{
Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("RestoreComplete",
duration));
}
}
else
{
// It will be reported after build finishes.
_restoreFailed = true;
}

_restoreContext = null;
}
else
// If this was a notable project build, we print it as completed only if it's produced an output or warnings/error.
else if (project.OutputPath is not null || project.BuildMessages is not null)
{
Terminal.Write(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_WithTF",
Indentation,
projectFile,
AnsiCodes.Colorize(project.TargetFramework, TargetFrameworkColor),
buildResult,
duration));
}
// Show project build complete and its output

// Print the output path as a link if we have it.
if (outputPath is not null)
{
ReadOnlySpan<char> outputPathSpan = outputPath.Value.Span;
ReadOnlySpan<char> url = outputPathSpan;
try
if (string.IsNullOrEmpty(project.TargetFramework))
{
// If possible, make the link point to the containing directory of the output.
url = Path.GetDirectoryName(url);
Terminal.Write(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_NoTF",
Indentation,
projectFile,
buildResult,
duration));
}
catch
else
{
// Ignore any GetDirectoryName exceptions.
Terminal.Write(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_WithTF",
Indentation,
projectFile,
AnsiCodes.Colorize(project.TargetFramework, TargetFrameworkColor),
buildResult,
duration));
}

// Generates file:// schema url string which is better handled by various Terminal clients than raw folder name.
string urlString = url.ToString();
if (Uri.TryCreate(urlString, UriKind.Absolute, out Uri? uri))
// Print the output path as a link if we have it.
if (outputPath is not null)
{
urlString = uri.AbsoluteUri;
}
ReadOnlySpan<char> outputPathSpan = outputPath.Value.Span;
ReadOnlySpan<char> url = outputPathSpan;
try
{
// If possible, make the link point to the containing directory of the output.
url = Path.GetDirectoryName(url);
}
catch
{
// Ignore any GetDirectoryName exceptions.
}

// If the output path is under the initial working directory, make the console output relative to that to save space.
if (outputPathSpan.StartsWith(_initialWorkingDirectory.AsSpan(), FileUtilities.PathComparison))
{
if (outputPathSpan.Length > _initialWorkingDirectory.Length
&& (outputPathSpan[_initialWorkingDirectory.Length] == Path.DirectorySeparatorChar
|| outputPathSpan[_initialWorkingDirectory.Length] == Path.AltDirectorySeparatorChar))
// Generates file:// schema url string which is better handled by various Terminal clients than raw folder name.
string urlString = url.ToString();
if (Uri.TryCreate(urlString, UriKind.Absolute, out Uri? uri))
{
outputPathSpan = outputPathSpan.Slice(_initialWorkingDirectory.Length + 1);
urlString = uri.AbsoluteUri;
}
}

Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_OutputPath",
$"{AnsiCodes.LinkPrefix}{urlString}{AnsiCodes.LinkInfix}{outputPathSpan.ToString()}{AnsiCodes.LinkSuffix}"));
}
else
{
Terminal.WriteLine(string.Empty);
// If the output path is under the initial working directory, make the console output relative to that to save space.
if (outputPathSpan.StartsWith(_initialWorkingDirectory.AsSpan(), FileUtilities.PathComparison))
{
if (outputPathSpan.Length > _initialWorkingDirectory.Length
&& (outputPathSpan[_initialWorkingDirectory.Length] == Path.DirectorySeparatorChar
|| outputPathSpan[_initialWorkingDirectory.Length] == Path.AltDirectorySeparatorChar))
{
outputPathSpan = outputPathSpan.Slice(_initialWorkingDirectory.Length + 1);
}
}

Terminal.WriteLine(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ProjectFinished_OutputPath",
$"{AnsiCodes.LinkPrefix}{urlString}{AnsiCodes.LinkInfix}{outputPathSpan.ToString()}{AnsiCodes.LinkSuffix}"));
}
else
{
Terminal.WriteLine(string.Empty);
}
}

// Print diagnostic output under the Project -> Output line.
Expand Down Expand Up @@ -762,15 +782,15 @@ private string RenderBuildResult(bool succeeded, bool hasError, bool hasWarning)
(false, true) => ResourceUtilities.GetResourceString("BuildResult_FailedWithWarnings"),
_ => ResourceUtilities.GetResourceString("BuildResult_Failed"),
};
return Terminal.RenderColor(TerminalColor.Red, text);
return AnsiCodes.Colorize(text, TerminalColor.Red);
}
else if (hasWarning)
{
return Terminal.RenderColor(TerminalColor.Yellow, ResourceUtilities.GetResourceString("BuildResult_SucceededWithWarnings"));
return AnsiCodes.Colorize(ResourceUtilities.GetResourceString("BuildResult_SucceededWithWarnings"), TerminalColor.Yellow);
}
else
{
return Terminal.RenderColor(TerminalColor.Green, ResourceUtilities.GetResourceString("BuildResult_Succeeded"));
return AnsiCodes.Colorize(ResourceUtilities.GetResourceString("BuildResult_Succeeded"), TerminalColor.Green);
}
}

Expand Down
8 changes: 1 addition & 7 deletions src/MSBuild/LiveLogger/Terminal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,10 @@ public void WriteColor(TerminalColor color, string text)
}
else
{
Write(RenderColor(color, text));
Write(AnsiCodes.Colorize(text, color));
}
}

/// <inheritdoc/>
public string RenderColor(TerminalColor color, string text)
{
return $"{AnsiCodes.CSI}{(int)color}{AnsiCodes.SetColor}{text}{AnsiCodes.SetDefaultColor}";
}

/// <inheritdoc/>
public void WriteColorLine(TerminalColor color, string text)
{
Expand Down
8 changes: 8 additions & 0 deletions src/MSBuild/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,14 @@
{0}: duration in seconds with 1 decimal point
</comment>
</data>
<data name="RestoreCompleteWithMessage" xml:space="preserve">
<value>Restore {0} in {1}s</value>
<comment>
Restore summary when finished with warning or error
{0}: BuildResult_X (below)
{1}: duration in seconds with 1 decimal point
</comment>
</data>
<data name="BuildFinished" xml:space="preserve">
<value>Build {0} in {1}s</value>
<comment>
Expand Down
9 changes: 9 additions & 0 deletions src/MSBuild/Resources/xlf/Strings.cs.xlf
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/MSBuild/Resources/xlf/Strings.de.xlf
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/MSBuild/Resources/xlf/Strings.es.xlf
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit b313662

Please sign in to comment.