Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Log TaskStarted line and column #6399

Merged
merged 1 commit into from
May 28, 2021
Merged

Log TaskStarted line and column #6399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions ref/Microsoft.Build.Framework/net/Microsoft.Build.Framework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,8 @@ public partial class TaskStartedEventArgs : Microsoft.Build.Framework.BuildStatu
protected TaskStartedEventArgs() { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName) { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, System.DateTime eventTimestamp) { }
public int ColumnNumber { get { throw null; } }
public int LineNumber { get { throw null; } }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TaskFile { get { throw null; } }
Expand Down
2 changes: 2 additions & 0 deletions ref/Microsoft.Build.Framework/netstandard/Microsoft.Build.Framework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,8 @@ public partial class TaskStartedEventArgs : Microsoft.Build.Framework.BuildStatu
protected TaskStartedEventArgs() { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName) { }
public TaskStartedEventArgs(string message, string helpKeyword, string projectFile, string taskFile, string taskName, System.DateTime eventTimestamp) { }
public int ColumnNumber { get { throw null; } }
public int LineNumber { get { throw null; } }
public override string Message { get { throw null; } }
public string ProjectFile { get { throw null; } }
public string TaskFile { get { throw null; } }
Expand Down
2 changes: 1 addition & 1 deletion src/Build.UnitTests/BackEnd/MockLoggingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ public void LogTaskStarted(BuildEventContext targetBuildEventContext, string tas
/// <param name="projectFile">The project file</param>
/// <param name="projectFileOfTaskNode">The project file containing the task node.</param>
/// <returns>The task logging context</returns>
public BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode)
public BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode, int line, int column)
{
return new BuildEventContext(0, 0, 0, 0);
}
Expand Down
8 changes: 7 additions & 1 deletion src/Build.UnitTests/BackEnd/NodePackets_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,11 @@ public void TestTranslation()
new BuildFinishedEventArgs("Message", "Keyword", true),
new BuildStartedEventArgs("Message", "Help"),
new BuildMessageEventArgs("Message", "help", "sender", MessageImportance.Low),
new TaskStartedEventArgs("message", "help", "projectFile", "taskFile", "taskName"),
new TaskStartedEventArgs("message", "help", "projectFile", "taskFile", "taskName")
{
LineNumber = 345,
ColumnNumber = 123
},
new TaskFinishedEventArgs("message", "help", "projectFile", "taskFile", "taskName", true),
new TaskCommandLineEventArgs("commandLine", "taskName", MessageImportance.Low),
CreateTaskParameter(),
Expand Down Expand Up @@ -454,6 +458,8 @@ private void CompareLogMessagePackets(LogMessagePacket left, LogMessagePacket ri
Assert.Equal(leftTaskStarted.ProjectFile, rightTaskStarted.ProjectFile);
Assert.Equal(leftTaskStarted.TaskFile, rightTaskStarted.TaskFile);
Assert.Equal(leftTaskStarted.TaskName, rightTaskStarted.TaskName);
Assert.Equal(leftTaskStarted.LineNumber, rightTaskStarted.LineNumber);
Assert.Equal(leftTaskStarted.ColumnNumber, rightTaskStarted.ColumnNumber);
break;

default:
Expand Down
6 changes: 5 additions & 1 deletion src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,15 @@ public void RoundtripTaskStartedEventArgs()
projectFile: "C:\\project.proj",
taskFile: "C:\\common.targets",
taskName: "Csc");
args.LineNumber = 42;
args.ColumnNumber = 999;
Forgind marked this conversation as resolved.
Show resolved Hide resolved

Roundtrip(args,
e => e.ProjectFile,
e => e.TaskFile,
e => e.TaskName);
e => e.TaskName,
e => e.LineNumber.ToString(),
e => e.ColumnNumber.ToString());
}

[Fact]
Expand Down
4 changes: 3 additions & 1 deletion src/Build/BackEnd/Components/Logging/ILoggingService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,10 @@ void LogProjectEvaluationFinished(
/// <param name="taskName">The name of the task</param>
/// <param name="projectFile">The project file which is being built</param>
/// <param name="projectFileOfTaskNode">The file in which the task is defined - typically a .targets file</param>
/// <param name="line">The line number in the file where the task invocation is located.</param>
/// <param name="column">The column number in the file where the task invocation is located.</param>
/// <returns>The task build event context</returns>
BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode);
BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode, int line, int column);

/// <summary>
/// Log that a task has just completed
Expand Down
6 changes: 5 additions & 1 deletion src/Build/BackEnd/Components/Logging/LoggingServiceLogMethods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,9 +754,11 @@ public void LogTaskStarted(BuildEventContext taskBuildEventContext, string taskN
/// <param name="taskName">Task Name</param>
/// <param name="projectFile">Project file being built</param>
/// <param name="projectFileOfTaskNode">Project file which contains the task</param>
/// <param name="line">The line number in the file where the task invocation is located.</param>
/// <param name="column">The column number in the file where the task invocation is located.</param>
/// <returns>The build event context for the task.</returns>
/// <exception cref="InternalErrorException">BuildEventContext is null</exception>
public BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode)
public BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventContext, string taskName, string projectFile, string projectFileOfTaskNode, int line, int column)
{
lock (_lockObject)
{
Expand All @@ -782,6 +784,8 @@ public BuildEventContext LogTaskStarted2(BuildEventContext targetBuildEventConte
taskName
);
buildEvent.BuildEventContext = taskBuildEventContext;
buildEvent.LineNumber = line;
buildEvent.ColumnNumber = column;
ProcessLoggingEvent(buildEvent);
}

Expand Down
4 changes: 3 additions & 1 deletion src/Build/BackEnd/Components/Logging/TaskLoggingContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ internal TaskLoggingContext(TargetLoggingContext targetLoggingContext, string pr
targetLoggingContext.BuildEventContext,
_taskName,
projectFullPath,
task.Location.File
task.Location.File,
task.Location.Line,
task.Location.Column
);
this.IsValid = true;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ private BuildEventArgs ReadTaskStartedEventArgs()
taskFile,
taskName,
fields.Timestamp);
e.LineNumber = fields.LineNumber;
e.ColumnNumber = fields.ColumnNumber;
SetCommonFields(e, fields);
return e;
}
Expand Down
11 changes: 9 additions & 2 deletions src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,9 @@ private void Write(TargetFinishedEventArgs e)
private void Write(TaskStartedEventArgs e)
{
Write(BinaryLogRecordKind.TaskStarted);
WriteBuildEventArgsFields(e, writeMessage: false);
WriteBuildEventArgsFields(e, writeMessage: false, writeLineAndColumn: true);
Write(e.LineNumber);
Write(e.ColumnNumber);
WriteDeduplicatedString(e.TaskName);
WriteDeduplicatedString(e.ProjectFile);
WriteDeduplicatedString(e.TaskFile);
Expand Down Expand Up @@ -512,9 +514,14 @@ private void Write(TaskParameterEventArgs e)
WriteTaskItemList(e.Items, e.LogItemMetadata);
}

private void WriteBuildEventArgsFields(BuildEventArgs e, bool writeMessage = true)
private void WriteBuildEventArgsFields(BuildEventArgs e, bool writeMessage = true, bool writeLineAndColumn = false)
{
var flags = GetBuildEventArgsFieldFlags(e, writeMessage);
if (writeLineAndColumn)
{
flags |= BuildEventArgsFieldFlags.LineNumber | BuildEventArgsFieldFlags.ColumnNumber;
}
ladipro marked this conversation as resolved.
Show resolved Hide resolved

Write((int)flags);
WriteBaseFields(e, flags);
}
Expand Down
14 changes: 14 additions & 0 deletions src/Framework/TaskStartedEventArgs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ internal override void WriteToStream(BinaryWriter writer)
writer.WriteOptionalString(taskName);
writer.WriteOptionalString(projectFile);
writer.WriteOptionalString(taskFile);
writer.Write7BitEncodedInt(LineNumber);
writer.Write7BitEncodedInt(ColumnNumber);
}

/// <summary>
Expand All @@ -105,6 +107,8 @@ internal override void CreateFromStream(BinaryReader reader, int version)
taskName = reader.ReadByte() == 0 ? null : reader.ReadString();
projectFile = reader.ReadByte() == 0 ? null : reader.ReadString();
taskFile = reader.ReadByte() == 0 ? null : reader.ReadString();
LineNumber = reader.Read7BitEncodedInt();
ColumnNumber = reader.Read7BitEncodedInt();
Comment on lines +110 to +111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like @Forgind I also wonder if the change is backward compatible. Shouldn't you be testing for BuildEventArgsFieldFlags.LineNumber and BuildEventArgsFieldFlags.ColumnNumber here before reading the integers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I understand. This code is separate from the binlog logic and has no versioning concerns, is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is unrelated to binlog and used by the node packet translator to send events from worker nodes to central node. Nodes will only talk to each other if they’re the same MSBuild version.

}
#endregion

Expand All @@ -123,6 +127,16 @@ internal override void CreateFromStream(BinaryReader reader, int version)
/// </summary>
public string TaskFile => taskFile;

/// <summary>
/// Line number of the task invocation in the project file
/// </summary>
public int LineNumber { get; internal set; }

/// <summary>
/// Column number of the task invocation in the project file
/// </summary>
public int ColumnNumber { get; internal set; }

public override string Message
{
get
Expand Down