From d9f7791af662a3171b73bc3135698f707801b902 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Fri, 6 Dec 2024 16:41:09 +0100 Subject: [PATCH 01/25] add location to the initial property assignment event and tracking props in targets/tasks --- .../RequestBuilder/IntrinsicTask.cs | 3 +- .../PropertyGroupIntrinsicTask.cs | 50 ++++++++++++++++++- .../TaskExecutionHost/TaskExecutionHost.cs | 48 ++++++++++++++++++ src/Build/Evaluation/Evaluator.cs | 1 - .../PropertyTrackingEvaluatorDataWrapper.cs | 48 ++++++++++-------- .../BinaryLogger/BuildEventArgsReader.cs | 5 ++ .../PropertyInitialValueSetEventArgs.cs | 34 ++++++++++++- 7 files changed, 165 insertions(+), 24 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs index 94c1ee183ac..68a82e27491 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs @@ -6,6 +6,7 @@ using System.Reflection; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Execution; +using Microsoft.Build.Framework; using Microsoft.Build.Shared; #nullable disable @@ -69,7 +70,7 @@ internal static IntrinsicTask InstantiateTask(ProjectTargetInstanceChild taskIns { if (taskInstance is ProjectPropertyGroupTaskInstance propertyGroupTaskInstance) { - return new PropertyGroupIntrinsicTask(propertyGroupTaskInstance, loggingContext, projectInstance, logTaskInputs); + return new PropertyGroupIntrinsicTask(propertyGroupTaskInstance, loggingContext, projectInstance, logTaskInputs, Traits.Instance.LogPropertyTracking); } else if (taskInstance is ProjectItemGroupTaskInstance itemGroupTaskInstance) { diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 02ca6a1dab8..c35cd5f634a 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -25,6 +25,8 @@ internal class PropertyGroupIntrinsicTask : IntrinsicTask /// private ProjectPropertyGroupTaskInstance _taskInstance; + private readonly PropertyTrackingSetting _propertyTrackingSettings; + /// /// Create a new PropertyGroup task. /// @@ -32,10 +34,12 @@ internal class PropertyGroupIntrinsicTask : IntrinsicTask /// The logging context /// The project instance /// Flag to determine whether or not to log task inputs. - public PropertyGroupIntrinsicTask(ProjectPropertyGroupTaskInstance taskInstance, TargetLoggingContext loggingContext, ProjectInstance projectInstance, bool logTaskInputs) + /// Flag to determine whether or not property tracking enabled. + public PropertyGroupIntrinsicTask(ProjectPropertyGroupTaskInstance taskInstance, TargetLoggingContext loggingContext, ProjectInstance projectInstance, bool logTaskInputs, int settingValue) : base(loggingContext, projectInstance, logTaskInputs) { _taskInstance = taskInstance; + _propertyTrackingSettings = (PropertyTrackingSetting)settingValue; } /// @@ -85,6 +89,8 @@ internal override void ExecuteTask(Lookup lookup) string evaluatedValue = bucket.Expander.ExpandIntoStringLeaveEscaped(property.Value, ExpanderOptions.ExpandAll, property.Location); bucket.Expander.PropertiesUseTracker.CheckPreexistingUndefinedUsage(property, evaluatedValue, LoggingContext); + LogPropertyInTargetAssignment(property, evaluatedValue); + if (LogTaskInputs && !LoggingContext.LoggingService.OnlyLogCriticalEvents) { LoggingContext.LogComment(MessageImportance.Low, "PropertyGroupLogMessage", property.Name, evaluatedValue); @@ -111,6 +117,48 @@ internal override void ExecuteTask(Lookup lookup) } } + /// + /// Logs property assignment information during target execution, providing detailed tracking of property value changes. + /// + /// The property instance being assigned or modified. + /// The new evaluated value of the property. + private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInstance property, string evaluatedValue) + { + if (_propertyTrackingSettings == 0 || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) + { + return; + } + + var previousPropertyValue = Project.GetProperty(property.Name)?.EvaluatedValue; + + if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + { + var args = new PropertyInitialValueSetEventArgs( + property.Name, + evaluatedValue, + propertySource: string.Empty, + property.Location.File, + property.Location.Line, + property.Location.Column, + ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", property.Name, evaluatedValue, property.Location.LocationString ?? "Target")) + { BuildEventContext = LoggingContext.BuildEventContext }; + + LoggingContext.LogBuildEvent(args); + } + else if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) + { + var args = new PropertyReassignmentEventArgs( + property.Name, + previousPropertyValue, + evaluatedValue, + property.Location.LocationString, + message: null) + { BuildEventContext = LoggingContext.BuildEventContext, }; + + LoggingContext.LogBuildEvent(args); + } + } + /// /// Adds batchable parameters from a property element into the list. If the property element was /// a task, these would be its raw parameter values. diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 0e4c160336f..697cfdfa889 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -156,6 +156,8 @@ internal class TaskExecutionHost : IDisposable /// private readonly Dictionary _intrinsicTasks = new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly PropertyTrackingSetting _propertyTrackingSettings; + /// /// Constructor /// @@ -172,6 +174,8 @@ internal TaskExecutionHost(IBuildComponentHost host) { LogTaskInputs = Traits.Instance.EscapeHatches.LogTaskInputs; } + + _propertyTrackingSettings = (PropertyTrackingSetting)Traits.Instance.LogPropertyTracking; } /// @@ -1582,12 +1586,56 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } + LogPropertyInTaskAssignment(outputTargetName, outputString, parameterLocation); _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable)); } } } } + /// + /// Logs property assignment information during task execution, based on configured property tracking settings. + /// + /// The name of the property being assigned or reassigned. + /// The new value being assigned to the property. + /// The source location where the property assignment occurs. + private void LogPropertyInTaskAssignment(string propertyName, string propertyValue, IElementLocation location) + { + if (_propertyTrackingSettings == 0 || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) + { + return; + } + + var previousPropertyValue = ProjectInstance.GetProperty(propertyName); + + if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + { + var args = new PropertyInitialValueSetEventArgs( + propertyName, + propertyValue, + propertySource: string.Empty, + location.File, + location.Line, + location.Column, + ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", propertyName, propertyValue, location?.LocationString ?? "Task")) + { BuildEventContext = _targetLoggingContext.BuildEventContext }; + + _targetLoggingContext.LogBuildEvent(args); + } + else if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) + { + var args = new PropertyReassignmentEventArgs( + propertyName, + ProjectInstance.GetProperty(propertyName).EvaluatedValue, + propertyValue, + location.LocationString, + message: null) + { BuildEventContext = _targetLoggingContext.BuildEventContext }; + + _targetLoggingContext.LogBuildEvent(args); + } + } + /// /// Finds all the task properties that are required. /// Returns them as keys in a dictionary. diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index daf8d0ea71b..7483294ca70 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -12,7 +12,6 @@ using Microsoft.Build.BackEnd; using Microsoft.Build.BackEnd.Components.Logging; using Microsoft.Build.BackEnd.Components.RequestBuilder; -using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.BackEnd.SdkResolution; using Microsoft.Build.Collections; using Microsoft.Build.Construction; diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 1e9861f742b..f9843110e34 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -258,12 +258,12 @@ private void TrackPropertyWrite( if (predecessor == null) { // If this property had no previous value, then track an initial value. - TrackPropertyInitialValueSet(property, source); + TrackPropertyInitialValueSet(property, source, location); } else { // There was a previous value, and it might have been changed. Track that. - TrackPropertyReassignment(predecessor, property, location?.LocationString); + TrackPropertyReassignment(predecessor, property, location); } // If this property was an environment variable but no longer is, track it. @@ -278,18 +278,26 @@ private void TrackPropertyWrite( /// /// The property being set. /// The source of the property. - private void TrackPropertyInitialValueSet(P property, PropertySource source) + /// The exact location of the property. Can be null if comes not form xml. + private void TrackPropertyInitialValueSet(P property, PropertySource source, IElementLocation? location) { if ((_settings & PropertyTrackingSetting.PropertyInitialValueSet) != PropertyTrackingSetting.PropertyInitialValueSet) { return; } - var args = new PropertyInitialValueSetEventArgs( - property.Name, - property.EvaluatedValue, - source.ToString(), - ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", property.Name, property.EvaluatedValue, source)); + property.Name, + property.EvaluatedValue, + location?.LocationString ?? source.ToString(), + location?.File, + location?.Line ?? 0, + location?.Column ?? 0, + ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword( + "PropertyAssignment", + property.Name, + property.EvaluatedValue, + location?.LocationString ?? source.ToString())); + args.BuildEventContext = _evaluationLoggingContext.BuildEventContext; _evaluationLoggingContext.LogBuildEvent(args); @@ -301,7 +309,7 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source) /// The property's preceding state. Null if none. /// The property's current state. /// The location of this property's reassignment. - private void TrackPropertyReassignment(P? predecessor, P property, string? location) + private void TrackPropertyReassignment(P? predecessor, P property, IElementLocation? location) { if (MSBuildNameIgnoreCaseComparer.Default.Equals(property.Name, "MSBuildAllProjects")) { @@ -326,7 +334,7 @@ private void TrackPropertyReassignment(P? predecessor, P property, string? locat property.Name, oldValue, newValue, - location, + location?.LocationString, message: null) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; @@ -375,18 +383,18 @@ private enum PropertySource Toolset, EnvironmentVariable } + } - [Flags] - private enum PropertyTrackingSetting - { - None = 0, + [Flags] + internal enum PropertyTrackingSetting + { + None = 0, - PropertyReassignment = 1, - PropertyInitialValueSet = 1 << 1, - EnvironmentVariableRead = 1 << 2, - UninitializedPropertyRead = 1 << 3, + PropertyReassignment = 1, + PropertyInitialValueSet = 1 << 1, + EnvironmentVariableRead = 1 << 2, + UninitializedPropertyRead = 1 << 3, - All = PropertyReassignment | PropertyInitialValueSet | EnvironmentVariableRead | UninitializedPropertyRead - } + All = PropertyReassignment | PropertyInitialValueSet | EnvironmentVariableRead | UninitializedPropertyRead } } diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 0b97024c472..8e5ad536b0b 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -4,6 +4,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Data.Common; using System.IO; using System.IO.Compression; using System.Linq; @@ -1214,10 +1215,14 @@ private BuildEventArgs ReadPropertyInitialValueSetEventArgs() propertyName, propertyValue, propertySource, + fields.File, + fields.LineNumber, + fields.ColumnNumber, fields.Message, fields.HelpKeyword, fields.SenderName, fields.Importance); + SetCommonFields(e, fields); return e; diff --git a/src/Framework/PropertyInitialValueSetEventArgs.cs b/src/Framework/PropertyInitialValueSetEventArgs.cs index 318755fcde3..e9b7d91666f 100644 --- a/src/Framework/PropertyInitialValueSetEventArgs.cs +++ b/src/Framework/PropertyInitialValueSetEventArgs.cs @@ -37,13 +37,45 @@ public PropertyInitialValueSetEventArgs( string message, string helpKeyword = null, string senderName = null, - MessageImportance importance = MessageImportance.Low) : base(message, helpKeyword, senderName, importance) + MessageImportance importance = MessageImportance.Low) + : base(message, helpKeyword, senderName, importance) { this.PropertyName = propertyName; this.PropertyValue = propertyValue; this.PropertySource = propertySource; } + /// + /// Creates an instance of the class. + /// + /// The name of the property. + /// The value of the property. + /// The source of the property. + /// The file associated with the event. + /// The line number (0 if not applicable). + /// The column number (0 if not applicable). + /// The message of the property. + /// The help keyword. + /// The sender name of the event. + /// The importance of the message. + public PropertyInitialValueSetEventArgs( + string propertyName, + string propertyValue, + string propertySource, + string file, + int line, + int column, + string message, + string helpKeyword = null, + string senderName = null, + MessageImportance importance = MessageImportance.Low) + : base(subcategory: null, code: null, file: file, lineNumber: line, columnNumber: column, 0, 0, message, helpKeyword, senderName, importance) + { + PropertyName = propertyName; + PropertyValue = propertyValue; + PropertySource = propertySource; + } + /// /// The name of the property. /// From 1988391696802bfbe6b50601bc321698171febf9 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Fri, 6 Dec 2024 16:50:40 +0100 Subject: [PATCH 02/25] extend condition --- .../IntrinsicTasks/PropertyGroupIntrinsicTask.cs | 3 ++- src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index c35cd5f634a..9be74820534 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -131,7 +131,8 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta var previousPropertyValue = Project.GetProperty(property.Name)?.EvaluatedValue; - if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + if (previousPropertyValue == null + && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) { var args = new PropertyInitialValueSetEventArgs( property.Name, diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 697cfdfa889..c3c0ada8f5e 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1606,9 +1606,9 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal return; } - var previousPropertyValue = ProjectInstance.GetProperty(propertyName); + var previousPropertyValue = _projectInstance.GetProperty(propertyName)?.EvaluatedValue; - if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + if (previousPropertyValue == null && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) { var args = new PropertyInitialValueSetEventArgs( propertyName, @@ -1626,7 +1626,7 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal { var args = new PropertyReassignmentEventArgs( propertyName, - ProjectInstance.GetProperty(propertyName).EvaluatedValue, + previousPropertyValue, propertyValue, location.LocationString, message: null) From c034187eb020b4541e112fd30d57b06db27db793 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Fri, 6 Dec 2024 17:02:33 +0100 Subject: [PATCH 03/25] fix typo --- .../RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs | 2 +- src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 9be74820534..d783deec077 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -132,7 +132,7 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta var previousPropertyValue = Project.GetProperty(property.Name)?.EvaluatedValue; if (previousPropertyValue == null - && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyInitialValueSet) { var args = new PropertyInitialValueSetEventArgs( property.Name, diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index c3c0ada8f5e..55bc4e6aa2c 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1608,7 +1608,7 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal var previousPropertyValue = _projectInstance.GetProperty(propertyName)?.EvaluatedValue; - if (previousPropertyValue == null && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyReassignment) + if (previousPropertyValue == null && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyInitialValueSet) { var args = new PropertyInitialValueSetEventArgs( propertyName, From 9de0ce2d8e79f2b4cd156c172200758b98669f5a Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 9 Dec 2024 14:50:18 +0100 Subject: [PATCH 04/25] add separate categorization of properties specified in command line --- .../BackEnd/BuildManager/BuildParameters.cs | 12 ++++++ src/Build/Definition/Project.cs | 3 +- src/Build/Definition/ProjectCollection.cs | 13 +++++++ src/Build/Evaluation/Evaluator.cs | 31 +++++++++++++++- src/Build/Evaluation/IEvaluatorData.cs | 2 +- .../LazyItemEvaluator.EvaluatorData.cs | 4 +- .../PropertyTrackingEvaluatorDataWrapper.cs | 37 ++++++++++--------- src/Build/Instance/ProjectInstance.cs | 3 +- 8 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index a8d528d4fb4..aa1dcf9f728 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -146,6 +146,11 @@ public class BuildParameters : ITranslatable /// private PropertyDictionary _globalProperties = new PropertyDictionary(); + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + private HashSet _propertiesFromCommandLine; + /// /// The loggers. /// @@ -250,6 +255,7 @@ public BuildParameters(ProjectCollection projectCollection) _defaultToolsVersion = projectCollection.DefaultToolsVersion; _globalProperties = new PropertyDictionary(projectCollection.GlobalPropertiesCollection); + _propertiesFromCommandLine = projectCollection.PropertiesFromCommandLine; } /// @@ -279,6 +285,7 @@ internal BuildParameters(BuildParameters other, bool resetEnvironment = false) _environmentProperties = other._environmentProperties != null ? new PropertyDictionary(other._environmentProperties) : null; _forwardingLoggers = other._forwardingLoggers != null ? new List(other._forwardingLoggers) : null; _globalProperties = other._globalProperties != null ? new PropertyDictionary(other._globalProperties) : null; + _propertiesFromCommandLine = other._propertiesFromCommandLine != null ? new HashSet(other._propertiesFromCommandLine) : null; HostServices = other.HostServices; _loggers = other._loggers != null ? new List(other._loggers) : null; _maxNodeCount = other._maxNodeCount; @@ -472,6 +479,11 @@ public IDictionary GlobalProperties } } + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + public HashSet PropertiesFromCommandLine => _propertiesFromCommandLine; + /// /// Interface allowing the host to provide additional control over the build process. /// diff --git a/src/Build/Definition/Project.cs b/src/Build/Definition/Project.cs index 4a868767aea..1348a7cffcc 100644 --- a/src/Build/Definition/Project.cs +++ b/src/Build/Definition/Project.cs @@ -3721,6 +3721,7 @@ private void Reevaluate( loadSettings, ProjectCollection.MaxNodeCount, ProjectCollection.EnvironmentProperties, + ProjectCollection.PropertiesFromCommandLine, loggingServiceForEvaluation, new ProjectItemFactory(Owner), ProjectCollection, @@ -4437,7 +4438,7 @@ public IItemDefinition GetItemDefinition(string itemType) /// /// Sets a property which is not derived from Xml. /// - public ProjectProperty SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable = false) + public ProjectProperty SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable = false, bool isCommandLineProperty = false) { ProjectProperty property = ProjectProperty.Create(Project, name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, loggingContext); Properties.Set(property); diff --git a/src/Build/Definition/ProjectCollection.cs b/src/Build/Definition/ProjectCollection.cs index 031e31f1e2e..6b73a927756 100644 --- a/src/Build/Definition/ProjectCollection.cs +++ b/src/Build/Definition/ProjectCollection.cs @@ -148,6 +148,11 @@ public class ProjectCollection : IToolsetProvider, IBuildComponent, IDisposable /// private readonly PropertyDictionary _globalProperties; + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + private readonly HashSet _propertiesFromCommandLine; + /// /// The properties representing the environment. /// @@ -341,6 +346,9 @@ public ProjectCollection(IDictionary globalProperties, IEnumerab { _globalProperties = new PropertyDictionary(globalProperties.Count); + // at this stage globalProperties collection contains entries passed from command line (e.g. /p:foo=bar). + _propertiesFromCommandLine = new HashSet(globalProperties.Keys); + foreach (KeyValuePair pair in globalProperties) { try @@ -497,6 +505,11 @@ public static string DisplayVersion } } + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + public HashSet PropertiesFromCommandLine => _propertiesFromCommandLine; + /// /// The default tools version of this project collection. Projects use this tools version if they /// aren't otherwise told what tools version to use. diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 7483294ca70..31a169d5e93 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -157,6 +157,11 @@ internal class Evaluator /// private readonly PropertyDictionary _environmentProperties; + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + private readonly HashSet _propertiesFromCommandLine; + /// /// The cache to consult for any imports that need loading. /// @@ -200,6 +205,7 @@ private Evaluator( ProjectLoadSettings loadSettings, int maxNodeCount, PropertyDictionary environmentProperties, + HashSet propertiesFromCommandLine, IItemFactory itemFactory, IToolsetProvider toolsetProvider, IDirectoryCacheFactory directoryCacheFactory, @@ -252,6 +258,7 @@ private Evaluator( _loadSettings = loadSettings; _maxNodeCount = maxNodeCount; _environmentProperties = environmentProperties; + _propertiesFromCommandLine = propertiesFromCommandLine; _itemFactory = itemFactory; _projectRootElementCache = projectRootElementCache; _sdkResolverService = sdkResolverService; @@ -300,6 +307,7 @@ internal static void Evaluate( ProjectLoadSettings loadSettings, int maxNodeCount, PropertyDictionary environmentProperties, + HashSet propertiesFromCommandLine, ILoggingService loggingService, IItemFactory itemFactory, IToolsetProvider toolsetProvider, @@ -320,6 +328,7 @@ internal static void Evaluate( loadSettings, maxNodeCount, environmentProperties, + propertiesFromCommandLine, itemFactory, toolsetProvider, directoryCacheFactory, @@ -1239,7 +1248,7 @@ private void AddToolsetProperties() } /// - /// Put all the global properties into our property bag + /// Put all the global properties into our property bag. /// private int AddGlobalProperties() { @@ -1250,7 +1259,25 @@ private int AddGlobalProperties() foreach (ProjectPropertyInstance globalProperty in _data.GlobalPropertiesDictionary) { - _data.SetProperty(globalProperty.Name, ((IProperty)globalProperty).EvaluatedValueEscaped, true /* IS global property */, false /* may NOT be a reserved name */, loggingContext: _evaluationLoggingContext); + if ( _propertiesFromCommandLine.Contains(globalProperty.Name)) + { + _ = _data.SetProperty( + globalProperty.Name, + ((IProperty)globalProperty).EvaluatedValueEscaped, + isGlobalProperty: false /* it is a global property, but it comes from command line and is tracked separately */, + false /* may NOT be a reserved name */, + loggingContext: _evaluationLoggingContext, + isCommandLineProperty: true /* IS coming from command line argument */); + } + else + { + _ = _data.SetProperty( + globalProperty.Name, + ((IProperty)globalProperty).EvaluatedValueEscaped, + isGlobalProperty: true /* IS global property */, + false /* may NOT be a reserved name */, + loggingContext: _evaluationLoggingContext); + } } return _data.GlobalPropertiesDictionary.Count; diff --git a/src/Build/Evaluation/IEvaluatorData.cs b/src/Build/Evaluation/IEvaluatorData.cs index 212f446d70f..82ad36d6650 100644 --- a/src/Build/Evaluation/IEvaluatorData.cs +++ b/src/Build/Evaluation/IEvaluatorData.cs @@ -268,7 +268,7 @@ List EvaluatedItemElements /// /// Sets a property which does not come from the Xml. /// - P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, BackEnd.Logging.LoggingContext loggingContext, bool isEnvironmentVariable = false); + P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, BackEnd.Logging.LoggingContext loggingContext, bool isEnvironmentVariable = false, bool isCommandLineProperty = false); /// /// Sets a property which comes from the Xml. diff --git a/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs b/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs index 5d749befd8d..b4685fa40d4 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs @@ -311,9 +311,9 @@ public P SetProperty(ProjectPropertyElement propertyElement, string evaluatedVal return _wrappedData.SetProperty(propertyElement, evaluatedValueEscaped, loggingContext); } - public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable = false) + public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable = false, bool isCommandLineProperty = false) { - return _wrappedData.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, loggingContext: loggingContext); + return _wrappedData.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, loggingContext: loggingContext, isCommandLineProperty); } } } diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index f9843110e34..50a20a8907d 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -87,7 +87,14 @@ public P GetProperty(string name, int startIndex, int endIndex) /// /// Sets a property which does not come from the Xml. /// - public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable = false) + public P SetProperty( + string name, + string evaluatedValueEscaped, + bool isGlobalProperty, + bool mayBeReserved, + LoggingContext loggingContext, + bool isEnvironmentVariable = false, + bool isCommandLineProperty = false) { P? originalProperty = _wrapped.GetProperty(name); P newProperty = _wrapped.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, _evaluationLoggingContext, isEnvironmentVariable); @@ -96,7 +103,7 @@ public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalPro originalProperty, newProperty, null, - this.DeterminePropertySource(isGlobalProperty, mayBeReserved, isEnvironmentVariable), + this.DeterminePropertySource(isGlobalProperty, mayBeReserved, isEnvironmentVariable, isCommandLineProperty), loggingContext); return newProperty; @@ -288,7 +295,7 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl var args = new PropertyInitialValueSetEventArgs( property.Name, property.EvaluatedValue, - location?.LocationString ?? source.ToString(), + source.ToString(), location?.File, location?.Line ?? 0, location?.Column ?? 0, @@ -355,20 +362,15 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat /// /// Determines the source of a property given the variables SetProperty arguments provided. This logic follows what's in . /// - private PropertySource DeterminePropertySource(bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable) - { - if (isEnvironmentVariable) - { - return PropertySource.EnvironmentVariable; - } - - if (isGlobalProperty) + private PropertySource DeterminePropertySource(bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable, bool isCommandLineProperty) => + (isGlobalProperty, mayBeReserved, isEnvironmentVariable, isCommandLineProperty) switch { - return PropertySource.Global; - } - - return mayBeReserved ? PropertySource.BuiltIn : PropertySource.Toolset; - } + (true, _, _, _) => PropertySource.Global, + (_, true, _, _) => PropertySource.BuiltIn, + (_, _, true, _) => PropertySource.EnvironmentVariable, + (_, _, _, true) => PropertySource.CommandLine, + _ => PropertySource.Toolset, + }; #endregion @@ -381,7 +383,8 @@ private enum PropertySource BuiltIn, Global, Toolset, - EnvironmentVariable + EnvironmentVariable, + CommandLine, } } diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index e5eba2b291e..938fcc5cba9 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -1791,7 +1791,7 @@ IItemDefinition IEvaluatorData - ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable) + ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, LoggingContext loggingContext, bool isEnvironmentVariable, bool isCommandLineProperty) { // Mutability not verified as this is being populated during evaluation ProjectPropertyInstance property = ProjectPropertyInstance.Create(name, evaluatedValueEscaped, mayBeReserved, _isImmutable, isEnvironmentVariable, loggingContext); @@ -3175,6 +3175,7 @@ private void Initialize( projectLoadSettings ?? buildParameters.ProjectLoadSettings, /* Use override ProjectLoadSettings if specified */ buildParameters.MaxNodeCount, buildParameters.EnvironmentPropertiesInternal, + buildParameters.PropertiesFromCommandLine, loggingService, new ProjectItemInstanceFactory(this), buildParameters.ToolsetProvider, From b1ed4a411b4555fb3be05aea8948019203db970e Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 9 Dec 2024 15:49:53 +0100 Subject: [PATCH 05/25] fix null ref exception --- src/Build/Evaluation/Evaluator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 31a169d5e93..876604aed05 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -258,7 +258,7 @@ private Evaluator( _loadSettings = loadSettings; _maxNodeCount = maxNodeCount; _environmentProperties = environmentProperties; - _propertiesFromCommandLine = propertiesFromCommandLine; + _propertiesFromCommandLine = propertiesFromCommandLine ?? []; _itemFactory = itemFactory; _projectRootElementCache = projectRootElementCache; _sdkResolverService = sdkResolverService; From 9bd143ea087b294ff303eb6acf8641311a75fcff Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 9 Dec 2024 16:52:35 +0100 Subject: [PATCH 06/25] fix invalid condition for property source --- src/Build/Evaluation/Evaluator.cs | 2 +- .../Evaluation/PropertyTrackingEvaluatorDataWrapper.cs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 876604aed05..c475fca14a1 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1264,7 +1264,7 @@ private int AddGlobalProperties() _ = _data.SetProperty( globalProperty.Name, ((IProperty)globalProperty).EvaluatedValueEscaped, - isGlobalProperty: false /* it is a global property, but it comes from command line and is tracked separately */, + isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, false /* may NOT be a reserved name */, loggingContext: _evaluationLoggingContext, isCommandLineProperty: true /* IS coming from command line argument */); diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 50a20a8907d..e5889735023 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -97,7 +97,7 @@ public P SetProperty( bool isCommandLineProperty = false) { P? originalProperty = _wrapped.GetProperty(name); - P newProperty = _wrapped.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, _evaluationLoggingContext, isEnvironmentVariable); + P newProperty = _wrapped.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved, _evaluationLoggingContext, isEnvironmentVariable, isCommandLineProperty); this.TrackPropertyWrite( originalProperty, @@ -292,6 +292,7 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl { return; } + var args = new PropertyInitialValueSetEventArgs( property.Name, property.EvaluatedValue, @@ -365,10 +366,10 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat private PropertySource DeterminePropertySource(bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable, bool isCommandLineProperty) => (isGlobalProperty, mayBeReserved, isEnvironmentVariable, isCommandLineProperty) switch { - (true, _, _, _) => PropertySource.Global, + (true, _, _, false) => PropertySource.Global, (_, true, _, _) => PropertySource.BuiltIn, (_, _, true, _) => PropertySource.EnvironmentVariable, - (_, _, _, true) => PropertySource.CommandLine, + (true, _, _, true) => PropertySource.CommandLine, _ => PropertySource.Toolset, }; From 923e833a855933156882c719471ad6f76ab40cf6 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Tue, 10 Dec 2024 21:05:02 +0100 Subject: [PATCH 07/25] change the event content to get dedupe functionality --- .../PropertyTrackingEvaluatorDataWrapper.cs | 36 ++++++++++--------- src/Build/Microsoft.Build.csproj | 3 ++ src/Shared/EnumUtilities.cs | 25 +++++++++++++ 3 files changed, 47 insertions(+), 17 deletions(-) create mode 100644 src/Shared/EnumUtilities.cs diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index e5889735023..92eab998994 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -296,17 +296,14 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl var args = new PropertyInitialValueSetEventArgs( property.Name, property.EvaluatedValue, - source.ToString(), + + // If the property is from XML, we don't need property source since a full location is available. + location == null ? EnumUtilities.GetEnumString(source) : string.Empty, location?.File, location?.Line ?? 0, location?.Column ?? 0, - ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword( - "PropertyAssignment", - property.Name, - property.EvaluatedValue, - location?.LocationString ?? source.ToString())); - - args.BuildEventContext = _evaluationLoggingContext.BuildEventContext; + ResourceUtilities.GetResourceString("PropertyAssignment")) + { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); } @@ -343,7 +340,7 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat oldValue, newValue, location?.LocationString, - message: null) + message: ResourceUtilities.GetResourceString("PropertyReassignment")) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); @@ -363,15 +360,20 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat /// /// Determines the source of a property given the variables SetProperty arguments provided. This logic follows what's in . /// - private PropertySource DeterminePropertySource(bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable, bool isCommandLineProperty) => - (isGlobalProperty, mayBeReserved, isEnvironmentVariable, isCommandLineProperty) switch + private PropertySource DeterminePropertySource(bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable, bool isCommandLineProperty) + { + if (isEnvironmentVariable) + { + return PropertySource.EnvironmentVariable; + } + + if (isGlobalProperty) { - (true, _, _, false) => PropertySource.Global, - (_, true, _, _) => PropertySource.BuiltIn, - (_, _, true, _) => PropertySource.EnvironmentVariable, - (true, _, _, true) => PropertySource.CommandLine, - _ => PropertySource.Toolset, - }; + return isCommandLineProperty ? PropertySource.CommandLine : PropertySource.Global; + } + + return mayBeReserved ? PropertySource.BuiltIn : PropertySource.Toolset; + } #endregion diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 7b95b38a8af..51fd327d59e 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -58,6 +58,9 @@ + + SharedUtilities\EnumUtilities.cs + SharedUtilities\EnvironmentUtilities.cs diff --git a/src/Shared/EnumUtilities.cs b/src/Shared/EnumUtilities.cs new file mode 100644 index 00000000000..256e87ef81a --- /dev/null +++ b/src/Shared/EnumUtilities.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; + +namespace Microsoft.Build.Shared +{ + public static class EnumUtilities + { + private static readonly Dictionary _enumStringCache = []; + + public static string GetEnumString(Enum value) + { + if (_enumStringCache.TryGetValue(value, out string? stringValue)) + { + return stringValue; + } + + _enumStringCache[value] = value.ToString(); + + return _enumStringCache[value]; + } + } +} From bf379227390ba4d97619e0bea626a950670de5b9 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Tue, 10 Dec 2024 22:46:15 +0100 Subject: [PATCH 08/25] add extra ctor to PropertyReassignmentEventArgs to improve parsing logic on bin log side --- .../PropertyGroupIntrinsicTask.cs | 14 +++--- .../TaskExecutionHost/TaskExecutionHost.cs | 8 ++-- .../PropertyTrackingEvaluatorDataWrapper.cs | 30 +++++++------ .../Logging/BinaryLogger/BinaryLogger.cs | 4 +- .../BinaryLogger/BuildEventArgsReader.cs | 4 +- .../BinaryLogger/BuildEventArgsWriter.cs | 3 +- .../PropertyInitialValueSetEventArgs.cs | 15 +++++++ .../PropertyReassignmentEventArgs.cs | 43 ++++++++++++++++--- 8 files changed, 90 insertions(+), 31 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index d783deec077..4396758a77e 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -141,7 +141,7 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta property.Location.File, property.Location.Line, property.Location.Column, - ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", property.Name, evaluatedValue, property.Location.LocationString ?? "Target")) + ResourceUtilities.GetResourceString("PropertyAssignment")) { BuildEventContext = LoggingContext.BuildEventContext }; LoggingContext.LogBuildEvent(args); @@ -149,11 +149,13 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta else if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) { var args = new PropertyReassignmentEventArgs( - property.Name, - previousPropertyValue, - evaluatedValue, - property.Location.LocationString, - message: null) + property.Name, + previousPropertyValue, + evaluatedValue, + property.Location.File, + property.Location.Line, + property.Location.Column, + message: ResourceUtilities.GetResourceString("PropertyReassignment")) { BuildEventContext = LoggingContext.BuildEventContext, }; LoggingContext.LogBuildEvent(args); diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 55bc4e6aa2c..4611cca9335 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1617,7 +1617,7 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal location.File, location.Line, location.Column, - ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", propertyName, propertyValue, location?.LocationString ?? "Task")) + ResourceUtilities.GetResourceString("PropertyAssignment")) { BuildEventContext = _targetLoggingContext.BuildEventContext }; _targetLoggingContext.LogBuildEvent(args); @@ -1628,8 +1628,10 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal propertyName, previousPropertyValue, propertyValue, - location.LocationString, - message: null) + location.File, + location.Line, + location.Column, + message: ResourceUtilities.GetResourceString("PropertyReassignment")) { BuildEventContext = _targetLoggingContext.BuildEventContext }; _targetLoggingContext.LogBuildEvent(args); diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 92eab998994..16c634d6aab 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -294,15 +294,15 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl } var args = new PropertyInitialValueSetEventArgs( - property.Name, - property.EvaluatedValue, - - // If the property is from XML, we don't need property source since a full location is available. - location == null ? EnumUtilities.GetEnumString(source) : string.Empty, - location?.File, - location?.Line ?? 0, - location?.Column ?? 0, - ResourceUtilities.GetResourceString("PropertyAssignment")) + property.Name, + property.EvaluatedValue, + + // If the property is from XML, we don't need property source since a full location is available. + location == null ? EnumUtilities.GetEnumString(source) : string.Empty, + location?.File, + location?.Line ?? 0, + location?.Column ?? 0, + ResourceUtilities.GetResourceString("PropertyAssignment")) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); @@ -336,11 +336,13 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat (_settings == 0 && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10))) { var args = new PropertyReassignmentEventArgs( - property.Name, - oldValue, - newValue, - location?.LocationString, - message: ResourceUtilities.GetResourceString("PropertyReassignment")) + property.Name, + oldValue, + newValue, + location?.File, + location?.Line ?? 0, + location?.Column ?? 0, + message: ResourceUtilities.GetResourceString("PropertyReassignment")) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); diff --git a/src/Build/Logging/BinaryLogger/BinaryLogger.cs b/src/Build/Logging/BinaryLogger/BinaryLogger.cs index be4eaa2288d..adadde7eb8b 100644 --- a/src/Build/Logging/BinaryLogger/BinaryLogger.cs +++ b/src/Build/Logging/BinaryLogger/BinaryLogger.cs @@ -80,6 +80,8 @@ public sealed class BinaryLogger : ILogger // BuildCheckTracingEvent, BuildCheckAcquisitionEvent, BuildSubmissionStartedEvent // version 24: // - new record kind: BuildCanceledEventArgs + // version 25: + // - add extra information to PropertyInitialValueSetEventArgs and PropertyReassignmentEventArgs and change message formatting logic. // MAKE SURE YOU KEEP BuildEventArgsWriter AND StructuredLogViewer.BuildEventArgsWriter IN SYNC WITH THE CHANGES ABOVE. // Both components must stay in sync to avoid issues with logging or event handling in the products. @@ -90,7 +92,7 @@ public sealed class BinaryLogger : ILogger // The current version of the binary log representation. // Changes with each update of the binary log format. - internal const int FileFormatVersion = 24; + internal const int FileFormatVersion = 25; // The minimum version of the binary log reader that can read log of above version. // This should be changed only when the binary log format is changed in a way that would prevent it from being diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 8e5ad536b0b..4b83547648a 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -1177,7 +1177,9 @@ private BuildEventArgs ReadPropertyReassignmentEventArgs() propertyName, previousValue, newValue, - location, + fields.File, + fields.LineNumber, + fields.ColumnNumber, fields.Message, fields.HelpKeyword, fields.SenderName, diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs index dba52023339..e6eeb51befd 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs @@ -567,11 +567,12 @@ private BinaryLogRecordKind Write(CriticalBuildMessageEventArgs e) private BinaryLogRecordKind Write(PropertyReassignmentEventArgs e) { - WriteMessageFields(e, writeMessage: false, writeImportance: true); + WriteMessageFields(e, writeMessage: true, writeImportance: true); WriteDeduplicatedString(e.PropertyName); WriteDeduplicatedString(e.PreviousValue); WriteDeduplicatedString(e.NewValue); WriteDeduplicatedString(e.Location); + return BinaryLogRecordKind.PropertyReassignment; } diff --git a/src/Framework/PropertyInitialValueSetEventArgs.cs b/src/Framework/PropertyInitialValueSetEventArgs.cs index e9b7d91666f..f258ca86d85 100644 --- a/src/Framework/PropertyInitialValueSetEventArgs.cs +++ b/src/Framework/PropertyInitialValueSetEventArgs.cs @@ -91,6 +91,21 @@ public PropertyInitialValueSetEventArgs( /// public string PropertySource { get; set; } + public override string Message + { + get + { + if (RawMessage == null) + { + var formattedSource = File != null ? $"{File} ({LineNumber}, {ColumnNumber})" : PropertySource; + RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", PropertyName, PropertyValue, formattedSource); + } + + return RawMessage; + } + } + + internal override void WriteToStream(BinaryWriter writer) { base.WriteToStream(writer); diff --git a/src/Framework/PropertyReassignmentEventArgs.cs b/src/Framework/PropertyReassignmentEventArgs.cs index 29ec2935e0b..64543af06c1 100644 --- a/src/Framework/PropertyReassignmentEventArgs.cs +++ b/src/Framework/PropertyReassignmentEventArgs.cs @@ -41,12 +41,45 @@ public PropertyReassignmentEventArgs( string message, string helpKeyword = null, string senderName = null, - MessageImportance importance = MessageImportance.Low) : base(message, helpKeyword, senderName, importance) + MessageImportance importance = MessageImportance.Low) + : base(message, helpKeyword, senderName, importance) { - this.PropertyName = propertyName; - this.PreviousValue = previousValue; - this.NewValue = newValue; - this.Location = location; + PropertyName = propertyName; + PreviousValue = previousValue; + NewValue = newValue; + Location = location; + } + + /// + /// Creates an instance of the class. + /// + /// The name of the property whose value was reassigned. + /// The previous value of the reassigned property. + /// The new value of the reassigned property. + /// The file associated with the event. + /// The line number (0 if not applicable). + /// The column number (0 if not applicable). + /// The message of the property. + /// The help keyword. + /// The sender name of the event. + /// The importance of the message. + public PropertyReassignmentEventArgs( + string propertyName, + string previousValue, + string newValue, + string file, + int line, + int column, + string message, + string helpKeyword = null, + string senderName = null, + MessageImportance importance = MessageImportance.Low) + : base(subcategory: null, code: null, file: file, lineNumber: line, columnNumber: column, 0, 0, message, helpKeyword, senderName, importance) + { + PropertyName = propertyName; + PreviousValue = previousValue; + NewValue = newValue; + Location = $"{file} ({line}, {column})"; } /// From 7da96c34e7eae1596bd86a05dce7c691f361b11d Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Tue, 10 Dec 2024 23:04:49 +0100 Subject: [PATCH 09/25] change location formatting --- src/Framework/PropertyInitialValueSetEventArgs.cs | 2 +- src/Framework/PropertyReassignmentEventArgs.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Framework/PropertyInitialValueSetEventArgs.cs b/src/Framework/PropertyInitialValueSetEventArgs.cs index f258ca86d85..0829012bb4f 100644 --- a/src/Framework/PropertyInitialValueSetEventArgs.cs +++ b/src/Framework/PropertyInitialValueSetEventArgs.cs @@ -97,7 +97,7 @@ public override string Message { if (RawMessage == null) { - var formattedSource = File != null ? $"{File} ({LineNumber}, {ColumnNumber})" : PropertySource; + var formattedSource = File != null ? $"{File} ({LineNumber},{ColumnNumber})" : PropertySource; RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", PropertyName, PropertyValue, formattedSource); } diff --git a/src/Framework/PropertyReassignmentEventArgs.cs b/src/Framework/PropertyReassignmentEventArgs.cs index 64543af06c1..2fb5b8153a0 100644 --- a/src/Framework/PropertyReassignmentEventArgs.cs +++ b/src/Framework/PropertyReassignmentEventArgs.cs @@ -79,7 +79,6 @@ public PropertyReassignmentEventArgs( PropertyName = propertyName; PreviousValue = previousValue; NewValue = newValue; - Location = $"{file} ({line}, {column})"; } /// @@ -108,7 +107,8 @@ public override string Message { if (RawMessage == null) { - RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyReassignment", PropertyName, NewValue, PreviousValue, Location); + string formattedLocation = File != null ? $"{File} ({LineNumber},{ColumnNumber})" : Location; + RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyReassignment", PropertyName, NewValue, PreviousValue, formattedLocation); } return RawMessage; From e7f4df1e927667442944dbd84774629b08c71864 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Wed, 11 Dec 2024 10:48:02 +0100 Subject: [PATCH 10/25] adjust the tests to the code changes --- .../BuildEventArgsSerialization_Tests.cs | 6 ++-- .../Evaluation/Evaluator_Tests.cs | 34 +++++++++---------- .../PropertyInitialValueSetEventArgs.cs | 10 ++---- .../PropertyReassignmentEventArgs.cs | 9 ++--- 4 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index e18407ee9ed..c9e11f39a03 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -881,8 +881,10 @@ public void RoundTripPropertyReassignmentEventArgs() propertyName: "a", previousValue: "b", newValue: "c", - location: "d", - message: "Property reassignment: $(a)=\"c\" (previous value: \"b\") at d", + file: "file.cs", + line: 10, + column: 20, + message: "Property reassignment: $(a)=\"c\" (previous value: \"b\") at file.cs (10,20)", helpKeyword: "e", senderName: "f"); diff --git a/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs b/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs index 4f9b2b14f92..8f53177e30f 100644 --- a/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs +++ b/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs @@ -4708,7 +4708,7 @@ public void VerifyPropertyTrackingLoggingDefault() // Having just environment variables defined should default to nothing being logged except one environment variable read. VerifyPropertyTrackingLoggingScenario( null, - logger => + (logger, _) => { logger .AllBuildEvents @@ -4740,7 +4740,7 @@ public void VerifyPropertyTrackingLoggingPropertyReassignment() { VerifyPropertyTrackingLoggingScenario( "1", - logger => + (logger, _) => { logger .AllBuildEvents @@ -4771,7 +4771,7 @@ public void VerifyPropertyTrackingLoggingNone() { this.VerifyPropertyTrackingLoggingScenario( "0", - logger => + (logger, _) => { logger .AllBuildEvents @@ -4803,7 +4803,7 @@ public void VerifyPropertyTrackingLoggingPropertyInitialValue() { this.VerifyPropertyTrackingLoggingScenario( "2", - logger => + (logger, projectPath) => { logger .AllBuildEvents @@ -4829,11 +4829,11 @@ public void VerifyPropertyTrackingLoggingPropertyInitialValue() // Verify logging of property initial values. propertyInitialValueMap.ShouldContainKey("Prop"); - propertyInitialValueMap["Prop"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["Prop"].File.ShouldBe(projectPath); propertyInitialValueMap["Prop"].PropertyValue.ShouldBe(string.Empty); propertyInitialValueMap.ShouldContainKey("EnvVar"); - propertyInitialValueMap["EnvVar"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["EnvVar"].File.ShouldBe(projectPath); propertyInitialValueMap["EnvVar"].PropertyValue.ShouldBe("It's also Defined!"); propertyInitialValueMap.ShouldContainKey("DEFINED_ENVIRONMENT_VARIABLE"); @@ -4841,11 +4841,11 @@ public void VerifyPropertyTrackingLoggingPropertyInitialValue() propertyInitialValueMap["DEFINED_ENVIRONMENT_VARIABLE"].PropertyValue.ShouldBe("It's Defined!"); propertyInitialValueMap.ShouldContainKey("NotEnvVarRead"); - propertyInitialValueMap["NotEnvVarRead"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["NotEnvVarRead"].File.ShouldBe(projectPath); propertyInitialValueMap["NotEnvVarRead"].PropertyValue.ShouldBe("Overwritten!"); propertyInitialValueMap.ShouldContainKey("Prop2"); - propertyInitialValueMap["Prop2"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["Prop2"].File.ShouldBe(projectPath); propertyInitialValueMap["Prop2"].PropertyValue.ShouldBe("Value1"); }); } @@ -4855,7 +4855,7 @@ public void VerifyPropertyTrackingLoggingEnvironmentVariableRead() { this.VerifyPropertyTrackingLoggingScenario( "4", - logger => + (logger, _) => { logger .AllBuildEvents @@ -4889,7 +4889,7 @@ public void VerifyPropertyTrackingLoggingUninitializedPropertyRead() { this.VerifyPropertyTrackingLoggingScenario( "8", - logger => + (logger, _) => { logger .AllBuildEvents @@ -4920,7 +4920,7 @@ public void VerifyPropertyTrackingLoggingAll() { this.VerifyPropertyTrackingLoggingScenario( "15", - logger => + (logger, projectPath) => { logger .AllBuildEvents @@ -4949,11 +4949,11 @@ public void VerifyPropertyTrackingLoggingAll() // Verify logging of property initial values. propertyInitialValueMap.ShouldContainKey("Prop"); - propertyInitialValueMap["Prop"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["Prop"].File.ShouldBe(projectPath); propertyInitialValueMap["Prop"].PropertyValue.ShouldBe(string.Empty); propertyInitialValueMap.ShouldContainKey("EnvVar"); - propertyInitialValueMap["EnvVar"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["EnvVar"].File.ShouldBe(projectPath); propertyInitialValueMap["EnvVar"].PropertyValue.ShouldBe("It's also Defined!"); propertyInitialValueMap.ShouldContainKey("DEFINED_ENVIRONMENT_VARIABLE"); @@ -4961,11 +4961,11 @@ public void VerifyPropertyTrackingLoggingAll() propertyInitialValueMap["DEFINED_ENVIRONMENT_VARIABLE"].PropertyValue.ShouldBe("It's Defined!"); propertyInitialValueMap.ShouldContainKey("NotEnvVarRead"); - propertyInitialValueMap["NotEnvVarRead"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["NotEnvVarRead"].File.ShouldBe(projectPath); propertyInitialValueMap["NotEnvVarRead"].PropertyValue.ShouldBe("Overwritten!"); propertyInitialValueMap.ShouldContainKey("Prop2"); - propertyInitialValueMap["Prop2"].PropertySource.ShouldBe("Xml"); + propertyInitialValueMap["Prop2"].File.ShouldBe(projectPath); propertyInitialValueMap["Prop2"].PropertyValue.ShouldBe("Value1"); }); } @@ -4987,7 +4987,7 @@ public void VerifyGetTypeEvaluationBlocked() new Project(XmlReader.Create(new StringReader(projectContents)), null, "Fake", fakeProjectCollection)); } - private void VerifyPropertyTrackingLoggingScenario(string envVarValue, Action loggerEvaluatorAction) + private void VerifyPropertyTrackingLoggingScenario(string envVarValue, Action loggerEvaluatorAction) { // The default is that only reassignments are logged. @@ -5026,7 +5026,7 @@ private void VerifyPropertyTrackingLoggingScenario(string envVarValue, Action Date: Wed, 11 Dec 2024 16:40:42 +0100 Subject: [PATCH 11/25] remove extra using --- src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 4b83547648a..9f970137dbd 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -4,7 +4,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Data.Common; using System.IO; using System.IO.Compression; using System.Linq; From 6ed8628e116e97c737848bc24b5db474b5579c08 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:55:53 +0100 Subject: [PATCH 12/25] Update src/Build/Evaluation/Evaluator.cs Co-authored-by: Jan Krivanek --- src/Build/Evaluation/Evaluator.cs | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index c475fca14a1..7e6eddc236e 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1259,25 +1259,13 @@ private int AddGlobalProperties() foreach (ProjectPropertyInstance globalProperty in _data.GlobalPropertiesDictionary) { - if ( _propertiesFromCommandLine.Contains(globalProperty.Name)) - { - _ = _data.SetProperty( - globalProperty.Name, - ((IProperty)globalProperty).EvaluatedValueEscaped, - isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, - false /* may NOT be a reserved name */, - loggingContext: _evaluationLoggingContext, - isCommandLineProperty: true /* IS coming from command line argument */); - } - else - { - _ = _data.SetProperty( - globalProperty.Name, - ((IProperty)globalProperty).EvaluatedValueEscaped, - isGlobalProperty: true /* IS global property */, - false /* may NOT be a reserved name */, - loggingContext: _evaluationLoggingContext); - } + _ = _data.SetProperty( + globalProperty.Name, + ((IProperty)globalProperty).EvaluatedValueEscaped, + isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, + false /* may NOT be a reserved name */, + loggingContext: _evaluationLoggingContext, + isCommandLineProperty: _propertiesFromCommandLine.Contains(globalProperty.Name) /* IS coming from command line argument */); } return _data.GlobalPropertiesDictionary.Count; From 9964cb96f44f5352e76683108506e4653d513668 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Wed, 22 Jan 2025 11:15:31 +0100 Subject: [PATCH 13/25] remove changewave check and enumutils --- .../PropertyGroupIntrinsicTask.cs | 2 +- .../TaskExecutionHost/TaskExecutionHost.cs | 2 +- .../PropertyTrackingEvaluatorDataWrapper.cs | 16 +++++++++--- src/Build/Microsoft.Build.csproj | 3 --- src/Shared/EnumUtilities.cs | 25 ------------------- 5 files changed, 15 insertions(+), 33 deletions(-) delete mode 100644 src/Shared/EnumUtilities.cs diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 4396758a77e..ad4fae9c692 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -124,7 +124,7 @@ internal override void ExecuteTask(Lookup lookup) /// The new evaluated value of the property. private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInstance property, string evaluatedValue) { - if (_propertyTrackingSettings == 0 || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) + if (_propertyTrackingSettings == 0) { return; } diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 4611cca9335..71def813d93 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1601,7 +1601,7 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou /// The source location where the property assignment occurs. private void LogPropertyInTaskAssignment(string propertyName, string propertyValue, IElementLocation location) { - if (_propertyTrackingSettings == 0 || !ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) + if (_propertyTrackingSettings == 0) { return; } diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 16c634d6aab..a60e16e07e0 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -298,7 +298,7 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl property.EvaluatedValue, // If the property is from XML, we don't need property source since a full location is available. - location == null ? EnumUtilities.GetEnumString(source) : string.Empty, + location == null ? GetPropertySourceName(source) : string.Empty, location?.File, location?.Line ?? 0, location?.Column ?? 0, @@ -332,8 +332,7 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat // Either we want to specifically track property reassignments // or we do not want to track nothing - in which case the prop reassignment is enabled by default. - if ((_settings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment || - (_settings == 0 && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10))) + if (_settings == 0 || (_settings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) { var args = new PropertyReassignmentEventArgs( property.Name, @@ -391,6 +390,17 @@ private enum PropertySource EnvironmentVariable, CommandLine, } + + private static string GetPropertySourceName(PropertySource source) => source switch + { + PropertySource.Xml => "XML", + PropertySource.BuiltIn => "Built-in", + PropertySource.Global => "Global", + PropertySource.Toolset => "Toolset", + PropertySource.EnvironmentVariable => "Environment Variable", + PropertySource.CommandLine => "Command Line", + _ => throw new ArgumentOutOfRangeException(nameof(source), source, null) + }; } [Flags] diff --git a/src/Build/Microsoft.Build.csproj b/src/Build/Microsoft.Build.csproj index 618272429c2..66d6c38d007 100644 --- a/src/Build/Microsoft.Build.csproj +++ b/src/Build/Microsoft.Build.csproj @@ -59,9 +59,6 @@ - - SharedUtilities\EnumUtilities.cs - SharedUtilities\EnvironmentUtilities.cs diff --git a/src/Shared/EnumUtilities.cs b/src/Shared/EnumUtilities.cs deleted file mode 100644 index 256e87ef81a..00000000000 --- a/src/Shared/EnumUtilities.cs +++ /dev/null @@ -1,25 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; - -namespace Microsoft.Build.Shared -{ - public static class EnumUtilities - { - private static readonly Dictionary _enumStringCache = []; - - public static string GetEnumString(Enum value) - { - if (_enumStringCache.TryGetValue(value, out string? stringValue)) - { - return stringValue; - } - - _enumStringCache[value] = value.ToString(); - - return _enumStringCache[value]; - } - } -} From 75fe7843e1afe61b434319b12497b3718610aa9e Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 27 Jan 2025 12:16:57 +0100 Subject: [PATCH 14/25] fix review comments --- .../BuildEventArgsSerialization_Tests.cs | 1 + .../IntrinsicTasks/PropertyGroupIntrinsicTask.cs | 5 +++-- .../BackEnd/TaskExecutionHost/TaskExecutionHost.cs | 5 +++-- .../PropertyTrackingEvaluatorDataWrapper.cs | 11 ++++++----- .../Logging/BinaryLogger/BuildEventArgsReader.cs | 1 + .../Logging/BinaryLogger/BuildEventArgsWriter.cs | 4 ++-- src/Framework/PropertyInitialValueSetEventArgs.cs | 9 +++++++-- src/Framework/PropertyReassignmentEventArgs.cs | 12 ++++++++++-- 8 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index af9437b5cf3..3689868f212 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -881,6 +881,7 @@ public void RoundTripPropertyReassignmentEventArgs() propertyName: "a", previousValue: "b", newValue: "c", + location: null, file: "file.cs", line: 10, column: 20, diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index ad4fae9c692..4a2e3f2078e 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -141,7 +141,7 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta property.Location.File, property.Location.Line, property.Location.Column, - ResourceUtilities.GetResourceString("PropertyAssignment")) + message: null) { BuildEventContext = LoggingContext.BuildEventContext }; LoggingContext.LogBuildEvent(args); @@ -152,10 +152,11 @@ private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInsta property.Name, previousPropertyValue, evaluatedValue, + location: null, property.Location.File, property.Location.Line, property.Location.Column, - message: ResourceUtilities.GetResourceString("PropertyReassignment")) + message: null) { BuildEventContext = LoggingContext.BuildEventContext, }; LoggingContext.LogBuildEvent(args); diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 71def813d93..92e48b19885 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1617,7 +1617,7 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal location.File, location.Line, location.Column, - ResourceUtilities.GetResourceString("PropertyAssignment")) + message: null) { BuildEventContext = _targetLoggingContext.BuildEventContext }; _targetLoggingContext.LogBuildEvent(args); @@ -1628,10 +1628,11 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal propertyName, previousPropertyValue, propertyValue, + location: null, location.File, location.Line, location.Column, - message: ResourceUtilities.GetResourceString("PropertyReassignment")) + message: null) { BuildEventContext = _targetLoggingContext.BuildEventContext }; _targetLoggingContext.LogBuildEvent(args); diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index a60e16e07e0..b9dee42bbc5 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -302,7 +302,7 @@ private void TrackPropertyInitialValueSet(P property, PropertySource source, IEl location?.File, location?.Line ?? 0, location?.Column ?? 0, - ResourceUtilities.GetResourceString("PropertyAssignment")) + message: null) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); @@ -338,10 +338,11 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat property.Name, oldValue, newValue, + location: null, location?.File, location?.Line ?? 0, location?.Column ?? 0, - message: ResourceUtilities.GetResourceString("PropertyReassignment")) + message: null) { BuildEventContext = _evaluationLoggingContext.BuildEventContext, }; _evaluationLoggingContext.LogBuildEvent(args); @@ -394,11 +395,11 @@ private enum PropertySource private static string GetPropertySourceName(PropertySource source) => source switch { PropertySource.Xml => "XML", - PropertySource.BuiltIn => "Built-in", + PropertySource.BuiltIn => "BuiltIn", PropertySource.Global => "Global", PropertySource.Toolset => "Toolset", - PropertySource.EnvironmentVariable => "Environment Variable", - PropertySource.CommandLine => "Command Line", + PropertySource.EnvironmentVariable => "EnvironmentVariable", + PropertySource.CommandLine => "CommandLine", _ => throw new ArgumentOutOfRangeException(nameof(source), source, null) }; } diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 9f970137dbd..81ee91b3a14 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -1176,6 +1176,7 @@ private BuildEventArgs ReadPropertyReassignmentEventArgs() propertyName, previousValue, newValue, + location, fields.File, fields.LineNumber, fields.ColumnNumber, diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs index e6eeb51befd..e601be70919 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs @@ -567,7 +567,7 @@ private BinaryLogRecordKind Write(CriticalBuildMessageEventArgs e) private BinaryLogRecordKind Write(PropertyReassignmentEventArgs e) { - WriteMessageFields(e, writeMessage: true, writeImportance: true); + WriteMessageFields(e, writeMessage: false, writeImportance: true); WriteDeduplicatedString(e.PropertyName); WriteDeduplicatedString(e.PreviousValue); WriteDeduplicatedString(e.NewValue); @@ -585,7 +585,7 @@ private BinaryLogRecordKind Write(UninitializedPropertyReadEventArgs e) private BinaryLogRecordKind Write(PropertyInitialValueSetEventArgs e) { - WriteMessageFields(e, writeImportance: true); + WriteMessageFields(e, writeMessage: false, writeImportance: true); WriteDeduplicatedString(e.PropertyName); WriteDeduplicatedString(e.PropertyValue); WriteDeduplicatedString(e.PropertySource); diff --git a/src/Framework/PropertyInitialValueSetEventArgs.cs b/src/Framework/PropertyInitialValueSetEventArgs.cs index ca9c20ef8da..fe1e52023b6 100644 --- a/src/Framework/PropertyInitialValueSetEventArgs.cs +++ b/src/Framework/PropertyInitialValueSetEventArgs.cs @@ -95,8 +95,13 @@ public override string Message { get { - var formattedSource = File != null ? $"{File} ({LineNumber},{ColumnNumber})" : PropertySource; - return string.Format(RawMessage, PropertyName, PropertyValue, formattedSource); + if (RawMessage == null) + { + string formattedLocation = File == null ? PropertySource : $"{File} ({LineNumber},{ColumnNumber})"; + RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyAssignment", PropertyName, PropertyValue, formattedLocation); + } + + return RawMessage; } } diff --git a/src/Framework/PropertyReassignmentEventArgs.cs b/src/Framework/PropertyReassignmentEventArgs.cs index 64d48ecf6a9..d7477ee8caa 100644 --- a/src/Framework/PropertyReassignmentEventArgs.cs +++ b/src/Framework/PropertyReassignmentEventArgs.cs @@ -56,6 +56,7 @@ public PropertyReassignmentEventArgs( /// The name of the property whose value was reassigned. /// The previous value of the reassigned property. /// The new value of the reassigned property. + /// The property location (XML, command line, etc). /// The file associated with the event. /// The line number (0 if not applicable). /// The column number (0 if not applicable). @@ -67,6 +68,7 @@ public PropertyReassignmentEventArgs( string propertyName, string previousValue, string newValue, + string location, string file, int line, int column, @@ -79,6 +81,7 @@ public PropertyReassignmentEventArgs( PropertyName = propertyName; PreviousValue = previousValue; NewValue = newValue; + Location = location; } /// @@ -105,8 +108,13 @@ public override string Message { get { - string formattedLocation = File != null ? $"{File} ({LineNumber},{ColumnNumber})" : Location; - return string.Format(RawMessage, PropertyName, NewValue, PreviousValue, formattedLocation); + if (RawMessage == null) + { + string formattedLocation = File == null ? Location : $"{File} ({LineNumber},{ColumnNumber})"; + RawMessage = FormatResourceStringIgnoreCodeAndKeyword("PropertyReassignment", PropertyName, NewValue, PreviousValue, formattedLocation); + } + + return RawMessage; } } From aa62e8cdd84f4f53a1376a0ea36f02e5b65b80df Mon Sep 17 00:00:00 2001 From: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com> Date: Mon, 27 Jan 2025 21:47:22 +0100 Subject: [PATCH 15/25] fix the test --- .../BuildEventArgsSerialization_Tests.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index 3689868f212..e710de04938 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Text; using FluentAssertions; +using FluentAssertions.Equivalency; using Microsoft.Build.BackEnd; using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; @@ -919,17 +920,22 @@ public void UninitializedPropertyReadEventArgs() public void PropertyInitialValueEventArgs() { var args = new PropertyInitialValueSetEventArgs( - propertyName: Guid.NewGuid().ToString(), - propertyValue: Guid.NewGuid().ToString(), - propertySource: Guid.NewGuid().ToString(), - message: Guid.NewGuid().ToString(), + propertyName: "a", + propertyValue: "b", + propertySource: null, + file: "file.cs", + line: 10, + column: 20, + message: "Property initial value: $(a)=\"b\" Source: file.cs (10,20)", helpKeyword: Guid.NewGuid().ToString(), senderName: Guid.NewGuid().ToString()); Roundtrip(args, e => e.PropertyName, e => e.PropertyValue, - e => e.PropertySource, + e => e.File, + e => e.LineNumber.ToString(), + e => e.ColumnNumber.ToString(), e => e.Message, e => e.HelpKeyword, e => e.SenderName); From ee58f1899d03d702f531bd53e4ec9a45c8eef596 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova <95473390+YuliiaKovalova@users.noreply.github.com> Date: Tue, 28 Jan 2025 11:42:05 +0100 Subject: [PATCH 16/25] apply message optimization for UninitializedPropertyRead --- .../BuildEventArgsSerialization_Tests.cs | 4 ++-- .../PropertyTrackingEvaluatorDataWrapper.cs | 8 ++++---- .../Logging/BinaryLogger/BuildEventArgsReader.cs | 2 +- .../Logging/BinaryLogger/BuildEventArgsWriter.cs | 3 ++- .../UninitializedPropertyReadEventArgs.cs | 15 +++++++++++++++ 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index e710de04938..5d25dad5d68 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -904,8 +904,8 @@ public void RoundTripPropertyReassignmentEventArgs() public void UninitializedPropertyReadEventArgs() { var args = new UninitializedPropertyReadEventArgs( - propertyName: Guid.NewGuid().ToString(), - message: Guid.NewGuid().ToString(), + propertyName: "a", + message: "Read uninitialized property \"a\"", helpKeyword: Guid.NewGuid().ToString(), senderName: Guid.NewGuid().ToString()); diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index b9dee42bbc5..afccc78c9d8 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -243,10 +243,10 @@ private void TrackUninitializedPropertyRead(string name) return; } - var args = new UninitializedPropertyReadEventArgs( - name, - ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("UninitializedPropertyRead", name)); - args.BuildEventContext = _evaluationLoggingContext.BuildEventContext; + var args = new UninitializedPropertyReadEventArgs(name, message: null) + { + BuildEventContext = _evaluationLoggingContext.BuildEventContext, + }; _evaluationLoggingContext.LogBuildEvent(args); } diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs index 81ee91b3a14..4b48d9a8592 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsReader.cs @@ -1196,7 +1196,7 @@ private BuildEventArgs ReadUninitializedPropertyReadEventArgs() var e = new UninitializedPropertyReadEventArgs( propertyName, - fields.Message, + message: null, fields.HelpKeyword, fields.SenderName, fields.Importance); diff --git a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs index e601be70919..fc44128aba0 100644 --- a/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs +++ b/src/Build/Logging/BinaryLogger/BuildEventArgsWriter.cs @@ -578,8 +578,9 @@ private BinaryLogRecordKind Write(PropertyReassignmentEventArgs e) private BinaryLogRecordKind Write(UninitializedPropertyReadEventArgs e) { - WriteMessageFields(e, writeImportance: true); + WriteMessageFields(e, writeMessage: false, writeImportance: true); WriteDeduplicatedString(e.PropertyName); + return BinaryLogRecordKind.UninitializedPropertyRead; } diff --git a/src/Framework/UninitializedPropertyReadEventArgs.cs b/src/Framework/UninitializedPropertyReadEventArgs.cs index 781c8c33bc8..3c89e480e78 100644 --- a/src/Framework/UninitializedPropertyReadEventArgs.cs +++ b/src/Framework/UninitializedPropertyReadEventArgs.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Xml.Linq; using Microsoft.Build.Shared; #nullable disable @@ -51,11 +52,25 @@ internal override void WriteToStream(BinaryWriter writer) writer.WriteOptionalString(PropertyName); } + internal override void CreateFromStream(BinaryReader reader, int version) { base.CreateFromStream(reader, version); PropertyName = reader.ReadOptionalString(); } + + public override string Message + { + get + { + if (RawMessage == null) + { + RawMessage = FormatResourceStringIgnoreCodeAndKeyword("UninitializedPropertyRead", PropertyName); + } + + return RawMessage; + } + } } } From 9a5ad3ced31fc69bb0f366f8fb73db03848dc130 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Fri, 31 Jan 2025 16:03:31 +0100 Subject: [PATCH 17/25] fix review comments --- .../BuildEventArgsSerialization_Tests.cs | 1 - .../RequestBuilder/IntrinsicTask.cs | 2 +- .../PropertyGroupIntrinsicTask.cs | 53 +---------- .../TaskExecutionHost/TaskExecutionHost.cs | 9 +- src/Build/Definition/ProjectCollection.cs | 9 +- src/Build/Evaluation/Evaluator.cs | 14 +-- .../PropertyTrackingEvaluatorDataWrapper.cs | 90 ++++++++++++++++--- .../UninitializedPropertyReadEventArgs.cs | 1 - 8 files changed, 97 insertions(+), 82 deletions(-) diff --git a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs index 5d25dad5d68..2eef5fde018 100644 --- a/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs +++ b/src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs @@ -9,7 +9,6 @@ using System.Linq; using System.Text; using FluentAssertions; -using FluentAssertions.Equivalency; using Microsoft.Build.BackEnd; using Microsoft.Build.Experimental.BuildCheck; using Microsoft.Build.Framework; diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs index 68a82e27491..a5d7ea5622c 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs @@ -70,7 +70,7 @@ internal static IntrinsicTask InstantiateTask(ProjectTargetInstanceChild taskIns { if (taskInstance is ProjectPropertyGroupTaskInstance propertyGroupTaskInstance) { - return new PropertyGroupIntrinsicTask(propertyGroupTaskInstance, loggingContext, projectInstance, logTaskInputs, Traits.Instance.LogPropertyTracking); + return new PropertyGroupIntrinsicTask(propertyGroupTaskInstance, loggingContext, projectInstance, logTaskInputs); } else if (taskInstance is ProjectItemGroupTaskInstance itemGroupTaskInstance) { diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 4a2e3f2078e..89745b5aab9 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -34,12 +34,11 @@ internal class PropertyGroupIntrinsicTask : IntrinsicTask /// The logging context /// The project instance /// Flag to determine whether or not to log task inputs. - /// Flag to determine whether or not property tracking enabled. - public PropertyGroupIntrinsicTask(ProjectPropertyGroupTaskInstance taskInstance, TargetLoggingContext loggingContext, ProjectInstance projectInstance, bool logTaskInputs, int settingValue) + public PropertyGroupIntrinsicTask(ProjectPropertyGroupTaskInstance taskInstance, TargetLoggingContext loggingContext, ProjectInstance projectInstance, bool logTaskInputs) : base(loggingContext, projectInstance, logTaskInputs) { _taskInstance = taskInstance; - _propertyTrackingSettings = (PropertyTrackingSetting)settingValue; + _propertyTrackingSettings = (PropertyTrackingSetting)Traits.Instance.LogPropertyTracking; } /// @@ -89,7 +88,7 @@ internal override void ExecuteTask(Lookup lookup) string evaluatedValue = bucket.Expander.ExpandIntoStringLeaveEscaped(property.Value, ExpanderOptions.ExpandAll, property.Location); bucket.Expander.PropertiesUseTracker.CheckPreexistingUndefinedUsage(property, evaluatedValue, LoggingContext); - LogPropertyInTargetAssignment(property, evaluatedValue); + PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, property.Name, evaluatedValue, property.Location, Project.GetProperty(property.Name)?.EvaluatedValue ?? null, LoggingContext); if (LogTaskInputs && !LoggingContext.LoggingService.OnlyLogCriticalEvents) { @@ -117,52 +116,6 @@ internal override void ExecuteTask(Lookup lookup) } } - /// - /// Logs property assignment information during target execution, providing detailed tracking of property value changes. - /// - /// The property instance being assigned or modified. - /// The new evaluated value of the property. - private void LogPropertyInTargetAssignment(ProjectPropertyGroupTaskPropertyInstance property, string evaluatedValue) - { - if (_propertyTrackingSettings == 0) - { - return; - } - - var previousPropertyValue = Project.GetProperty(property.Name)?.EvaluatedValue; - - if (previousPropertyValue == null - && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyInitialValueSet) - { - var args = new PropertyInitialValueSetEventArgs( - property.Name, - evaluatedValue, - propertySource: string.Empty, - property.Location.File, - property.Location.Line, - property.Location.Column, - message: null) - { BuildEventContext = LoggingContext.BuildEventContext }; - - LoggingContext.LogBuildEvent(args); - } - else if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) - { - var args = new PropertyReassignmentEventArgs( - property.Name, - previousPropertyValue, - evaluatedValue, - location: null, - property.Location.File, - property.Location.Line, - property.Location.Column, - message: null) - { BuildEventContext = LoggingContext.BuildEventContext, }; - - LoggingContext.LogBuildEvent(args); - } - } - /// /// Adds batchable parameters from a property element into the list. If the property element was /// a task, these would be its raw parameter values. diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 92e48b19885..8e9ead325fd 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1586,7 +1586,8 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } - LogPropertyInTaskAssignment(outputTargetName, outputString, parameterLocation); + PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, outputTargetName, outputString, parameterLocation, _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, _targetLoggingContext); + _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable)); } } @@ -1601,14 +1602,14 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou /// The source location where the property assignment occurs. private void LogPropertyInTaskAssignment(string propertyName, string propertyValue, IElementLocation location) { - if (_propertyTrackingSettings == 0) + if (_propertyTrackingSettings == PropertyTrackingSetting.None) { return; } var previousPropertyValue = _projectInstance.GetProperty(propertyName)?.EvaluatedValue; - if (previousPropertyValue == null && (_propertyTrackingSettings & PropertyTrackingSetting.PropertyInitialValueSet) == PropertyTrackingSetting.PropertyInitialValueSet) + if (previousPropertyValue == null && PropertyTrackingUtils.IsPropertyTrackingEnabled(_propertyTrackingSettings, PropertyTrackingSetting.PropertyInitialValueSet)) { var args = new PropertyInitialValueSetEventArgs( propertyName, @@ -1622,7 +1623,7 @@ private void LogPropertyInTaskAssignment(string propertyName, string propertyVal _targetLoggingContext.LogBuildEvent(args); } - else if ((_propertyTrackingSettings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) + else if (PropertyTrackingUtils.IsPropertyTrackingEnabled(_propertyTrackingSettings, PropertyTrackingSetting.PropertyReassignment)) { var args = new PropertyReassignmentEventArgs( propertyName, diff --git a/src/Build/Definition/ProjectCollection.cs b/src/Build/Definition/ProjectCollection.cs index 6b73a927756..4b25d8ded91 100644 --- a/src/Build/Definition/ProjectCollection.cs +++ b/src/Build/Definition/ProjectCollection.cs @@ -148,11 +148,6 @@ public class ProjectCollection : IToolsetProvider, IBuildComponent, IDisposable /// private readonly PropertyDictionary _globalProperties; - /// - /// Properties passed from the command line (e.g. by using /p:). - /// - private readonly HashSet _propertiesFromCommandLine; - /// /// The properties representing the environment. /// @@ -347,7 +342,7 @@ public ProjectCollection(IDictionary globalProperties, IEnumerab _globalProperties = new PropertyDictionary(globalProperties.Count); // at this stage globalProperties collection contains entries passed from command line (e.g. /p:foo=bar). - _propertiesFromCommandLine = new HashSet(globalProperties.Keys); + PropertiesFromCommandLine = [.. globalProperties.Keys]; foreach (KeyValuePair pair in globalProperties) { @@ -508,7 +503,7 @@ public static string DisplayVersion /// /// Properties passed from the command line (e.g. by using /p:). /// - public HashSet PropertiesFromCommandLine => _propertiesFromCommandLine; + public HashSet PropertiesFromCommandLine { get; } /// /// The default tools version of this project collection. Projects use this tools version if they diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 7e6eddc236e..ceff67c2bc5 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1259,13 +1259,13 @@ private int AddGlobalProperties() foreach (ProjectPropertyInstance globalProperty in _data.GlobalPropertiesDictionary) { - _ = _data.SetProperty( - globalProperty.Name, - ((IProperty)globalProperty).EvaluatedValueEscaped, - isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, - false /* may NOT be a reserved name */, - loggingContext: _evaluationLoggingContext, - isCommandLineProperty: _propertiesFromCommandLine.Contains(globalProperty.Name) /* IS coming from command line argument */); + _ = _data.SetProperty( + globalProperty.Name, + ((IProperty)globalProperty).EvaluatedValueEscaped, + isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, + false /* may NOT be a reserved name */, + loggingContext: _evaluationLoggingContext, + isCommandLineProperty: _propertiesFromCommandLine.Contains(globalProperty.Name) /* IS coming from command line argument */); } return _data.GlobalPropertiesDictionary.Count; diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index afccc78c9d8..ed5a7130092 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Runtime; using Microsoft.Build.BackEnd; using Microsoft.Build.BackEnd.Components.Logging; using Microsoft.Build.BackEnd.Logging; @@ -175,13 +176,10 @@ public P SetProperty(ProjectPropertyElement propertyElement, string evaluatedVal #region Private Methods... private bool IsPropertyReadTrackingRequested - => IsEnvironmentVariableReadTrackingRequested || - (_settings & PropertyTrackingSetting.UninitializedPropertyRead) == - PropertyTrackingSetting.UninitializedPropertyRead; + => IsEnvironmentVariableReadTrackingRequested + || PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.UninitializedPropertyRead); - private bool IsEnvironmentVariableReadTrackingRequested - => (_settings & PropertyTrackingSetting.EnvironmentVariableRead) == - PropertyTrackingSetting.EnvironmentVariableRead; + private bool IsEnvironmentVariableReadTrackingRequested => PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.EnvironmentVariableRead); /// /// Logic containing what to do when a property is read. @@ -216,7 +214,7 @@ private void TrackPropertyRead(string name, P property) /// The name of the environment variable read. private void TrackEnvironmentVariableRead(string name) { - if ((_settings & PropertyTrackingSetting.EnvironmentVariableRead) != PropertyTrackingSetting.EnvironmentVariableRead) + if (!IsEnvironmentVariableReadTrackingRequested) { return; } @@ -238,7 +236,7 @@ private void TrackEnvironmentVariableRead(string name) /// The name of the uninitialized property read. private void TrackUninitializedPropertyRead(string name) { - if ((_settings & PropertyTrackingSetting.UninitializedPropertyRead) != PropertyTrackingSetting.UninitializedPropertyRead) + if (!PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.UninitializedPropertyRead)) { return; } @@ -288,7 +286,7 @@ private void TrackPropertyWrite( /// The exact location of the property. Can be null if comes not form xml. private void TrackPropertyInitialValueSet(P property, PropertySource source, IElementLocation? location) { - if ((_settings & PropertyTrackingSetting.PropertyInitialValueSet) != PropertyTrackingSetting.PropertyInitialValueSet) + if (!PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.PropertyInitialValueSet)) { return; } @@ -331,8 +329,9 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat } // Either we want to specifically track property reassignments - // or we do not want to track nothing - in which case the prop reassignment is enabled by default. - if (_settings == 0 || (_settings & PropertyTrackingSetting.PropertyReassignment) == PropertyTrackingSetting.PropertyReassignment) + // or we do not want to track nothing - in which case the prop reassignment is enabled by default. + if (PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.PropertyReassignment) + || (_settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) { var args = new PropertyReassignmentEventArgs( property.Name, @@ -416,4 +415,73 @@ internal enum PropertyTrackingSetting All = PropertyReassignment | PropertyInitialValueSet | EnvironmentVariableRead | UninitializedPropertyRead } + + internal class PropertyTrackingUtils + { + /// + /// Determines if a specific property tracking setting is enabled within the provided settings configuration. + /// + /// The combined property tracking settings value to check against. + /// The specific tracking setting to verify. + /// true if the specified tracking setting is enabled in the settings configuration. + internal static bool IsPropertyTrackingEnabled(PropertyTrackingSetting settings, PropertyTrackingSetting currentTrackingSetting) => (settings & currentTrackingSetting) == currentTrackingSetting; + + /// + /// Logs property assignment information during execution, providing detailed tracking of property value changes. + /// This internal method handles two scenarios: + /// 1. Initial property value assignment (when previousPropertyValue is null) + /// 2. Property value reassignment (when previousPropertyValue has a value) + /// If property tracking is disabled (PropertyTrackingSetting.None), no logging occurs. + /// + /// Controls what types of property assignments should be tracked. + /// Name of the property being modified. + /// New value being assigned to the property. + /// Source location information (file, line, column). + /// Previous value of the property, null if this is initial assignment. + /// Context for logging build events. + internal static void LogPropertyAssignment( + PropertyTrackingSetting settings, + string propertyName, + string propertyValue, + IElementLocation location, + string? previousPropertyValue, + LoggingContext loggingContext) + { + if (settings == PropertyTrackingSetting.None) + { + return; + } + + if (previousPropertyValue == null && IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyInitialValueSet)) + { + var args = new PropertyInitialValueSetEventArgs( + propertyName, + propertyValue, + propertySource: string.Empty, + location.File, + location.Line, + location.Column, + message: null) + { BuildEventContext = loggingContext.BuildEventContext }; + + loggingContext.LogBuildEvent(args); + } + else if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) + || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + { + var args = new PropertyReassignmentEventArgs( + propertyName, + previousPropertyValue, + propertyValue, + location: null, + location.File, + location.Line, + location.Column, + message: null) + { BuildEventContext = loggingContext.BuildEventContext, }; + + loggingContext.LogBuildEvent(args); + } + } + } } diff --git a/src/Framework/UninitializedPropertyReadEventArgs.cs b/src/Framework/UninitializedPropertyReadEventArgs.cs index 3c89e480e78..7980bdb5485 100644 --- a/src/Framework/UninitializedPropertyReadEventArgs.cs +++ b/src/Framework/UninitializedPropertyReadEventArgs.cs @@ -3,7 +3,6 @@ using System; using System.IO; -using System.Xml.Linq; using Microsoft.Build.Shared; #nullable disable From 48107543350750cc1faab0f9965602c1051a81ce Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Fri, 7 Feb 2025 10:10:50 +0100 Subject: [PATCH 18/25] fix review comments --- .../RequestBuilder/IntrinsicTask.cs | 1 - .../TaskExecutionHost/TaskExecutionHost.cs | 48 +-------------- .../PropertyTrackingEvaluatorDataWrapper.cs | 59 ++++++++++--------- 3 files changed, 33 insertions(+), 75 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs index a5d7ea5622c..94c1ee183ac 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTask.cs @@ -6,7 +6,6 @@ using System.Reflection; using Microsoft.Build.BackEnd.Logging; using Microsoft.Build.Execution; -using Microsoft.Build.Framework; using Microsoft.Build.Shared; #nullable disable diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 8e9ead325fd..cb14eb7df34 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1586,7 +1586,7 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } - PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, outputTargetName, outputString, parameterLocation, _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, _targetLoggingContext); + PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, outputTargetName, outputString, parameterLocation, _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, _taskLoggingContext); _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable)); } @@ -1594,52 +1594,6 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } - /// - /// Logs property assignment information during task execution, based on configured property tracking settings. - /// - /// The name of the property being assigned or reassigned. - /// The new value being assigned to the property. - /// The source location where the property assignment occurs. - private void LogPropertyInTaskAssignment(string propertyName, string propertyValue, IElementLocation location) - { - if (_propertyTrackingSettings == PropertyTrackingSetting.None) - { - return; - } - - var previousPropertyValue = _projectInstance.GetProperty(propertyName)?.EvaluatedValue; - - if (previousPropertyValue == null && PropertyTrackingUtils.IsPropertyTrackingEnabled(_propertyTrackingSettings, PropertyTrackingSetting.PropertyInitialValueSet)) - { - var args = new PropertyInitialValueSetEventArgs( - propertyName, - propertyValue, - propertySource: string.Empty, - location.File, - location.Line, - location.Column, - message: null) - { BuildEventContext = _targetLoggingContext.BuildEventContext }; - - _targetLoggingContext.LogBuildEvent(args); - } - else if (PropertyTrackingUtils.IsPropertyTrackingEnabled(_propertyTrackingSettings, PropertyTrackingSetting.PropertyReassignment)) - { - var args = new PropertyReassignmentEventArgs( - propertyName, - previousPropertyValue, - propertyValue, - location: null, - location.File, - location.Line, - location.Column, - message: null) - { BuildEventContext = _targetLoggingContext.BuildEventContext }; - - _targetLoggingContext.LogBuildEvent(args); - } - } - /// /// Finds all the task properties that are required. /// Returns them as keys in a dictionary. diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index ed5a7130092..2b04ff1e3d4 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Runtime; using Microsoft.Build.BackEnd; using Microsoft.Build.BackEnd.Components.Logging; using Microsoft.Build.BackEnd.Logging; @@ -452,35 +451,41 @@ internal static void LogPropertyAssignment( return; } - if (previousPropertyValue == null && IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyInitialValueSet)) + if (previousPropertyValue == null) { - var args = new PropertyInitialValueSetEventArgs( - propertyName, - propertyValue, - propertySource: string.Empty, - location.File, - location.Line, - location.Column, - message: null) - { BuildEventContext = loggingContext.BuildEventContext }; - - loggingContext.LogBuildEvent(args); + if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyInitialValueSet)) + { + var args = new PropertyInitialValueSetEventArgs( + propertyName, + propertyValue, + propertySource: string.Empty, + location.File, + location.Line, + location.Column, + message: null) { BuildEventContext = loggingContext.BuildEventContext }; + + loggingContext.LogBuildEvent(args); + } } - else if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) - || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + else { - var args = new PropertyReassignmentEventArgs( - propertyName, - previousPropertyValue, - propertyValue, - location: null, - location.File, - location.Line, - location.Column, - message: null) - { BuildEventContext = loggingContext.BuildEventContext, }; - - loggingContext.LogBuildEvent(args); + if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + { + if (propertyValue != previousPropertyValue) + { + var args = new PropertyReassignmentEventArgs( + propertyName, + previousPropertyValue, + propertyValue, + location: null, + location.File, + location.Line, + location.Column, + message: null) { BuildEventContext = loggingContext.BuildEventContext, }; + + loggingContext.LogBuildEvent(args); + } + } } } } From f6f2b20cdf467f66d6626879034eea9cc555cee3 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 10 Feb 2025 10:59:55 +0100 Subject: [PATCH 19/25] fix review comments --- src/Build/BackEnd/BuildManager/BuildParameters.cs | 9 ++------- .../IntrinsicTasks/PropertyGroupIntrinsicTask.cs | 11 ++++++++++- .../BackEnd/TaskExecutionHost/TaskExecutionHost.cs | 11 ++++++++++- src/Build/Definition/ProjectCollection.cs | 2 +- src/Build/Evaluation/Evaluator.cs | 2 +- .../PropertyTrackingEvaluatorDataWrapper.cs | 10 +++------- 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index aa1dcf9f728..6efee9336ba 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -149,7 +149,7 @@ public class BuildParameters : ITranslatable /// /// Properties passed from the command line (e.g. by using /p:). /// - private HashSet _propertiesFromCommandLine; + private ICollection _propertiesFromCommandLine; /// /// The loggers. @@ -285,7 +285,7 @@ internal BuildParameters(BuildParameters other, bool resetEnvironment = false) _environmentProperties = other._environmentProperties != null ? new PropertyDictionary(other._environmentProperties) : null; _forwardingLoggers = other._forwardingLoggers != null ? new List(other._forwardingLoggers) : null; _globalProperties = other._globalProperties != null ? new PropertyDictionary(other._globalProperties) : null; - _propertiesFromCommandLine = other._propertiesFromCommandLine != null ? new HashSet(other._propertiesFromCommandLine) : null; + _propertiesFromCommandLine = other._propertiesFromCommandLine != null ? new HashSet(other._propertiesFromCommandLine, StringComparer.OrdinalIgnoreCase) : null; HostServices = other.HostServices; _loggers = other._loggers != null ? new List(other._loggers) : null; _maxNodeCount = other._maxNodeCount; @@ -479,11 +479,6 @@ public IDictionary GlobalProperties } } - /// - /// Properties passed from the command line (e.g. by using /p:). - /// - public HashSet PropertiesFromCommandLine => _propertiesFromCommandLine; - /// /// Interface allowing the host to provide additional control over the build process. /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 89745b5aab9..1326ea054fc 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -88,7 +88,16 @@ internal override void ExecuteTask(Lookup lookup) string evaluatedValue = bucket.Expander.ExpandIntoStringLeaveEscaped(property.Value, ExpanderOptions.ExpandAll, property.Location); bucket.Expander.PropertiesUseTracker.CheckPreexistingUndefinedUsage(property, evaluatedValue, LoggingContext); - PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, property.Name, evaluatedValue, property.Location, Project.GetProperty(property.Name)?.EvaluatedValue ?? null, LoggingContext); + if (_propertyTrackingSettings != PropertyTrackingSetting.None) + { + PropertyTrackingUtils.LogPropertyAssignment( + _propertyTrackingSettings, + property.Name, + evaluatedValue, + property.Location, + Project.GetProperty(property.Name)?.EvaluatedValue ?? null, + LoggingContext); + } if (LogTaskInputs && !LoggingContext.LoggingService.OnlyLogCriticalEvents) { diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index cb14eb7df34..3280ce578af 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1586,7 +1586,16 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } - PropertyTrackingUtils.LogPropertyAssignment(_propertyTrackingSettings, outputTargetName, outputString, parameterLocation, _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, _taskLoggingContext); + if (_propertyTrackingSettings != PropertyTrackingSetting.None) + { + PropertyTrackingUtils.LogPropertyAssignment( + _propertyTrackingSettings, + outputTargetName, + outputString, + parameterLocation, + _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, + _taskLoggingContext); + } _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable)); } diff --git a/src/Build/Definition/ProjectCollection.cs b/src/Build/Definition/ProjectCollection.cs index 4b25d8ded91..247c986423f 100644 --- a/src/Build/Definition/ProjectCollection.cs +++ b/src/Build/Definition/ProjectCollection.cs @@ -503,7 +503,7 @@ public static string DisplayVersion /// /// Properties passed from the command line (e.g. by using /p:). /// - public HashSet PropertiesFromCommandLine { get; } + public ICollection PropertiesFromCommandLine { get; } /// /// The default tools version of this project collection. Projects use this tools version if they diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index ceff67c2bc5..f7775e3edc6 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1263,7 +1263,7 @@ private int AddGlobalProperties() globalProperty.Name, ((IProperty)globalProperty).EvaluatedValueEscaped, isGlobalProperty: true /* it is a global property, but it comes from command line and is tracked separately */, - false /* may NOT be a reserved name */, + mayBeReserved: false /* may NOT be a reserved name */, loggingContext: _evaluationLoggingContext, isCommandLineProperty: _propertiesFromCommandLine.Contains(globalProperty.Name) /* IS coming from command line argument */); } diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 2b04ff1e3d4..8a66ca3f63f 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -392,7 +392,7 @@ private enum PropertySource private static string GetPropertySourceName(PropertySource source) => source switch { - PropertySource.Xml => "XML", + PropertySource.Xml => "Xml", PropertySource.BuiltIn => "BuiltIn", PropertySource.Global => "Global", PropertySource.Toolset => "Toolset", @@ -446,11 +446,6 @@ internal static void LogPropertyAssignment( string? previousPropertyValue, LoggingContext loggingContext) { - if (settings == PropertyTrackingSetting.None) - { - return; - } - if (previousPropertyValue == null) { if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyInitialValueSet)) @@ -469,7 +464,8 @@ internal static void LogPropertyAssignment( } else { - if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) + || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) { if (propertyValue != previousPropertyValue) { From d51cfc7ae59e485a4d025221c67f6808baa4df89 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Mon, 10 Feb 2025 11:46:41 +0100 Subject: [PATCH 20/25] fix review comments --- src/Build/BackEnd/BuildManager/BuildParameters.cs | 4 ++++ src/Build/Evaluation/Evaluator.cs | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index 6efee9336ba..6a60fd9ac0c 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -339,6 +339,10 @@ public bool UseSynchronousLogging set => _useSynchronousLogging = value; } + /// + /// Properties passed from the command line (e.g. by using /p:). + /// + public ICollection PropertiesFromCommandLine => _propertiesFromCommandLine; /// /// Indicates whether to emit a default error if a task returns false without logging an error. diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index f7775e3edc6..e285a948aa4 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -160,7 +160,7 @@ internal class Evaluator /// /// Properties passed from the command line (e.g. by using /p:). /// - private readonly HashSet _propertiesFromCommandLine; + private readonly ICollection _propertiesFromCommandLine; /// /// The cache to consult for any imports that need loading. @@ -205,7 +205,7 @@ private Evaluator( ProjectLoadSettings loadSettings, int maxNodeCount, PropertyDictionary environmentProperties, - HashSet propertiesFromCommandLine, + ICollection propertiesFromCommandLine, IItemFactory itemFactory, IToolsetProvider toolsetProvider, IDirectoryCacheFactory directoryCacheFactory, @@ -307,7 +307,7 @@ internal static void Evaluate( ProjectLoadSettings loadSettings, int maxNodeCount, PropertyDictionary environmentProperties, - HashSet propertiesFromCommandLine, + ICollection propertiesFromCommandLine, ILoggingService loggingService, IItemFactory itemFactory, IToolsetProvider toolsetProvider, From 80beaa6b26389d5e34a11b19c9aa30aee8a9bbae Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Wed, 12 Feb 2025 13:46:35 +0100 Subject: [PATCH 21/25] fix review comments --- .../PropertyGroupIntrinsicTask.cs | 17 +++++++---------- .../TaskExecutionHost/TaskExecutionHost.cs | 17 +++++++---------- .../PropertyTrackingEvaluatorDataWrapper.cs | 13 +++++++------ 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs index 1326ea054fc..5bd3a49c331 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/PropertyGroupIntrinsicTask.cs @@ -88,16 +88,13 @@ internal override void ExecuteTask(Lookup lookup) string evaluatedValue = bucket.Expander.ExpandIntoStringLeaveEscaped(property.Value, ExpanderOptions.ExpandAll, property.Location); bucket.Expander.PropertiesUseTracker.CheckPreexistingUndefinedUsage(property, evaluatedValue, LoggingContext); - if (_propertyTrackingSettings != PropertyTrackingSetting.None) - { - PropertyTrackingUtils.LogPropertyAssignment( - _propertyTrackingSettings, - property.Name, - evaluatedValue, - property.Location, - Project.GetProperty(property.Name)?.EvaluatedValue ?? null, - LoggingContext); - } + PropertyTrackingUtils.LogPropertyAssignment( + _propertyTrackingSettings, + property.Name, + evaluatedValue, + property.Location, + Project.GetProperty(property.Name)?.EvaluatedValue ?? null, + LoggingContext); if (LogTaskInputs && !LoggingContext.LoggingService.OnlyLogCriticalEvents) { diff --git a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs index 3280ce578af..261f52fcb76 100644 --- a/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs +++ b/src/Build/BackEnd/TaskExecutionHost/TaskExecutionHost.cs @@ -1586,16 +1586,13 @@ private void GatherArrayStringAndValueOutputs(bool outputTargetIsItem, string ou } } - if (_propertyTrackingSettings != PropertyTrackingSetting.None) - { - PropertyTrackingUtils.LogPropertyAssignment( - _propertyTrackingSettings, - outputTargetName, - outputString, - parameterLocation, - _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, - _taskLoggingContext); - } + PropertyTrackingUtils.LogPropertyAssignment( + _propertyTrackingSettings, + outputTargetName, + outputString, + parameterLocation, + _projectInstance.GetProperty(outputTargetName)?.EvaluatedValue ?? null, + _taskLoggingContext); _batchBucket.Lookup.SetProperty(ProjectPropertyInstance.Create(outputTargetName, outputString, parameterLocation, _projectInstance.IsImmutable)); } diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 8a66ca3f63f..47e7e8db160 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -327,10 +327,7 @@ private void TrackPropertyReassignment(P? predecessor, P property, IElementLocat return; } - // Either we want to specifically track property reassignments - // or we do not want to track nothing - in which case the prop reassignment is enabled by default. - if (PropertyTrackingUtils.IsPropertyTrackingEnabled(_settings, PropertyTrackingSetting.PropertyReassignment) - || (_settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + if (PropertyTrackingUtils.IsPropertyReassignmentEnabled(_settings)) { var args = new PropertyReassignmentEventArgs( property.Name, @@ -425,6 +422,11 @@ internal class PropertyTrackingUtils /// true if the specified tracking setting is enabled in the settings configuration. internal static bool IsPropertyTrackingEnabled(PropertyTrackingSetting settings, PropertyTrackingSetting currentTrackingSetting) => (settings & currentTrackingSetting) == currentTrackingSetting; + // Either we want to specifically track property reassignments + // or we do not want to track nothing - in which case the prop reassignment is enabled by default. + internal static bool IsPropertyReassignmentEnabled(PropertyTrackingSetting currentTrackingSetting) => IsPropertyTrackingEnabled(currentTrackingSetting, PropertyTrackingSetting.PropertyReassignment) + || (currentTrackingSetting == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)); + /// /// Logs property assignment information during execution, providing detailed tracking of property value changes. /// This internal method handles two scenarios: @@ -464,8 +466,7 @@ internal static void LogPropertyAssignment( } else { - if (IsPropertyTrackingEnabled(settings, PropertyTrackingSetting.PropertyReassignment) - || (settings == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14))) + if (IsPropertyReassignmentEnabled(settings)) { if (propertyValue != previousPropertyValue) { From bdadaea8aa91de921767a5686592dfd86d62c299 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Wed, 12 Feb 2025 13:52:40 +0100 Subject: [PATCH 22/25] change PropertiesFromCommandLine population --- src/Build/Definition/ProjectCollection.cs | 5 +---- src/MSBuild/XMake.cs | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Build/Definition/ProjectCollection.cs b/src/Build/Definition/ProjectCollection.cs index 247c986423f..9973e5d3d80 100644 --- a/src/Build/Definition/ProjectCollection.cs +++ b/src/Build/Definition/ProjectCollection.cs @@ -341,9 +341,6 @@ public ProjectCollection(IDictionary globalProperties, IEnumerab { _globalProperties = new PropertyDictionary(globalProperties.Count); - // at this stage globalProperties collection contains entries passed from command line (e.g. /p:foo=bar). - PropertiesFromCommandLine = [.. globalProperties.Keys]; - foreach (KeyValuePair pair in globalProperties) { try @@ -503,7 +500,7 @@ public static string DisplayVersion /// /// Properties passed from the command line (e.g. by using /p:). /// - public ICollection PropertiesFromCommandLine { get; } + public ICollection PropertiesFromCommandLine { get; set; } /// /// The default tools version of this project collection. Projects use this tools version if they diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index aeddef7aba4..9afa243f2bc 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -835,6 +835,8 @@ public static ExitType Execute( { using (ProjectCollection collection = new(globalProperties, loggers, ToolsetDefinitionLocations.Default)) { + collection.PropertiesFromCommandLine = [.. globalProperties.Keys]; + Project project = collection.LoadProject(projectFile, globalProperties, toolsVersion); if (getResultOutputFile.Length == 0) From 9a7cd552859efc75a758227a84b5aaa150486c46 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Thu, 13 Feb 2025 11:33:01 +0100 Subject: [PATCH 23/25] fix the missed case with command line --- src/MSBuild/XMake.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs index 9afa243f2bc..c12cd96ec57 100644 --- a/src/MSBuild/XMake.cs +++ b/src/MSBuild/XMake.cs @@ -835,6 +835,7 @@ public static ExitType Execute( { using (ProjectCollection collection = new(globalProperties, loggers, ToolsetDefinitionLocations.Default)) { + // globalProperties collection contains values only from CommandLine at this stage populated by ProcessCommandLineSwitches collection.PropertiesFromCommandLine = [.. globalProperties.Keys]; Project project = collection.LoadProject(projectFile, globalProperties, toolsVersion); @@ -1395,6 +1396,9 @@ internal static bool BuildProject( useAsynchronousLogging: true, reuseProjectRootElementCache: s_isServerNode); + // globalProperties collection contains values only from CommandLine at this stage populated by ProcessCommandLineSwitches + projectCollection.PropertiesFromCommandLine = [.. globalProperties.Keys]; + if (toolsVersion != null && !projectCollection.ContainsToolset(toolsVersion)) { ThrowInvalidToolsVersionInitializationException(projectCollection.Toolsets, toolsVersion); From 78eaf82cbb02639688f0e236b1f719cfe07bb7ec Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Thu, 13 Feb 2025 11:57:29 +0100 Subject: [PATCH 24/25] undo change of change wave --- src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs index 47e7e8db160..c7b8ae0d6ad 100644 --- a/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs +++ b/src/Build/Evaluation/PropertyTrackingEvaluatorDataWrapper.cs @@ -425,7 +425,7 @@ internal class PropertyTrackingUtils // Either we want to specifically track property reassignments // or we do not want to track nothing - in which case the prop reassignment is enabled by default. internal static bool IsPropertyReassignmentEnabled(PropertyTrackingSetting currentTrackingSetting) => IsPropertyTrackingEnabled(currentTrackingSetting, PropertyTrackingSetting.PropertyReassignment) - || (currentTrackingSetting == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_14)); + || (currentTrackingSetting == PropertyTrackingSetting.None && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)); /// /// Logs property assignment information during execution, providing detailed tracking of property value changes. From 152ceb23e79cc60be0112cca6782d276b3841277 Mon Sep 17 00:00:00 2001 From: YuliiaKovalova Date: Thu, 13 Feb 2025 12:15:14 +0100 Subject: [PATCH 25/25] add doc about property assignment --- .../Property-tracking-capabilities.md | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 documentation/Property-tracking-capabilities.md diff --git a/documentation/Property-tracking-capabilities.md b/documentation/Property-tracking-capabilities.md new file mode 100644 index 00000000000..a2cef4e2543 --- /dev/null +++ b/documentation/Property-tracking-capabilities.md @@ -0,0 +1,49 @@ +# MSBuild's property tracking capabilities + +MSBuild Property Tracking is a built-in diagnostic feature that tracks property value changes during the build process. +By default, this feature is opted out due to performance considerations. + +## Property Tracking Coverage + +The implementation tracks properties in the following scenarios: + +1. Properties set via command-line arguments (e.g. using `/p:` switches) + +2. Properties defined based on environment variables and used by MSBuild + +3. Properties set as target outputs + - Tracks changes when properties are modified by target execution + +4. Properties set as task outputs + - Monitors property modifications resulting from task execution + +5. Properties defined in XML during evaluation + - Provides exact location information for properties defined in project files + - Includes line and column information from the source XML + - Reports on property modifications + +## Event Types and Message Formatting + +The feature implements specialized event handling for three scenarios: + +1. `PropertyReassignmentEventArgs` + - Triggered when a property value is changed + `set MsBuildLogPropertyTracking=1` + +2. `PropertyInitialValueSetEventArgs` + - Triggered when a property is first initialized + `set MsBuildLogPropertyTracking=2` + +3. `EnvironmentVariableRead` + - Tracks when environment variables are read + `set MsBuildLogPropertyTracking=4` + +4. `UninitializedPropertyReadEventArgs` + - Triggered when attempting to read a property that hasn't been initialized + `set MsBuildLogPropertyTracking=8` + +5. None + - Disables all property tracking + `set MsBuildLogPropertyTracking=0` + +If you want to enable all these events reporting, enable it by `set MsBuildLogPropertyTracking=15`. \ No newline at end of file