From 5e158602534def123a38e947d0236fc9749b8b77 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Mon, 22 Jul 2019 11:52:18 -0700 Subject: [PATCH 01/11] Initial changes to read property-related build log events --- .../ProjectGraphWithPredictionsResult.cs | 17 ++-- .../MsBuildGraphBuilder.cs | 12 ++- .../PropertyTrackingLogger.cs | 77 +++++++++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) create mode 100644 Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs diff --git a/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs b/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs index 685a640c9a..2a2bcad8f4 100644 --- a/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs +++ b/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; using System.Diagnostics.ContractsLight; using Newtonsoft.Json; @@ -48,12 +49,17 @@ public readonly struct ProjectGraphWithPredictionsResult /// public bool Succeeded { get; } + /// + /// A sorted list of the names of environment variables that could affect the build. + /// + public IEnumerable EnvironmentVariablesAffectingBuild { get; } + /// - public static ProjectGraphWithPredictionsResult CreateSuccessfulGraph(ProjectGraphWithPredictions projectGraphWithPredictions, IReadOnlyDictionary assemblyPathsToLoad, TPathType pathToMsBuild) + public static ProjectGraphWithPredictionsResult CreateSuccessfulGraph(ProjectGraphWithPredictions projectGraphWithPredictions, IReadOnlyDictionary assemblyPathsToLoad, TPathType pathToMsBuild, IEnumerable environmentVariablesAffectingBuild) { Contract.Requires(projectGraphWithPredictions != null); Contract.Requires(assemblyPathsToLoad != null); - return new ProjectGraphWithPredictionsResult(projectGraphWithPredictions, failure: default, msBuildAssemblyPaths: assemblyPathsToLoad, pathToMsBuild: pathToMsBuild, pathToDotNetExe: default(TPathType), succeeded: true); + return new ProjectGraphWithPredictionsResult(projectGraphWithPredictions, failure: default, msBuildAssemblyPaths: assemblyPathsToLoad, pathToMsBuild: pathToMsBuild, pathToDotNetExe: default(TPathType), succeeded: true, environmentVariablesAffectingBuild); } /// @@ -61,7 +67,7 @@ public static ProjectGraphWithPredictionsResult CreateFailure(GraphCo { Contract.Requires(failure != null); Contract.Requires(assemblyPathsToLoad != null); - return new ProjectGraphWithPredictionsResult(default, failure, assemblyPathsToLoad, pathToMsBuild, pathToDotNetExe: default(TPathType), succeeded: false); + return new ProjectGraphWithPredictionsResult(default, failure, assemblyPathsToLoad, pathToMsBuild, pathToDotNetExe: default(TPathType), succeeded: false, environmentVariablesAffectingBuild: null); } /// @@ -69,11 +75,11 @@ public static ProjectGraphWithPredictionsResult CreateFailure(GraphCo /// public ProjectGraphWithPredictionsResult WithPathToDotNetExe(TPathType pathToDotNetExe) { - return new ProjectGraphWithPredictionsResult(Result, Failure, MsBuildAssemblyPaths, PathToMsBuild, pathToDotNetExe, Succeeded); + return new ProjectGraphWithPredictionsResult(Result, Failure, MsBuildAssemblyPaths, PathToMsBuild, pathToDotNetExe, Succeeded, EnvironmentVariablesAffectingBuild); } [JsonConstructor] - private ProjectGraphWithPredictionsResult(ProjectGraphWithPredictions result, GraphConstructionError failure, IReadOnlyDictionary msBuildAssemblyPaths, TPathType pathToMsBuild, TPathType pathToDotNetExe, bool succeeded) + private ProjectGraphWithPredictionsResult(ProjectGraphWithPredictions result, GraphConstructionError failure, IReadOnlyDictionary msBuildAssemblyPaths, TPathType pathToMsBuild, TPathType pathToDotNetExe, bool succeeded, IEnumerable environmentVariablesAffectingBuild) { Result = result; Failure = failure; @@ -81,6 +87,7 @@ private ProjectGraphWithPredictionsResult(ProjectGraphWithPredictions PathToMsBuild = pathToMsBuild; PathToDotNetExe = pathToDotNetExe; MsBuildAssemblyPaths = msBuildAssemblyPaths; + EnvironmentVariablesAffectingBuild = environmentVariablesAffectingBuild; } } } diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs index da29364146..b3a757ccb7 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs @@ -24,6 +24,8 @@ using ProjectGraphWithPredictions = BuildXL.FrontEnd.MsBuild.Serialization.ProjectGraphWithPredictions; using ProjectWithPredictions = BuildXL.FrontEnd.MsBuild.Serialization.ProjectWithPredictions; using BuildXL.Utilities.Collections; +using Microsoft.Build.Framework; +using ProjectGraphBuilder; namespace MsBuildGraphBuilderTool { @@ -144,10 +146,12 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( locatedMsBuildPath); } + var environmentVariablesLogger = new PropertyTrackingLogger(); + var projectGraph = new ProjectGraph( entryPoints, // The project collection doesn't need any specific global properties, since entry points already contain all the ones that are needed, and the project graph will merge them - new ProjectCollection(), + new ProjectCollection(globalProperties: null, loggers: new ILogger[]{ environmentVariablesLogger }, ToolsetDefinitionLocations.Default), (projectPath, globalProps, projectCollection) => ProjectInstanceFactory(projectPath, globalProps, projectCollection, projectInstanceToProjectCache)); // This is a defensive check to make sure the assembly loader actually honored the search locations provided by the user. The path of the assembly where ProjectGraph @@ -182,7 +186,11 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( locatedMsBuildPath); } - return ProjectGraphWithPredictionsResult.CreateSuccessfulGraph(projectGraphWithPredictions, assemblyPathsToLoad, locatedMsBuildPath); + return ProjectGraphWithPredictionsResult.CreateSuccessfulGraph( + projectGraphWithPredictions, + assemblyPathsToLoad, + locatedMsBuildPath, + environmentVariablesLogger.PotentialEnvironmentVariablesReads.OrderBy(s => s)); } catch (InvalidProjectFileException e) { diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs new file mode 100644 index 0000000000..62982e20da --- /dev/null +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using BuildXL.Utilities.Collections; +using Microsoft.Build.Framework; + +namespace ProjectGraphBuilder +{ + internal class PropertyTrackingLogger : ILogger + { + private readonly HashSet _variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); + private bool _nonTrackingSdkResolversExist = false; + + public IEnumerable PotentialEnvironmentVariablesReads + { + get + { + // Return a copy of what's there to minimize threading issues. + lock (_variablesRead) + { + return _variablesRead.ToArray(); + } + } + } + + public void Initialize(IEventSource eventSource) + { + eventSource.AnyEventRaised += (sender, args) => + { + if (args is EnvironmentVariableReadEventArgs evrArgs) + { + lock(_variablesRead) + { + _variablesRead.Add(evrArgs.EnvironmentVariableName); + } + } + else if (args is UninitializedPropertyReadEventArgs upArgs) + { + lock(_variablesRead) + { + _variablesRead.Add(upArgs.PropertyName); + } + } + else if (args is BuildWarningEventArgs warnArgs && warnArgs.Code == "MSB4263") + { + // We just want to know whether or not we can trust the above tracking data. + _nonTrackingSdkResolversExist = true; + } + }; + } + + public void Shutdown() + { + } + + public LoggerVerbosity Verbosity { get; set; } + + public string Parameters { get; set; } + + public IReadOnlyCollection BuildAffectingEnvironmentVariables + { + get + { + IEnumerable copy; + + lock(_variablesRead) + { + copy = _variablesRead.ToArray(); + } + + return copy.AsReadOnlyCollection(); + } + } + + public bool NonTrackingSdkResolversExist => _nonTrackingSdkResolversExist; + } +} From c10418dbb102aec4644f18ace184b07179e1a244 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Tue, 23 Jul 2019 05:56:21 -0700 Subject: [PATCH 02/11] Adding env vars to ProjectGraphResult. --- Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs | 2 +- Public/Src/FrontEnd/MsBuild/ProjectGraphResult.cs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs b/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs index 0dd9f191bd..f42c232350 100644 --- a/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs +++ b/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs @@ -424,7 +424,7 @@ private async Task> TryComputeBuildGraphAsync(IEnum allowedModuleDependencies: null, // no module policies cyclicalFriendModules: null); // no whitelist of cycles - return new ProjectGraphResult(projectGraph, moduleDefinition, projectGraphResult.PathToMsBuild, projectGraphResult.PathToDotNetExe); + return new ProjectGraphResult(projectGraph, moduleDefinition, projectGraphResult.PathToMsBuild, projectGraphResult.PathToDotNetExe, projectGraphResult.EnvironmentVariablesAffectingBuild); } private void DeleteGraphBuilderRelatedFiles(AbsolutePath outputFile, AbsolutePath responseFile) diff --git a/Public/Src/FrontEnd/MsBuild/ProjectGraphResult.cs b/Public/Src/FrontEnd/MsBuild/ProjectGraphResult.cs index 1739af0962..33c44955c8 100644 --- a/Public/Src/FrontEnd/MsBuild/ProjectGraphResult.cs +++ b/Public/Src/FrontEnd/MsBuild/ProjectGraphResult.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.Collections.Generic; using System.Diagnostics.ContractsLight; using BuildXL.FrontEnd.MsBuild.Serialization; using BuildXL.FrontEnd.Workspaces.Core; @@ -27,7 +28,10 @@ public readonly struct ProjectGraphResult public AbsolutePath DotNetExeLocation { get; } /// - public ProjectGraphResult(ProjectGraphWithPredictions projectGraphWithPredictions, ModuleDefinition moduleDefinition, AbsolutePath msBuildLocation, AbsolutePath dotnetExeLocation) + public IEnumerable EnvironmentVariablesAffectingBuild { get; } + + /// + public ProjectGraphResult(ProjectGraphWithPredictions projectGraphWithPredictions, ModuleDefinition moduleDefinition, AbsolutePath msBuildLocation, AbsolutePath dotnetExeLocation, IEnumerable environmentVariablesAffectingBuild) { Contract.Requires(projectGraphWithPredictions != null); Contract.Requires(moduleDefinition != null); @@ -37,6 +41,7 @@ public ProjectGraphResult(ProjectGraphWithPredictions projectGraph ModuleDefinition = moduleDefinition; MsBuildLocation = msBuildLocation; DotNetExeLocation = dotnetExeLocation; + EnvironmentVariablesAffectingBuild = environmentVariablesAffectingBuild; } } } From 5ea6bc1a3adcea2645958a1d8cd11649aa4a4ae7 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Wed, 24 Jul 2019 09:21:08 -0700 Subject: [PATCH 03/11] Some changes per PR feedback. --- .../ProjectGraphWithPredictionsResult.cs | 2 +- .../Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs | 2 +- .../Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs b/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs index 2a2bcad8f4..9136b9092d 100644 --- a/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs +++ b/Public/Src/FrontEnd/MsBuild.Serialization/ProjectGraphWithPredictionsResult.cs @@ -50,7 +50,7 @@ public readonly struct ProjectGraphWithPredictionsResult public bool Succeeded { get; } /// - /// A sorted list of the names of environment variables that could affect the build. + /// A list of the names of environment variables that could affect the build. /// public IEnumerable EnvironmentVariablesAffectingBuild { get; } diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs index b3a757ccb7..86d278a93a 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs @@ -190,7 +190,7 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( projectGraphWithPredictions, assemblyPathsToLoad, locatedMsBuildPath, - environmentVariablesLogger.PotentialEnvironmentVariablesReads.OrderBy(s => s)); + environmentVariablesLogger.PotentialEnvironmentVariablesReads); } catch (InvalidProjectFileException e) { diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index 62982e20da..d044ebdf53 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -1,4 +1,7 @@ -using System; +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; using System.Collections.Generic; using System.Linq; using BuildXL.Utilities.Collections; @@ -6,7 +9,7 @@ namespace ProjectGraphBuilder { - internal class PropertyTrackingLogger : ILogger + internal sealed class PropertyTrackingLogger : ILogger { private readonly HashSet _variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); private bool _nonTrackingSdkResolversExist = false; From 021f94b9955ab3f7957347ac0157c159626bf8e4 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Wed, 24 Jul 2019 09:27:52 -0700 Subject: [PATCH 04/11] Some more changes per PR feedback. --- .../PropertyTrackingLogger.cs | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index d044ebdf53..46d1172f8c 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -9,19 +9,22 @@ namespace ProjectGraphBuilder { + /// + /// + /// internal sealed class PropertyTrackingLogger : ILogger { - private readonly HashSet _variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); - private bool _nonTrackingSdkResolversExist = false; + private readonly HashSet m_variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); + private bool m_nonTrackingSdkResolversExist = false; public IEnumerable PotentialEnvironmentVariablesReads { get { // Return a copy of what's there to minimize threading issues. - lock (_variablesRead) + lock (m_variablesRead) { - return _variablesRead.ToArray(); + return m_variablesRead.ToArray(); } } } @@ -32,22 +35,22 @@ public void Initialize(IEventSource eventSource) { if (args is EnvironmentVariableReadEventArgs evrArgs) { - lock(_variablesRead) + lock(m_variablesRead) { - _variablesRead.Add(evrArgs.EnvironmentVariableName); + m_variablesRead.Add(evrArgs.EnvironmentVariableName); } } else if (args is UninitializedPropertyReadEventArgs upArgs) { - lock(_variablesRead) + lock(m_variablesRead) { - _variablesRead.Add(upArgs.PropertyName); + m_variablesRead.Add(upArgs.PropertyName); } } else if (args is BuildWarningEventArgs warnArgs && warnArgs.Code == "MSB4263") { // We just want to know whether or not we can trust the above tracking data. - _nonTrackingSdkResolversExist = true; + m_nonTrackingSdkResolversExist = true; } }; } @@ -64,17 +67,17 @@ public IReadOnlyCollection BuildAffectingEnvironmentVariables { get { - IEnumerable copy; + string[] copy; - lock(_variablesRead) + lock(m_variablesRead) { - copy = _variablesRead.ToArray(); + copy = m_variablesRead.ToArray(); } - return copy.AsReadOnlyCollection(); + return copy; } } - public bool NonTrackingSdkResolversExist => _nonTrackingSdkResolversExist; + public bool NonTrackingSdkResolversExist => m_nonTrackingSdkResolversExist; } } From 1a23a0777e6bab7cbcc5c8709c2b8a6678717832 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Mon, 29 Jul 2019 08:38:44 -0700 Subject: [PATCH 05/11] Additions made for PR feedback. --- .../MsBuildGraphBuilder.cs | 11 ++++-- .../PropertyTrackingLogger.cs | 35 +++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs index 86d278a93a..a0b533cc06 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs @@ -12,6 +12,7 @@ using System.Reflection; using System.Threading.Tasks; using BuildXL.FrontEnd.MsBuild.Serialization; +using BuildXL.Utilities; using BuildXL.Utilities.Instrumentation.Common; using Microsoft.Build.Definition; using Microsoft.Build.Evaluation; @@ -186,11 +187,17 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( locatedMsBuildPath); } + var envVarResult = environmentVariablesLogger.PotentialEnvironmentVariableReads; + if (!envVarResult.Succeeded) + { + reporter.ReportMessage(envVarResult.Failure.Describe()); + } + return ProjectGraphWithPredictionsResult.CreateSuccessfulGraph( projectGraphWithPredictions, assemblyPathsToLoad, - locatedMsBuildPath, - environmentVariablesLogger.PotentialEnvironmentVariablesReads); + locatedMsBuildPath, + envVarResult.Result); } catch (InvalidProjectFileException e) { diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index 46d1172f8c..715a37cee0 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using BuildXL.Utilities; using BuildXL.Utilities.Collections; using Microsoft.Build.Framework; @@ -15,7 +16,7 @@ namespace ProjectGraphBuilder internal sealed class PropertyTrackingLogger : ILogger { private readonly HashSet m_variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); - private bool m_nonTrackingSdkResolversExist = false; + private readonly HashSet m_sdkResolversNotTrackingEnvVars = new HashSet(); public IEnumerable PotentialEnvironmentVariablesReads { @@ -47,10 +48,12 @@ public void Initialize(IEventSource eventSource) m_variablesRead.Add(upArgs.PropertyName); } } - else if (args is BuildWarningEventArgs warnArgs && warnArgs.Code == "MSB4263") + else if (args is SdkResolverDoesNotTrackEnvironmentVariablesEventArgs trackArgs) { - // We just want to know whether or not we can trust the above tracking data. - m_nonTrackingSdkResolversExist = true; + lock (m_sdkResolversNotTrackingEnvVars) + { + m_sdkResolversNotTrackingEnvVars.Add(trackArgs.SdkResolverName); + } } }; } @@ -63,21 +66,31 @@ public void Shutdown() public string Parameters { get; set; } - public IReadOnlyCollection BuildAffectingEnvironmentVariables + public Possible> PotentialEnvironmentVariableReads { get { - string[] copy; + if (m_sdkResolversNotTrackingEnvVars.Count == 0) + { + string[] copy; + + lock (m_variablesRead) + { + copy = m_variablesRead.ToArray(); + } - lock(m_variablesRead) + return new Possible>(copy); + } + + string errorMessage = null; + + lock (m_sdkResolversNotTrackingEnvVars) { - copy = m_variablesRead.ToArray(); + errorMessage = $"Unreliable environment variable tracking due to SdkResolvers not participating in tracking. Offenders: {string.Join(",", m_sdkResolversNotTrackingEnvVars)}"; } - return copy; + return new Failure(errorMessage); } } - - public bool NonTrackingSdkResolversExist => m_nonTrackingSdkResolversExist; } } From c770383907665f2e1bd486d7840bf078cf5767f0 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Tue, 30 Jul 2019 09:51:13 -0700 Subject: [PATCH 06/11] Work to track the (potential) environment variables determined during evaluation and surface those to the pips. --- .../Src/FrontEnd/MsBuild/MsBuildResolver.cs | 14 ++++++++-- .../MsBuild/MsBuildWorkspaceResolver.cs | 26 ++++++++++++++----- .../MsBuildGraphBuilder.cs | 4 ++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/Public/Src/FrontEnd/MsBuild/MsBuildResolver.cs b/Public/Src/FrontEnd/MsBuild/MsBuildResolver.cs index faae1489b3..f0f81edaca 100644 --- a/Public/Src/FrontEnd/MsBuild/MsBuildResolver.cs +++ b/Public/Src/FrontEnd/MsBuild/MsBuildResolver.cs @@ -156,6 +156,16 @@ private Task EvaluateAllFilesAsync(IReadOnlySet evaluationGo .Where(project => ProjectMatchesQualifier(project, qualifier)) .ToReadOnlySet(); + IEnumerable> pipEnvironment = m_msBuildWorkspaceResolver.UserDefinedEnvironment; + + // If a unique environment wasn't specified and the project graph determined the environment variables affecting the build, + // we're going to give the subset of those environment variables to the Pip. + if (m_msBuildResolverSettings.Environment == null && result.EnvironmentVariablesAffectingBuild != null) + { + var graphedEnvVars = result.EnvironmentVariablesAffectingBuild.ToHashSet(); + pipEnvironment = m_msBuildWorkspaceResolver.UserDefinedEnvironment.Where(kvp => graphedEnvVars.Contains(kvp.Key)); + } + var graphConstructor = new PipGraphConstructor( m_context, m_host, @@ -163,8 +173,8 @@ private Task EvaluateAllFilesAsync(IReadOnlySet evaluationGo m_msBuildResolverSettings, result.MsBuildLocation, result.DotNetExeLocation, - m_frontEndName, - m_msBuildWorkspaceResolver.UserDefinedEnvironment, + m_frontEndName, + pipEnvironment, m_msBuildWorkspaceResolver.UserDefinedPassthroughVariables); return graphConstructor.TrySchedulePipsForFilesAsync(filteredBuildFiles, qualifierId); diff --git a/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs b/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs index f42c232350..c2564ba22d 100644 --- a/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs +++ b/Public/Src/FrontEnd/MsBuild/MsBuildWorkspaceResolver.cs @@ -538,8 +538,6 @@ private async Task>> Co standardError); } - TrackFilesAndEnvironment(result.AllUnexpectedFileAccesses, outputFile.GetParent(m_context.PathTable)); - var serializer = JsonSerializer.Create(ProjectGraphSerializationSettings.Settings); serializer.Converters.Add(new AbsolutePathJsonConverter(m_context.PathTable)); serializer.Converters.Add(new ValidAbsolutePathEnumerationJsonConverter()); @@ -549,6 +547,8 @@ private async Task>> Co { var projectGraphWithPredictionsResult = serializer.Deserialize>(reader); + TrackFilesAndEnvironment(result.AllUnexpectedFileAccesses, outputFile.GetParent(m_context.PathTable), projectGraphWithPredictionsResult.EnvironmentVariablesAffectingBuild); + // A successfully constructed graph should always have a valid path to MsBuild Contract.Assert(!projectGraphWithPredictionsResult.Succeeded || projectGraphWithPredictionsResult.PathToMsBuild.IsValid); // A successfully constructed graph should always have at least one project node @@ -586,15 +586,27 @@ private bool TryFindDotNetExe(IEnumerable dotnetSearchLocations, o return false; } - private void TrackFilesAndEnvironment(ISet fileAccesses, AbsolutePath frontEndFolder) + private void TrackFilesAndEnvironment(ISet fileAccesses, AbsolutePath frontEndFolder, IEnumerable environmentVariablesAffectingBuild) { // Register all build parameters passed to the graph construction process // Observe passthrough variables are explicitly skipped: we don't want the engine to track them - // TODO: we actually need the build parameters *used* by the graph construction process, but for now this is a compromise to keep - // graph caching sound. We need to modify this when MsBuild static graph API starts providing used env vars. - foreach (string key in m_userDefinedEnvironment.Keys) + + // If a specific environment wasn't passed in AND if the graph's env vars parameter is NOT null + // (meaning we successfully retrieved the list of environment variables that affect + // the build during evaluation), then tell the engine to track them. + if (m_resolverSettings.Environment == null && environmentVariablesAffectingBuild != null) + { + foreach (string environmentVariable in environmentVariablesAffectingBuild) + { + m_host.Engine.TryGetBuildParameter(environmentVariable, MsBuildFrontEnd.Name, out _); + } + } + else { - m_host.Engine.TryGetBuildParameter(key, MsBuildFrontEnd.Name, out _); + foreach (string key in m_userDefinedEnvironment.Keys) + { + m_host.Engine.TryGetBuildParameter(key, MsBuildFrontEnd.Name, out _); + } } FrontEndUtilities.TrackToolFileAccesses(m_host.Engine, m_context, MsBuildFrontEnd.Name, fileAccesses, frontEndFolder); diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs index a0b533cc06..ccebdb0639 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs @@ -188,6 +188,8 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( } var envVarResult = environmentVariablesLogger.PotentialEnvironmentVariableReads; + var reads = envVarResult.Succeeded ? envVarResult.Result : (IReadOnlyCollection)null; + if (!envVarResult.Succeeded) { reporter.ReportMessage(envVarResult.Failure.Describe()); @@ -197,7 +199,7 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( projectGraphWithPredictions, assemblyPathsToLoad, locatedMsBuildPath, - envVarResult.Result); + reads); } catch (InvalidProjectFileException e) { From 5323b563f2a8ee5e6048e3634cbbe1f88c8de988 Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Mon, 5 Aug 2019 10:40:25 -0700 Subject: [PATCH 07/11] Some more changes per PR feedback. Some more comments. --- .../MsBuildGraphBuilder.cs | 2 + .../PropertyTrackingLogger.cs | 46 +++++-------------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs index ccebdb0639..394eda5ff3 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/MsBuildGraphBuilder.cs @@ -187,6 +187,8 @@ private static ProjectGraphWithPredictionsResult BuildGraphInternal( locatedMsBuildPath); } + // Get the possible environment variable reads from the logger. + // If there was a failure, report the failure message. var envVarResult = environmentVariablesLogger.PotentialEnvironmentVariableReads; var reads = envVarResult.Succeeded ? envVarResult.Result : (IReadOnlyCollection)null; diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index 715a37cee0..5f551f70d0 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.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.Concurrent; using System.Collections.Generic; using System.Linq; using BuildXL.Utilities; @@ -11,24 +12,12 @@ namespace ProjectGraphBuilder { /// - /// + /// Logger that handles property tracking events. /// internal sealed class PropertyTrackingLogger : ILogger { - private readonly HashSet m_variablesRead = new HashSet(StringComparer.OrdinalIgnoreCase); - private readonly HashSet m_sdkResolversNotTrackingEnvVars = new HashSet(); - - public IEnumerable PotentialEnvironmentVariablesReads - { - get - { - // Return a copy of what's there to minimize threading issues. - lock (m_variablesRead) - { - return m_variablesRead.ToArray(); - } - } - } + private readonly ConcurrentDictionary m_variablesRead = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary m_sdkResolversNotTrackingEnvVars = new ConcurrentDictionary(); public void Initialize(IEventSource eventSource) { @@ -36,24 +25,15 @@ public void Initialize(IEventSource eventSource) { if (args is EnvironmentVariableReadEventArgs evrArgs) { - lock(m_variablesRead) - { - m_variablesRead.Add(evrArgs.EnvironmentVariableName); - } + m_variablesRead[evrArgs.EnvironmentVariableName] = null; } else if (args is UninitializedPropertyReadEventArgs upArgs) { - lock(m_variablesRead) - { - m_variablesRead.Add(upArgs.PropertyName); - } + m_variablesRead[upArgs.PropertyName] = null; } else if (args is SdkResolverDoesNotTrackEnvironmentVariablesEventArgs trackArgs) { - lock (m_sdkResolversNotTrackingEnvVars) - { - m_sdkResolversNotTrackingEnvVars.Add(trackArgs.SdkResolverName); - } + m_sdkResolversNotTrackingEnvVars[trackArgs.SdkResolverName] = null; } }; } @@ -66,20 +46,16 @@ public void Shutdown() public string Parameters { get; set; } + /// + /// The list of environment variables that would be read by the build system if present. + /// public Possible> PotentialEnvironmentVariableReads { get { if (m_sdkResolversNotTrackingEnvVars.Count == 0) { - string[] copy; - - lock (m_variablesRead) - { - copy = m_variablesRead.ToArray(); - } - - return new Possible>(copy); + return new Possible>(m_variablesRead.Keys.ToArray()); } string errorMessage = null; From 6285ebab520efdcfaf722287183df155fa1772da Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Tue, 13 Aug 2019 07:42:04 -0700 Subject: [PATCH 08/11] Reference BuildXL.Utilities to access the Possible<> class. --- .../Tools/Tool.MsBuildGraphBuilder/Tool.MsBuildGraphBuilder.dsc | 1 + 1 file changed, 1 insertion(+) diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/Tool.MsBuildGraphBuilder.dsc b/Public/Src/Tools/Tool.MsBuildGraphBuilder/Tool.MsBuildGraphBuilder.dsc index ff0120a13f..f1f489c6c7 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/Tool.MsBuildGraphBuilder.dsc +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/Tool.MsBuildGraphBuilder.dsc @@ -23,6 +23,7 @@ namespace MsBuildGraphBuilder { importFrom("BuildXL.FrontEnd").MsBuild.Serialization.dll, importFrom("BuildXL.Utilities").Collections.dll, importFrom("BuildXL.Utilities").Native.dll, + importFrom("BuildXL.Utilities").dll, importFrom("BuildXL.Utilities.Instrumentation").Common.dll, ...addIf(BuildXLSdk.isFullFramework, importFrom("System.Collections.Immutable").pkg), ...addIf(BuildXLSdk.isFullFramework, importFrom("System.Threading.Tasks.Dataflow").pkg), From a587ee25da073ab8bffef47b29ac015f43fc1f9e Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Thu, 15 Aug 2019 13:32:00 -0700 Subject: [PATCH 09/11] Testing 123... --- .../Test.Tool.MsBuildGraphBuilder.dsc | 2 ++ config.dsc | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc index 6395788c55..fdd971bd05 100644 --- a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc @@ -12,6 +12,7 @@ namespace Test.Tool.MsBuildGraphBuilder { export const dll = BuildXLSdk.test({ assemblyName: "Test.Tool.ProjectGraphBuilder", sources: globR(d`.`, "*.cs"), + appConfig: f`app.config`, testFramework: importFrom("Sdk.Managed.Testing.XUnit").framework, references:[ importFrom("BuildXL.Tools").MsBuildGraphBuilder.exe, @@ -19,6 +20,7 @@ namespace Test.Tool.MsBuildGraphBuilder { importFrom("Newtonsoft.Json").pkg, importFrom("BuildXL.FrontEnd").MsBuild.Serialization.dll, ...MSBuild.msbuildReferences, + BuildXLSdk.Factory.createAssembly(importFrom("System.Memory").Contents.all, r`lib/netstandard2.0/System.memory.dll`), ], runtimeContent: [ ...MSBuild.msbuildRuntimeContent, diff --git a/config.dsc b/config.dsc index c4e08a18a2..13b35b121a 100644 --- a/config.dsc +++ b/config.dsc @@ -48,6 +48,7 @@ config({ "BuildXL.Selfhost": "https://pkgs.dev.azure.com/cloudbuild/_packaging/BuildXL.Selfhost/nuget/v3/index.json", // Note: From a compliance point of view it is important that MicrosoftInternal has a single feed. // If you need to consume packages make sure they are upstreamed in that feed. + "msbuild-local" : "file://D:/src/msbuild/artifacts/packages/Debug/Shipping", } : { "buildxl-selfhost" : "https://pkgs.dev.azure.com/ms/BuildXL/_packaging/BuildXL.Selfhost/nuget/v3/index.json", @@ -678,6 +679,14 @@ config({ trackSourceFileChanges: true, isWritable: false, isReadable: true + }, + { + name: a`LocalMsBuildNupkgs`, + path: p`D:/src/msbuild/artifacts/packages/Debug/Shipping`, + trackSourceFileChanges: true, + isWritable: true, + isReadable: true, + isScrubbable: true } ], From 60744322d3748526fa3177ec05a41284a1d67c9b Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Fri, 16 Aug 2019 12:49:47 -0700 Subject: [PATCH 10/11] Getting unit tests for the MsBuildGraphBuilder tool working with latest msbuild. Added tests for property tracking logging. --- .../PropertyTrackingLogger.cs | 9 +- .../UnitTests/MsBuildGraphBuilder/App.config | 11 +++ .../MsBuildGraphProjectPredictionTests.cs | 68 +++++++++++++++ .../MsBuildPropertyTrackingLoggerTests.cs | 87 +++++++++++++++++++ .../Test.Tool.MsBuildGraphBuilder.dsc | 5 +- 5 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 Public/Src/Tools/UnitTests/MsBuildGraphBuilder/App.config create mode 100644 Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildPropertyTrackingLoggerTests.cs diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index 5f551f70d0..bd68b8c72f 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -31,10 +31,11 @@ public void Initialize(IEventSource eventSource) { m_variablesRead[upArgs.PropertyName] = null; } - else if (args is SdkResolverDoesNotTrackEnvironmentVariablesEventArgs trackArgs) - { - m_sdkResolversNotTrackingEnvVars[trackArgs.SdkResolverName] = null; - } + // This is for future work being done. + //else if (args is SdkResolverDoesNotTrackEnvironmentVariablesEventArgs trackArgs) + //{ + // m_sdkResolversNotTrackingEnvVars[trackArgs.SdkResolverName] = null; + //} }; } diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/App.config b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/App.config new file mode 100644 index 0000000000..d5a4ff5b8f --- /dev/null +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/App.config @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs index 7d3845d228..3e14c66caf 100644 --- a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs @@ -92,5 +92,73 @@ public void ProblematicPredictorsAreHandled() Assert.False(result.Succeeded); Assert.True(result.Failure.Message != null); } + + [Fact] + public void PropertyTrackingIgnoredWhenDisabled() + { + string outputFile = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); + string entryPoint = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); + + // Create an environment variable with a unique name. + string envVarName = Guid.NewGuid().ToString().Replace("-", string.Empty); + Environment.SetEnvironmentVariable(envVarName, "value not important"); + + File.WriteAllText(entryPoint, + "$(" + envVarName + ")"); + + MsBuildGraphBuilder.BuildGraphAndSerialize( + GetStandardBuilderArguments( + new[] { entryPoint }, + outputFile, + globalProperties: GlobalProperties.Empty, + entryPointTargets: new string[0], + requestedQualifiers: new GlobalProperties[] { GlobalProperties.Empty }, + allowProjectsWithoutTargetProtocol: false)); + + // Remove the environment variable. + Environment.SetEnvironmentVariable(envVarName, string.Empty); + + var result = SimpleDeserializer.Instance.DeserializeGraph(outputFile); + Assert.True(result.Succeeded); + + // There should have been no tracking. + Assert.NotNull(result.EnvironmentVariablesAffectingBuild); + Assert.Equal(0, result.EnvironmentVariablesAffectingBuild.Count()); + } + + [Fact] + public void PropertyTrackingWorksWhenEnabled() + { + string outputFile = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); + string entryPoint = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); + + // Create an environment variable with a unique name. + string envVarName = Guid.NewGuid().ToString().Replace("-", string.Empty); + Environment.SetEnvironmentVariable(envVarName, "value not important"); + Environment.SetEnvironmentVariable("MsBuildLogPropertyTracking", "15"); // 15 = Log all property tracking events. + + File.WriteAllText(entryPoint, + "$(" + envVarName + ")"); + + MsBuildGraphBuilder.BuildGraphAndSerialize( + GetStandardBuilderArguments( + new[] { entryPoint }, + outputFile, + globalProperties: GlobalProperties.Empty, + entryPointTargets: new string[0], + requestedQualifiers: new GlobalProperties[] { GlobalProperties.Empty }, + allowProjectsWithoutTargetProtocol: false)); + + // Remove the environment variables. + Environment.SetEnvironmentVariable(envVarName, string.Empty); + Environment.SetEnvironmentVariable("MsBuildLogPropertyTracking", string.Empty); + + var result = SimpleDeserializer.Instance.DeserializeGraph(outputFile); + Assert.True(result.Succeeded); + + // Tracking should have reported the set (and read) env var. + Assert.NotNull(result.EnvironmentVariablesAffectingBuild); + Assert.True(result.EnvironmentVariablesAffectingBuild.Contains(envVarName)); + } } } \ No newline at end of file diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildPropertyTrackingLoggerTests.cs b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildPropertyTrackingLoggerTests.cs new file mode 100644 index 0000000000..60bcffbd56 --- /dev/null +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildPropertyTrackingLoggerTests.cs @@ -0,0 +1,87 @@ +using System; +using System.Linq; +using Microsoft.Build.Framework; +using ProjectGraphBuilder; +using Test.BuildXL.TestUtilities.Xunit; +using Xunit; +using Xunit.Abstractions; + +namespace Test.Tool.ProjectGraphBuilder +{ + public class MsBuildPropertyTrackingLoggerTests : XunitBuildXLTest + { + public MsBuildPropertyTrackingLoggerTests(ITestOutputHelper output) : base(output) { } + + [Fact] + public void TracksEnvironmentVariableReads() + { + var eventSource = new TestEventSource(); + var logger = new PropertyTrackingLogger(); + logger.Initialize(eventSource); + + string envVarName = Guid.NewGuid().ToString(); + string message = "unused"; + eventSource.FireAnyEvent(new EnvironmentVariableReadEventArgs(envVarName, message)); + + Assert.NotNull(logger.PotentialEnvironmentVariableReads); + Assert.True(logger.PotentialEnvironmentVariableReads.Succeeded); + Assert.NotNull(logger.PotentialEnvironmentVariableReads.Result); + Assert.True(logger.PotentialEnvironmentVariableReads.Result.Contains(envVarName)); + Assert.Equal(1, logger.PotentialEnvironmentVariableReads.Result.Count); + } + + [Fact] + public void TracksUninitializedPropertyReads() + { + var eventSource = new TestEventSource(); + var logger = new PropertyTrackingLogger(); + logger.Initialize(eventSource); + + string propertyName = Guid.NewGuid().ToString(); + string message = "unused"; + eventSource.FireAnyEvent(new UninitializedPropertyReadEventArgs(propertyName, message)); + + Assert.NotNull(logger.PotentialEnvironmentVariableReads); + Assert.True(logger.PotentialEnvironmentVariableReads.Succeeded); + Assert.NotNull(logger.PotentialEnvironmentVariableReads.Result); + Assert.True(logger.PotentialEnvironmentVariableReads.Result.Contains(propertyName)); + Assert.Equal(1, logger.PotentialEnvironmentVariableReads.Result.Count); + } + + private class TestEventSource : IEventSource + { + public void FireAnyEvent(BuildEventArgs e) + { + this.AnyEventRaised?.Invoke(this, e); + } + + public event BuildMessageEventHandler MessageRaised; + + public event BuildErrorEventHandler ErrorRaised; + + public event BuildWarningEventHandler WarningRaised; + + public event BuildStartedEventHandler BuildStarted; + + public event BuildFinishedEventHandler BuildFinished; + + public event ProjectStartedEventHandler ProjectStarted; + + public event ProjectFinishedEventHandler ProjectFinished; + + public event TargetStartedEventHandler TargetStarted; + + public event TargetFinishedEventHandler TargetFinished; + + public event TaskStartedEventHandler TaskStarted; + + public event TaskFinishedEventHandler TaskFinished; + + public event CustomBuildEventHandler CustomEventRaised; + + public event BuildStatusEventHandler StatusEventRaised; + + public event AnyEventHandler AnyEventRaised; + } + } +} diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc index fdd971bd05..dea32418c0 100644 --- a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/Test.Tool.MsBuildGraphBuilder.dsc @@ -12,19 +12,18 @@ namespace Test.Tool.MsBuildGraphBuilder { export const dll = BuildXLSdk.test({ assemblyName: "Test.Tool.ProjectGraphBuilder", sources: globR(d`.`, "*.cs"), - appConfig: f`app.config`, + appConfig: f`App.Config`, testFramework: importFrom("Sdk.Managed.Testing.XUnit").framework, references:[ importFrom("BuildXL.Tools").MsBuildGraphBuilder.exe, + importFrom("BuildXL.Utilities").dll, importFrom("Microsoft.Build.Prediction").pkg, importFrom("Newtonsoft.Json").pkg, importFrom("BuildXL.FrontEnd").MsBuild.Serialization.dll, ...MSBuild.msbuildReferences, - BuildXLSdk.Factory.createAssembly(importFrom("System.Memory").Contents.all, r`lib/netstandard2.0/System.memory.dll`), ], runtimeContent: [ ...MSBuild.msbuildRuntimeContent, - ...MSBuild.msbuildReferences, ], runtimeContentToSkip: [ importFrom("System.Threading.Tasks.Dataflow").pkg From 1db14cd8f28aadaf16c719e0b495bb787efe517c Mon Sep 17 00:00:00 2001 From: Matt Neely Date: Fri, 16 Aug 2019 14:16:24 -0700 Subject: [PATCH 11/11] Setting the necessary environment variable to tell msbuild to start tracking properties. --- .../PropertyTrackingLogger.cs | 8 ++++ .../MsBuildGraphProjectPredictionTests.cs | 37 +------------------ 2 files changed, 9 insertions(+), 36 deletions(-) diff --git a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs index bd68b8c72f..12e9a38c24 100644 --- a/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs +++ b/Public/Src/Tools/Tool.MsBuildGraphBuilder/PropertyTrackingLogger.cs @@ -19,6 +19,14 @@ internal sealed class PropertyTrackingLogger : ILogger private readonly ConcurrentDictionary m_variablesRead = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); private readonly ConcurrentDictionary m_sdkResolversNotTrackingEnvVars = new ConcurrentDictionary(); + public PropertyTrackingLogger() + { + // This environment variable enable property tracking in MsBuild. + // Without it, no events would be logged or received. + // A value of "12" tells MsBuild to log events "Environment Variable Read" and "Uninitialized Property Read". + Environment.SetEnvironmentVariable("MsBuildLogPropertyTracking", "12"); + } + public void Initialize(IEventSource eventSource) { eventSource.AnyEventRaised += (sender, args) => diff --git a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs index 3e14c66caf..86a77037c2 100644 --- a/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs +++ b/Public/Src/Tools/UnitTests/MsBuildGraphBuilder/MsBuildGraphProjectPredictionTests.cs @@ -94,7 +94,7 @@ public void ProblematicPredictorsAreHandled() } [Fact] - public void PropertyTrackingIgnoredWhenDisabled() + public void PropertyTrackingIsEnabledAndWorks() { string outputFile = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); string entryPoint = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); @@ -103,40 +103,6 @@ public void PropertyTrackingIgnoredWhenDisabled() string envVarName = Guid.NewGuid().ToString().Replace("-", string.Empty); Environment.SetEnvironmentVariable(envVarName, "value not important"); - File.WriteAllText(entryPoint, - "$(" + envVarName + ")"); - - MsBuildGraphBuilder.BuildGraphAndSerialize( - GetStandardBuilderArguments( - new[] { entryPoint }, - outputFile, - globalProperties: GlobalProperties.Empty, - entryPointTargets: new string[0], - requestedQualifiers: new GlobalProperties[] { GlobalProperties.Empty }, - allowProjectsWithoutTargetProtocol: false)); - - // Remove the environment variable. - Environment.SetEnvironmentVariable(envVarName, string.Empty); - - var result = SimpleDeserializer.Instance.DeserializeGraph(outputFile); - Assert.True(result.Succeeded); - - // There should have been no tracking. - Assert.NotNull(result.EnvironmentVariablesAffectingBuild); - Assert.Equal(0, result.EnvironmentVariablesAffectingBuild.Count()); - } - - [Fact] - public void PropertyTrackingWorksWhenEnabled() - { - string outputFile = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); - string entryPoint = Path.Combine(TemporaryDirectory, Guid.NewGuid().ToString()); - - // Create an environment variable with a unique name. - string envVarName = Guid.NewGuid().ToString().Replace("-", string.Empty); - Environment.SetEnvironmentVariable(envVarName, "value not important"); - Environment.SetEnvironmentVariable("MsBuildLogPropertyTracking", "15"); // 15 = Log all property tracking events. - File.WriteAllText(entryPoint, "$(" + envVarName + ")"); @@ -151,7 +117,6 @@ public void PropertyTrackingWorksWhenEnabled() // Remove the environment variables. Environment.SetEnvironmentVariable(envVarName, string.Empty); - Environment.SetEnvironmentVariable("MsBuildLogPropertyTracking", string.Empty); var result = SimpleDeserializer.Instance.DeserializeGraph(outputFile); Assert.True(result.Succeeded);