From cd64d54d7c6d4dccc84ef614f61a7346e9135048 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Thu, 13 Jun 2019 09:05:43 -0700 Subject: [PATCH 1/6] Implementing tracking of environment variables (and uninitialized properties) for graph evaluation. --- ref/Microsoft.Build/net/Microsoft.Build.cs | 5 + .../netstandard/Microsoft.Build.cs | 5 + .../Graph/ProjectGraph_Tests.cs | 197 ++++++++++++++++++ .../BackEnd/BuildManager/BuildParameters.cs | 14 ++ src/Build/Definition/Project.cs | 2 +- src/Build/Evaluation/Evaluator.cs | 2 +- src/Build/Evaluation/IEvaluatorData.cs | 2 +- .../LazyItemEvaluator.EvaluatorData.cs | 2 +- src/Build/Graph/ProjectGraph.cs | 11 + src/Build/Instance/ProjectInstance.cs | 92 +++++++- 10 files changed, 325 insertions(+), 7 deletions(-) diff --git a/ref/Microsoft.Build/net/Microsoft.Build.cs b/ref/Microsoft.Build/net/Microsoft.Build.cs index e52adc709ef..cfe1727b572 100644 --- a/ref/Microsoft.Build/net/Microsoft.Build.cs +++ b/ref/Microsoft.Build/net/Microsoft.Build.cs @@ -973,6 +973,7 @@ public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectColle public bool ShutdownInProcNodeOnBuildFinish { get { throw null; } set { } } public Microsoft.Build.Evaluation.ToolsetDefinitionLocations ToolsetDefinitionLocations { get { throw null; } set { } } public System.Collections.Generic.ICollection Toolsets { get { throw null; } } + public bool TrackReadsOnEnvironmentVariables { get { throw null; } set { } } public System.Globalization.CultureInfo UICulture { get { throw null; } set { } } public bool UseSynchronousLogging { get { throw null; } set { } } public System.Collections.Generic.ISet WarningsAsErrors { get { throw null; } set { } } @@ -1095,6 +1096,7 @@ public ProjectInstance(string projectFile, System.Collections.Generic.IDictionar public ProjectInstance(string projectFile, System.Collections.Generic.IDictionary globalProperties, string toolsVersion, string subToolsetVersion, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.List DefaultTargets { get { throw null; } } public string Directory { get { throw null; } } + public System.Collections.Generic.ICollection EnvironmentVariableReads { get { throw null; } } public System.Collections.Generic.List EvaluatedItemElements { get { throw null; } } public int EvaluationId { get { throw null; } set { } } public string FullPath { get { throw null; } } @@ -1109,6 +1111,7 @@ public ProjectInstance(string projectFile, System.Collections.Generic.IDictionar public System.Collections.Generic.IDictionary Targets { get { throw null; } } public string ToolsVersion { get { throw null; } } public bool TranslateEntireState { get { throw null; } set { } } + public System.Collections.Generic.ICollection UninitializedPropertyReads { get { throw null; } } public Microsoft.Build.Execution.ProjectItemInstance AddItem(string itemType, string evaluatedInclude) { throw null; } public Microsoft.Build.Execution.ProjectItemInstance AddItem(string itemType, string evaluatedInclude, System.Collections.Generic.IEnumerable> metadata) { throw null; } public bool Build() { throw null; } @@ -1425,9 +1428,11 @@ public ProjectGraph(string entryProjectFile, Microsoft.Build.Evaluation.ProjectC public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties) { } public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.IReadOnlyCollection EntryPointNodes { get { throw null; } } + public System.Collections.Generic.IEnumerable EnvironmentVariableReads { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection GraphRoots { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodes { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodesTopologicallySorted { get { throw null; } } + public System.Collections.Generic.IEnumerable UninitializedPropertyReads { get { throw null; } } public System.Collections.Generic.IReadOnlyDictionary> GetTargetLists(System.Collections.Generic.ICollection entryProjectTargets) { throw null; } public delegate Microsoft.Build.Execution.ProjectInstance ProjectInstanceFactoryFunc(string projectPath, System.Collections.Generic.Dictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection); } diff --git a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs index e464b7cfc94..d1d8336afb3 100644 --- a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs +++ b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs @@ -968,6 +968,7 @@ public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectColle public bool ShutdownInProcNodeOnBuildFinish { get { throw null; } set { } } public Microsoft.Build.Evaluation.ToolsetDefinitionLocations ToolsetDefinitionLocations { get { throw null; } set { } } public System.Collections.Generic.ICollection Toolsets { get { throw null; } } + public bool TrackReadsOnEnvironmentVariables { get { throw null; } set { } } public System.Globalization.CultureInfo UICulture { get { throw null; } set { } } public bool UseSynchronousLogging { get { throw null; } set { } } public System.Collections.Generic.ISet WarningsAsErrors { get { throw null; } set { } } @@ -1089,6 +1090,7 @@ public ProjectInstance(string projectFile, System.Collections.Generic.IDictionar public ProjectInstance(string projectFile, System.Collections.Generic.IDictionary globalProperties, string toolsVersion, string subToolsetVersion, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.List DefaultTargets { get { throw null; } } public string Directory { get { throw null; } } + public System.Collections.Generic.ICollection EnvironmentVariableReads { get { throw null; } } public System.Collections.Generic.List EvaluatedItemElements { get { throw null; } } public int EvaluationId { get { throw null; } set { } } public string FullPath { get { throw null; } } @@ -1103,6 +1105,7 @@ public ProjectInstance(string projectFile, System.Collections.Generic.IDictionar public System.Collections.Generic.IDictionary Targets { get { throw null; } } public string ToolsVersion { get { throw null; } } public bool TranslateEntireState { get { throw null; } set { } } + public System.Collections.Generic.ICollection UninitializedPropertyReads { get { throw null; } } public Microsoft.Build.Execution.ProjectItemInstance AddItem(string itemType, string evaluatedInclude) { throw null; } public Microsoft.Build.Execution.ProjectItemInstance AddItem(string itemType, string evaluatedInclude, System.Collections.Generic.IEnumerable> metadata) { throw null; } public bool Build() { throw null; } @@ -1419,9 +1422,11 @@ public ProjectGraph(string entryProjectFile, Microsoft.Build.Evaluation.ProjectC public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties) { } public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.IReadOnlyCollection EntryPointNodes { get { throw null; } } + public System.Collections.Generic.IEnumerable EnvironmentVariableReads { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection GraphRoots { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodes { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodesTopologicallySorted { get { throw null; } } + public System.Collections.Generic.IEnumerable UninitializedPropertyReads { get { throw null; } } public System.Collections.Generic.IReadOnlyDictionary> GetTargetLists(System.Collections.Generic.ICollection entryProjectTargets) { throw null; } public delegate Microsoft.Build.Execution.ProjectInstance ProjectInstanceFactoryFunc(string projectPath, System.Collections.Generic.Dictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection); } diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index 079258236b7..ff25a21c0df 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -1847,6 +1847,203 @@ public void InnerBuildsProducedByOuterBuildsCanBeReferencedByOtherInnerBuilds() innerBuild1WithReferenceToInnerBuild2.ProjectReferences.ShouldBeEquivalentTo(new []{outerBuild2, innerBuild2}); } + [Fact] + public void EnvironmentVariablesReadReturnsNullWhenDisabled() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", ""); + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + booyah + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.ShouldBeNull(); + } + } + + [Fact] + public void EnvironmentVariablesReadReturnsNotNullWhenEnabled() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + booyah + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(0); + } + } + + [Fact] + public void EnvironmentVariablesReadReturnsNumActuallyRead() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + $(Path) + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(1); + first.ProjectInstance.EnvironmentVariableReads.First().ShouldBe("Path"); + } + } + + [Fact] + public void EnvironmentVariablesReadIgnoresOverwrittenEnvVars() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + env.SetEnvironmentVariable("SomeTestEnvVar", "SomeValue"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + $(Path) + Overwritten! + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(1); // NOT TWO! + first.ProjectInstance.EnvironmentVariableReads.First().ShouldBe("Path"); + } + } + + [Fact] + public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + env.SetEnvironmentVariable("SomeTestEnvVar", "SomeValue"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + $(Path) + $(SomeTestEnvVar) + $(ComputerName) + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.ShouldNotBeNull(); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(3); + } + } + + [Fact] + public void UninitializedVariablesNullWhenDisabled() + { + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", ""); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + $(This_Should_Not_Exist) + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.UninitializedPropertyReads.ShouldBeNull(); + } + } + + [Fact] + public void UninitializedVariablesWithActualValues() + { + int defaultUninitPropCount = 0; + + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.UninitializedPropertyReads.ShouldNotBeNull(); + + // Determine how many uninitialized variables are present by default. + defaultUninitPropCount = first.ProjectInstance.UninitializedPropertyReads.Count; + } + + using (var env = TestEnvironment.Create()) + { + env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); + + var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" + + + + "); + + var graph = new ProjectGraph(projFile.Path); + graph.ProjectNodes.ShouldNotBeNull(); + graph.ProjectNodes.Count.ShouldBe(1); + var first = graph.ProjectNodes.First(); + first.ProjectInstance.ShouldNotBeNull(); + first.ProjectInstance.UninitializedPropertyReads.ShouldNotBeNull(); + int count = first.ProjectInstance.UninitializedPropertyReads.Count; + + first.ProjectInstance.UninitializedPropertyReads.Where(p => p.Equals("This_Should_Not_Exist")).FirstOrDefault().ShouldNotBeNull(); + (count - defaultUninitPropCount).ShouldBe(1); + } + } + public static IEnumerable AllNodesShouldHaveGraphBuildGlobalPropertyData { get diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index b19980a2b61..753f04ab9ac 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -96,6 +96,11 @@ public class BuildParameters : ITranslatable /// private static string s_msbuildExeKnownToExistAt; + /// + /// Specifies whether or not MSBuild will track environment variable reads + /// + private static bool? s_trackReadsOnEnvironmentVariables; + /// /// The build id /// @@ -763,6 +768,15 @@ public string OutputResultsCacheFile set => _outputResultsCacheFile = value; } + /// + /// Track the environment variables actually read and used. + /// + public bool TrackReadsOnEnvironmentVariables + { + get => GetStaticBoolVariableOrDefault("MSBUILDTRACKENVVARREADS", ref s_trackReadsOnEnvironmentVariables, false); + set => s_trackReadsOnEnvironmentVariables = value; + } + /// /// Retrieves a toolset. /// diff --git a/src/Build/Definition/Project.cs b/src/Build/Definition/Project.cs index 67215eadefd..8c1b9716347 100644 --- a/src/Build/Definition/Project.cs +++ b/src/Build/Definition/Project.cs @@ -3371,7 +3371,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) + public ProjectProperty SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar = false) { ProjectProperty property = ProjectProperty.Create(Project, name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved); Properties.Set(property); diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs index 666338f2406..32fa18b6c1d 100644 --- a/src/Build/Evaluation/Evaluator.cs +++ b/src/Build/Evaluation/Evaluator.cs @@ -1256,7 +1256,7 @@ private ICollection

AddEnvironmentProperties() foreach (ProjectPropertyInstance environmentProperty in _environmentProperties) { - P property = _data.SetProperty(environmentProperty.Name, ((IProperty)environmentProperty).EvaluatedValueEscaped, false /* NOT global property */, false /* may NOT be a reserved name */); + P property = _data.SetProperty(environmentProperty.Name, ((IProperty)environmentProperty).EvaluatedValueEscaped, false /* NOT global property */, false /* may NOT be a reserved name */, true /* IS environment variable. */); environmentPropertiesList.Add(property); } diff --git a/src/Build/Evaluation/IEvaluatorData.cs b/src/Build/Evaluation/IEvaluatorData.cs index eb965e2c2ec..8ba17db92ef 100644 --- a/src/Build/Evaluation/IEvaluatorData.cs +++ b/src/Build/Evaluation/IEvaluatorData.cs @@ -263,7 +263,7 @@ List EvaluatedItemElements ///

/// Sets a property which does not come from the Xml. /// - P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved); + P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar = 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 df3f02f78ac..89fe36b66e1 100644 --- a/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs +++ b/src/Build/Evaluation/LazyItemEvaluator.EvaluatorData.cs @@ -297,7 +297,7 @@ public P SetProperty(ProjectPropertyElement propertyElement, string evaluatedVal return _wrappedData.SetProperty(propertyElement, evaluatedValueEscaped, predecessor); } - public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved) + public P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar = false) { return _wrappedData.SetProperty(name, evaluatedValueEscaped, isGlobalProperty, mayBeReserved); } diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index ea6e244bd8e..ca4527cd202 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; @@ -75,6 +76,16 @@ public delegate ProjectInstance ProjectInstanceFactoryFunc( public IReadOnlyCollection GraphRoots { get; } + /// + /// Gets a distinct enumeration of all of the environment variables reads during evaluation. + /// + public IEnumerable EnvironmentVariableReads => _projectNodesTopologicallySorted?.Value.Select(p => p.ProjectInstance).SelectMany(pi => pi.EnvironmentVariableReads).Distinct(); + + /// + /// Get a distinct enumeration of all the uninitialized property reads during evaluation. + /// + public IEnumerable UninitializedPropertyReads => _projectNodesTopologicallySorted?.Value.Select(p => p.ProjectInstance).SelectMany(pi => pi.UninitializedPropertyReads).Distinct(); + /// /// Constructs a graph starting from the given project file, evaluating with the global project collection and no /// global properties. diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index ad7099d9d69..ce9c9a02132 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -178,6 +178,11 @@ public class ProjectInstance : IPropertyProvider, IItem private bool _translateEntireState; private int _evaluationId = BuildEventContext.InvalidEvaluationId; + // Fields for tracking property usage: + private bool _trackPropertyReads = false; + private HashSet _environmentVariables; + private HashSet _environmentVariableReads; + private HashSet _uninitializedPropertyReads; /// /// Creates a ProjectInstance directly. @@ -818,6 +823,16 @@ public bool IsImmutable get { return _isImmutable; } } + /// + /// The collection of environment variables read. + /// + public ICollection EnvironmentVariableReads => _environmentVariableReads; + + /// + /// The collection of uninitialized variables read. + /// + public ICollection UninitializedPropertyReads => _uninitializedPropertyReads; + /// /// Task classes and locations known to this project. /// This is the project-specific task registry, which is consulted before @@ -1314,11 +1329,30 @@ IItemDefinition IEvaluatorData - ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved) + ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar) { // Mutability not verified as this is being populated during evaluation ProjectPropertyInstance property = ProjectPropertyInstance.Create(name, evaluatedValueEscaped, mayBeReserved, _isImmutable); _properties.Set(property); + + if (_trackPropertyReads) + { + if (isEnvVar) + { + if (!_environmentVariables.Contains(name)) + { + // If it's a new env var, track it. + _environmentVariables.Add(name); + } + } + else if (_environmentVariables.Contains(name)) + { + // If it's NOT an env var but the property has the same name, + // then we're not dependent on the env var any more: remove it. + _environmentVariables.Remove(name); + } + } + return property; } @@ -1383,7 +1417,14 @@ void IEvaluatorData @@ -1394,7 +1435,14 @@ public ProjectPropertyInstance GetProperty(string name) [DebuggerStepThrough] ProjectPropertyInstance IPropertyProvider.GetProperty(string name, int startIndex, int endIndex) { - return _properties.GetProperty(name, startIndex, endIndex); + ProjectPropertyInstance prop = _properties.GetProperty(name, startIndex, endIndex); + + if (_trackPropertyReads) + { + this.TrackPropertyRead(name.Substring(startIndex, endIndex-startIndex + 1), prop == null); + } + + return prop; } /// @@ -1416,6 +1464,11 @@ public string GetPropertyValue(string name) ProjectPropertyInstance property = _properties[name]; string value = (property == null) ? String.Empty : property.EvaluatedValue; + if (_trackPropertyReads) + { + this.TrackPropertyRead(name, property == null); + } + return value; } @@ -2477,6 +2530,14 @@ private void Initialize(ProjectRootElement xml, IDictionary glob _hostServices = buildParameters.HostServices; this.ProjectRootElementCache = buildParameters.ProjectRootElementCache; + _trackPropertyReads = buildParameters.TrackReadsOnEnvironmentVariables; + if (_trackPropertyReads) + { + _environmentVariables = new HashSet(StringComparer.OrdinalIgnoreCase); + _environmentVariableReads = new HashSet(StringComparer.OrdinalIgnoreCase); + _uninitializedPropertyReads = new HashSet(StringComparer.OrdinalIgnoreCase); + } + this.EvaluatedItemElements = new List(); _explicitToolsVersionSpecified = (explicitToolsVersion != null); @@ -2710,5 +2771,30 @@ private void CreatePropertiesSnapshot(Evaluation.Project.Data data, bool isImmut _properties.Set(instance); } } + + /// + /// Tracks this property read if it's from an environment variable or uninitialized. + /// + /// The name of the property being read. + private void TrackPropertyRead(string name, bool propertyMissing) + { + if (string.IsNullOrEmpty(name)) + return; + + if (propertyMissing) + { + if (!_uninitializedPropertyReads.Contains(name)) + { + _uninitializedPropertyReads.Add(name); + } + } + else if (_environmentVariables.Contains(name)) + { + if (!_environmentVariableReads.Contains(name)) + { + _environmentVariableReads.Add(name); + } + } + } } } From 9a11e67e97b340faf17aa63c389840870470eb97 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Fri, 14 Jun 2019 10:15:44 -0700 Subject: [PATCH 2/6] Changes based on PR feedback. --- ref/Microsoft.Build/net/Microsoft.Build.cs | 2 +- .../netstandard/Microsoft.Build.cs | 2 +- .../Graph/ProjectGraph_Tests.cs | 7 ++- .../BackEnd/BuildManager/BuildParameters.cs | 14 ----- src/Build/Evaluation/IEvaluatorData.cs | 2 +- src/Build/Graph/ProjectGraph.cs | 57 ++++++++++++++++++- src/Build/Instance/ProjectInstance.cs | 11 ++-- src/Shared/Traits.cs | 5 ++ 8 files changed, 74 insertions(+), 26 deletions(-) diff --git a/ref/Microsoft.Build/net/Microsoft.Build.cs b/ref/Microsoft.Build/net/Microsoft.Build.cs index cfe1727b572..5c8595965da 100644 --- a/ref/Microsoft.Build/net/Microsoft.Build.cs +++ b/ref/Microsoft.Build/net/Microsoft.Build.cs @@ -973,7 +973,6 @@ public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectColle public bool ShutdownInProcNodeOnBuildFinish { get { throw null; } set { } } public Microsoft.Build.Evaluation.ToolsetDefinitionLocations ToolsetDefinitionLocations { get { throw null; } set { } } public System.Collections.Generic.ICollection Toolsets { get { throw null; } } - public bool TrackReadsOnEnvironmentVariables { get { throw null; } set { } } public System.Globalization.CultureInfo UICulture { get { throw null; } set { } } public bool UseSynchronousLogging { get { throw null; } set { } } public System.Collections.Generic.ISet WarningsAsErrors { get { throw null; } set { } } @@ -1429,6 +1428,7 @@ public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDiction public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.IReadOnlyCollection EntryPointNodes { get { throw null; } } public System.Collections.Generic.IEnumerable EnvironmentVariableReads { get { throw null; } } + public System.Collections.Generic.IEnumerable EnvironmentVariablesImpactingBuild { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection GraphRoots { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodes { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodesTopologicallySorted { get { throw null; } } diff --git a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs index d1d8336afb3..af8ca9a6c19 100644 --- a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs +++ b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs @@ -968,7 +968,6 @@ public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectColle public bool ShutdownInProcNodeOnBuildFinish { get { throw null; } set { } } public Microsoft.Build.Evaluation.ToolsetDefinitionLocations ToolsetDefinitionLocations { get { throw null; } set { } } public System.Collections.Generic.ICollection Toolsets { get { throw null; } } - public bool TrackReadsOnEnvironmentVariables { get { throw null; } set { } } public System.Globalization.CultureInfo UICulture { get { throw null; } set { } } public bool UseSynchronousLogging { get { throw null; } set { } } public System.Collections.Generic.ISet WarningsAsErrors { get { throw null; } set { } } @@ -1423,6 +1422,7 @@ public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDiction public ProjectGraph(string entryProjectFile, System.Collections.Generic.IDictionary globalProperties, Microsoft.Build.Evaluation.ProjectCollection projectCollection) { } public System.Collections.Generic.IReadOnlyCollection EntryPointNodes { get { throw null; } } public System.Collections.Generic.IEnumerable EnvironmentVariableReads { get { throw null; } } + public System.Collections.Generic.IEnumerable EnvironmentVariablesImpactingBuild { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection GraphRoots { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodes { get { throw null; } } public System.Collections.Generic.IReadOnlyCollection ProjectNodesTopologicallySorted { get { throw null; } } diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index ff25a21c0df..f5f3d1973a4 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -1957,7 +1957,7 @@ public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() $(Path) - $(SomeTestEnvVar) + $(SomeTestEnvVar) $(ComputerName) "); @@ -1968,7 +1968,10 @@ public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() var first = graph.ProjectNodes.First(); first.ProjectInstance.ShouldNotBeNull(); first.ProjectInstance.EnvironmentVariableReads.ShouldNotBeNull(); - first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(3); + + string readEnvVars = string.Join(", ", first.ProjectInstance.EnvironmentVariableReads); + string allEnvVars = string.Join(", ", first.ProjectInstance.TestEnvironmentalProperties.Select(p => p.Name)); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(3, $"All: {allEnvVars} | Read: {readEnvVars}"); } } diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs index 753f04ab9ac..b19980a2b61 100644 --- a/src/Build/BackEnd/BuildManager/BuildParameters.cs +++ b/src/Build/BackEnd/BuildManager/BuildParameters.cs @@ -96,11 +96,6 @@ public class BuildParameters : ITranslatable /// private static string s_msbuildExeKnownToExistAt; - /// - /// Specifies whether or not MSBuild will track environment variable reads - /// - private static bool? s_trackReadsOnEnvironmentVariables; - /// /// The build id /// @@ -768,15 +763,6 @@ public string OutputResultsCacheFile set => _outputResultsCacheFile = value; } - /// - /// Track the environment variables actually read and used. - /// - public bool TrackReadsOnEnvironmentVariables - { - get => GetStaticBoolVariableOrDefault("MSBUILDTRACKENVVARREADS", ref s_trackReadsOnEnvironmentVariables, false); - set => s_trackReadsOnEnvironmentVariables = value; - } - /// /// Retrieves a toolset. /// diff --git a/src/Build/Evaluation/IEvaluatorData.cs b/src/Build/Evaluation/IEvaluatorData.cs index 8ba17db92ef..ce3e4d2d085 100644 --- a/src/Build/Evaluation/IEvaluatorData.cs +++ b/src/Build/Evaluation/IEvaluatorData.cs @@ -263,7 +263,7 @@ List EvaluatedItemElements /// /// Sets a property which does not come from the Xml. /// - P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar = false); + P SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable = false); /// /// Sets a property which comes from the Xml. diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index ca4527cd202..dba21f8163d 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -79,12 +79,65 @@ public delegate ProjectInstance ProjectInstanceFactoryFunc( /// /// Gets a distinct enumeration of all of the environment variables reads during evaluation. /// - public IEnumerable EnvironmentVariableReads => _projectNodesTopologicallySorted?.Value.Select(p => p.ProjectInstance).SelectMany(pi => pi.EnvironmentVariableReads).Distinct(); + public IEnumerable EnvironmentVariableReads + { + get + { + if (!ProjectNodes.Any()) + { + return Enumerable.Empty(); + } + + var distinctReads = new HashSet(); + + foreach (ProjectGraphNode projectNode in ProjectNodes) + { + foreach (string environmentVariableRead in projectNode.ProjectInstance.EnvironmentVariableReads) + { + if (!distinctReads.Contains(environmentVariableRead)) + { + distinctReads.Add(environmentVariableRead); + } + } + } + + return distinctReads; + } + } /// /// Get a distinct enumeration of all the uninitialized property reads during evaluation. /// - public IEnumerable UninitializedPropertyReads => _projectNodesTopologicallySorted?.Value.Select(p => p.ProjectInstance).SelectMany(pi => pi.UninitializedPropertyReads).Distinct(); + public IEnumerable UninitializedPropertyReads + { + get + { + if (!ProjectNodes.Any()) + { + return Enumerable.Empty(); + } + + var distinctUninitializedProperties = new HashSet(); + + foreach (ProjectGraphNode projectNode in ProjectNodes) + { + foreach (string uninitializedPropertyRead in projectNode.ProjectInstance.UninitializedPropertyReads) + { + if (!distinctUninitializedProperties.Contains(uninitializedPropertyRead)) + { + distinctUninitializedProperties.Add(uninitializedPropertyRead); + } + } + } + + return distinctUninitializedProperties; + } + } + + /// + /// A list of all potentially impactful environment variables. + /// + public IEnumerable EnvironmentVariablesImpactingBuild => UninitializedPropertyReads.Concat(EnvironmentVariableReads); /// /// Constructs a graph starting from the given project file, evaluating with the global project collection and no diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index ce9c9a02132..4e054f0d31b 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -1329,7 +1329,7 @@ IItemDefinition IEvaluatorData - ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvVar) + ProjectPropertyInstance IEvaluatorData.SetProperty(string name, string evaluatedValueEscaped, bool isGlobalProperty, bool mayBeReserved, bool isEnvironmentVariable) { // Mutability not verified as this is being populated during evaluation ProjectPropertyInstance property = ProjectPropertyInstance.Create(name, evaluatedValueEscaped, mayBeReserved, _isImmutable); @@ -1337,7 +1337,7 @@ ProjectPropertyInstance IEvaluatorData glob _hostServices = buildParameters.HostServices; this.ProjectRootElementCache = buildParameters.ProjectRootElementCache; - _trackPropertyReads = buildParameters.TrackReadsOnEnvironmentVariables; + _trackPropertyReads = Traits.Instance.TrackReadsOnEnvironmentVariables; if (_trackPropertyReads) { _environmentVariables = new HashSet(StringComparer.OrdinalIgnoreCase); @@ -2776,12 +2777,12 @@ private void CreatePropertiesSnapshot(Evaluation.Project.Data data, bool isImmut /// Tracks this property read if it's from an environment variable or uninitialized. /// /// The name of the property being read. - private void TrackPropertyRead(string name, bool propertyMissing) + private void TrackPropertyRead(string name, bool propertyUninitialized) { if (string.IsNullOrEmpty(name)) return; - if (propertyMissing) + if (propertyUninitialized) { if (!_uninitializedPropertyReads.Contains(name)) { diff --git a/src/Shared/Traits.cs b/src/Shared/Traits.cs index a6a6d80f899..51bcc5c76eb 100644 --- a/src/Shared/Traits.cs +++ b/src/Shared/Traits.cs @@ -77,6 +77,11 @@ public Traits() /// public readonly bool LogPropertyFunctionsRequiringReflection = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBuildLogPropertyFunctionsRequiringReflection")); + /// + /// Track the environment variables actually read and used. + /// + public readonly bool TrackReadsOnEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDTRACKENVVARREADS")); + private static int ParseIntFromEnvironmentVariableOrDefault(string environmentVariable, int defaultValue) { return int.TryParse(Environment.GetEnvironmentVariable(environmentVariable), out int result) From b942ce292137ba6418c8d16ae1b5af3bef4e0a4a Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Fri, 14 Jun 2019 11:16:30 -0700 Subject: [PATCH 3/6] Fixed problemmatic unit test. --- src/Build.UnitTests/Graph/ProjectGraph_Tests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index f5f3d1973a4..ee187a6f281 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -1951,13 +1951,11 @@ public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() using (var env = TestEnvironment.Create()) { env.SetEnvironmentVariable("MSBUILDTRACKENVVARREADS", "1"); - env.SetEnvironmentVariable("SomeTestEnvVar", "SomeValue"); var projFile = env.CreateFile($"{Guid.NewGuid()}.proj", @" $(Path) - $(SomeTestEnvVar) $(ComputerName) "); @@ -1971,7 +1969,7 @@ public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() string readEnvVars = string.Join(", ", first.ProjectInstance.EnvironmentVariableReads); string allEnvVars = string.Join(", ", first.ProjectInstance.TestEnvironmentalProperties.Select(p => p.Name)); - first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(3, $"All: {allEnvVars} | Read: {readEnvVars}"); + first.ProjectInstance.EnvironmentVariableReads.Count.ShouldBe(2, $"All: {allEnvVars} | Read: {readEnvVars}"); } } From 21329fb0142cc659dab81b9b53298812f1da670a Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Fri, 14 Jun 2019 14:07:57 -0700 Subject: [PATCH 4/6] Some changes per PR. Fix for unit test on non-Windows environments. --- src/Build.UnitTests/Graph/ProjectGraph_Tests.cs | 2 +- src/Build/Graph/ProjectGraph.cs | 10 ++-------- src/Build/Instance/ProjectInstance.cs | 7 ++----- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index ee187a6f281..1ba327e12b9 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -1956,7 +1956,7 @@ public void EnvironmentVariablesReadReturnsNumActuallyReadMultiple() $(Path) - $(ComputerName) + $(LocalAppData) "); diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index dba21f8163d..eed42518fc6 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -94,10 +94,7 @@ public IEnumerable EnvironmentVariableReads { foreach (string environmentVariableRead in projectNode.ProjectInstance.EnvironmentVariableReads) { - if (!distinctReads.Contains(environmentVariableRead)) - { - distinctReads.Add(environmentVariableRead); - } + distinctReads.Add(environmentVariableRead); } } @@ -123,10 +120,7 @@ public IEnumerable UninitializedPropertyReads { foreach (string uninitializedPropertyRead in projectNode.ProjectInstance.UninitializedPropertyReads) { - if (!distinctUninitializedProperties.Contains(uninitializedPropertyRead)) - { - distinctUninitializedProperties.Add(uninitializedPropertyRead); - } + distinctUninitializedProperties.Add(uninitializedPropertyRead); } } diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index 4e054f0d31b..3d2f0826613 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -1339,11 +1339,8 @@ ProjectPropertyInstance IEvaluatorData Date: Mon, 17 Jun 2019 09:37:49 -0700 Subject: [PATCH 5/6] Removing an unneeded check. --- src/Build/Instance/ProjectInstance.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs index 3d2f0826613..293934a1f39 100644 --- a/src/Build/Instance/ProjectInstance.cs +++ b/src/Build/Instance/ProjectInstance.cs @@ -1342,7 +1342,7 @@ ProjectPropertyInstance IEvaluatorData Date: Tue, 18 Jun 2019 10:13:15 -0700 Subject: [PATCH 6/6] More strict environment variable check for MSBUILDTRACKENVVARREADS. --- src/Shared/Traits.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/Traits.cs b/src/Shared/Traits.cs index 51bcc5c76eb..f8531d998d4 100644 --- a/src/Shared/Traits.cs +++ b/src/Shared/Traits.cs @@ -80,7 +80,7 @@ public Traits() /// /// Track the environment variables actually read and used. /// - public readonly bool TrackReadsOnEnvironmentVariables = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("MSBUILDTRACKENVVARREADS")); + public readonly bool TrackReadsOnEnvironmentVariables = Environment.GetEnvironmentVariable("MSBUILDTRACKENVVARREADS") == "1"; private static int ParseIntFromEnvironmentVariableOrDefault(string environmentVariable, int defaultValue) {