Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Allow MSBuildWorkspace to output a binary log #42319

Merged
merged 15 commits into from Mar 31, 2020
Merged

Allow MSBuildWorkspace to output a binary log #42319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4980272
upgrade MSBuild to a version that contains BinaryLogger
jmarolf Mar 10, 2020
3d641d8
add API to produce binary Log
jmarolf Mar 10, 2020
dc48489
allow user to pass in custom msbuild logger
jmarolf Mar 16, 2020
12c9715
fix up package references
jmarolf Mar 16, 2020
827eb12
remove newlines
jmarolf Mar 23, 2020
e65f144
update editorconfig to change using style
jmarolf Mar 23, 2020
49ccccb
nullable annotate methods
jmarolf Mar 23, 2020
997e5db
updating constructors for back-compat
jmarolf Mar 23, 2020
20c6a60
have ProjectBuildManager own logging
jmarolf Mar 23, 2020
e134698
skip test
jmarolf Mar 25, 2020
85471fe
Update src/Workspaces/Core/MSBuild/MSBuild/MSBuildProjectLoader.Worke…
jmarolf Mar 31, 2020
f5fbec2
Update src/Workspaces/Core/MSBuild/MSBuild/MSBuildProjectLoader.Worke…
jmarolf Mar 31, 2020
975b649
Update src/Workspaces/Core/MSBuild/MSBuild/MSBuildProjectLoader.Worke…
jmarolf Mar 31, 2020
3942482
Update src/Workspaces/Core/MSBuild/MSBuild/MSBuildProjectLoader.Worke…
jmarolf Mar 31, 2020
11b67ef
reduce indentation
jmarolf Mar 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .editorconfig
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ indent_size = 2
[*.{cs,vb}]
# Sort using and Import directives with System.* appearing first
dotnet_sort_system_directives_first = true
dotnet_separate_import_directive_groups = false
Copy link
Member

Choose a reason for hiding this comment

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

Missed this earlier, is it intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I get yelled at if I add spaces to the usings so this is an implicit style that wasn't captured in the editorcofig. I see no place where separators in usings exist for roslyn

# Avoid "this." and "Me." if not necessary
dotnet_style_qualification_for_field = false:refactoring
dotnet_style_qualification_for_property = false:refactoring
Expand Down
8 changes: 4 additions & 4 deletions eng/Versions.props
View file
Open in desktop
Expand Up @@ -48,11 +48,11 @@
<FakeSignVersion>0.9.2</FakeSignVersion>
<HumanizerCoreVersion>2.2.0</HumanizerCoreVersion>
<ICSharpCodeDecompilerVersion>5.0.2.5153</ICSharpCodeDecompilerVersion>
<MicrosoftBuildVersion>15.1.548</MicrosoftBuildVersion>
<MicrosoftBuildFrameworkVersion>15.1.548</MicrosoftBuildFrameworkVersion>
<MicrosoftBuildVersion>15.3.409</MicrosoftBuildVersion>
<MicrosoftBuildFrameworkVersion>15.3.409</MicrosoftBuildFrameworkVersion>
<MicrosoftBuildLocatorVersion>1.2.2</MicrosoftBuildLocatorVersion>
<MicrosoftBuildRuntimeVersion>15.1.548</MicrosoftBuildRuntimeVersion>
<MicrosoftBuildTasksCoreVersion>15.1.548</MicrosoftBuildTasksCoreVersion>
<MicrosoftBuildRuntimeVersion>15.3.409</MicrosoftBuildRuntimeVersion>
<MicrosoftBuildTasksCoreVersion>15.3.409</MicrosoftBuildTasksCoreVersion>
<MicrosoftCodeAnalysisAnalyzersVersion>$(RoslynDiagnosticsNugetPackageVersion)</MicrosoftCodeAnalysisAnalyzersVersion>
<MicrosoftCodeAnalysisBuildTasksVersion>2.0.0-rc2-61102-09</MicrosoftCodeAnalysisBuildTasksVersion>
<MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>$(MicrosoftCodeAnalysisTestingVersion)</MicrosoftCodeAnalysisCSharpCodeFixTestingXUnitVersion>
Expand Down
2 changes: 2 additions & 0 deletions src/Tools/AnalyzerRunner/AnalyzerRunner.csproj
View file
Open in desktop
Expand Up @@ -16,7 +16,9 @@
<ItemGroup>
<PackageReference Include="SQLitePCLRaw.bundle_green" Version="$(SQLitePCLRawbundle_greenVersion)" PrivateAssets="all" />
<PackageReference Include="Microsoft.VisualStudio.Composition" Version="$(MicrosoftVisualStudioCompositionVersion)" />
<PackageReference Include="Microsoft.Build" Version="$(MicrosoftBuildVersion)" ExcludeAssets="Runtime" PrivateAssets="All" />
<PackageReference Include="Microsoft.Build.Locator" Version="$(MicrosoftBuildLocatorVersion)" />
<PackageReference Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCoreVersion)" ExcludeAssets="Runtime" PrivateAssets="All" />
<PackageReference Include="System.Buffers" Version="$(SystemBuffersVersion)" />
<PackageReference Include="System.ComponentModel.Composition" Version="$(SystemComponentModelCompositionVersion)" />
<PackageReference Include="Microsoft.Win32.Registry" Version="$(MicrosoftWin32RegistryVersion)" />
Expand Down
34 changes: 20 additions & 14 deletions src/Workspaces/Core/MSBuild/MSBuild/Build/ProjectBuildManager.cs
View file
Open in desktop
Expand Up @@ -2,14 +2,18 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

# nullable enable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using Microsoft.Build.Framework;
using Microsoft.CodeAnalysis.MSBuild.Logging;
using Roslyn.Utilities;
using MSB = Microsoft.Build;
Expand Down Expand Up @@ -57,9 +61,9 @@ internal class ProjectBuildManager
}.ToImmutableDictionary();

private readonly ImmutableDictionary<string, string> _additionalGlobalProperties;

private MSB.Evaluation.ProjectCollection _batchBuildProjectCollection;
private MSBuildDiagnosticLogger _batchBuildLogger;
private readonly ILogger? _msbuildLogger;
private MSB.Evaluation.ProjectCollection? _batchBuildProjectCollection;
private MSBuildDiagnosticLogger? _batchBuildLogger;
private bool _batchBuildStarted;

~ProjectBuildManager()
Expand All @@ -70,22 +74,23 @@ internal class ProjectBuildManager
}
}

public ProjectBuildManager(ImmutableDictionary<string, string> additionalGlobalProperties)
public ProjectBuildManager(ImmutableDictionary<string, string> additionalGlobalProperties, ILogger? msbuildLogger = null)
{
_additionalGlobalProperties = additionalGlobalProperties ?? ImmutableDictionary<string, string>.Empty;
_msbuildLogger = msbuildLogger;
}

private ImmutableDictionary<string, string> AllGlobalProperties
=> s_defaultGlobalProperties.AddRange(_additionalGlobalProperties);

private static async Task<(MSB.Evaluation.Project project, DiagnosticLog log)> LoadProjectAsync(
string path, MSB.Evaluation.ProjectCollection projectCollection, CancellationToken cancellationToken)
private static async Task<(MSB.Evaluation.Project? project, DiagnosticLog log)> LoadProjectAsync(
string path, MSB.Evaluation.ProjectCollection? projectCollection, CancellationToken cancellationToken)
Copy link
Member

Choose a reason for hiding this comment

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

When could projectCollection ever be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its nulled out and re-allocated at the start of each Start/End build. so it can easily be null if things are ever called slightly out of order. How does roslyn typically handle complex lifetimes like this that the compiler can't track? I was inclined to say that if your object has a complex lifetime beyond some kind of Initialize method then it should be considered nullable. Happy to refactor this so the lifetime isn't so complex though.

In reply to: 401260471 [](ancestors = 401260471)

{
var log = new DiagnosticLog();

try
{
var loadedProjects = projectCollection.GetLoadedProjects(path);
var loadedProjects = projectCollection?.GetLoadedProjects(path);
if (loadedProjects != null && loadedProjects.Count > 0)
{
Debug.Assert(loadedProjects.Count == 1);
Expand Down Expand Up @@ -115,7 +120,7 @@ public ProjectBuildManager(ImmutableDictionary<string, string> additionalGlobalP
}
}

public Task<(MSB.Evaluation.Project project, DiagnosticLog log)> LoadProjectAsync(
public Task<(MSB.Evaluation.Project? project, DiagnosticLog log)> LoadProjectAsync(
string path, CancellationToken cancellationToken)
{
if (_batchBuildStarted)
Expand All @@ -137,7 +142,7 @@ public ProjectBuildManager(ImmutableDictionary<string, string> additionalGlobalP
}
}

public async Task<string> TryGetOutputFilePathAsync(
public async Task<string?> TryGetOutputFilePathAsync(
string path, CancellationToken cancellationToken)
{
Debug.Assert(_batchBuildStarted);
Expand All @@ -150,7 +155,7 @@ public ProjectBuildManager(ImmutableDictionary<string, string> additionalGlobalP

public bool BatchBuildStarted => _batchBuildStarted;

public void StartBatchBuild(IDictionary<string, string> globalProperties = null)
public void StartBatchBuild(IDictionary<string, string>? globalProperties = null)
{
if (_batchBuildStarted)
{
Expand All @@ -160,15 +165,16 @@ public void StartBatchBuild(IDictionary<string, string> globalProperties = null)
globalProperties = globalProperties ?? ImmutableDictionary<string, string>.Empty;
var allProperties = s_defaultGlobalProperties.AddRange(globalProperties);
_batchBuildProjectCollection = new MSB.Evaluation.ProjectCollection(allProperties);

_batchBuildLogger = new MSBuildDiagnosticLogger()
{
Verbosity = MSB.Framework.LoggerVerbosity.Normal
};

var buildParameters = new MSB.Execution.BuildParameters(_batchBuildProjectCollection)
{
Loggers = new MSB.Framework.ILogger[] { _batchBuildLogger }
Loggers = _msbuildLogger is null
? (new MSB.Framework.ILogger[] { _batchBuildLogger })
: (new MSB.Framework.ILogger[] { _batchBuildLogger, _msbuildLogger })
};

MSB.Execution.BuildManager.DefaultBuildManager.BeginBuild(buildParameters);
Expand All @@ -186,7 +192,7 @@ public void EndBatchBuild()
MSB.Execution.BuildManager.DefaultBuildManager.EndBuild();

// unload project so collection will release global strings
_batchBuildProjectCollection.UnloadAllProjects();
_batchBuildProjectCollection?.UnloadAllProjects();
Copy link
Member

Choose a reason for hiding this comment

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

Why a ?. here? It feels like a Contract.ThrowIfNull() is more appropriate because we shouldn't have been able to get here without a build already going, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I agree. I'll create a mop up PR

In reply to: 401259147 [](ancestors = 401259147)

_batchBuildProjectCollection = null;
_batchBuildLogger = null;
_batchBuildStarted = false;
Expand Down Expand Up @@ -219,7 +225,7 @@ public void EndBatchBuild()
}
}

_batchBuildLogger.SetProjectAndLog(projectInstance.FullPath, log);
_batchBuildLogger?.SetProjectAndLog(projectInstance.FullPath, log);

var buildRequestData = new MSB.Execution.BuildRequestData(projectInstance, targets);

Expand Down
38 changes: 28 additions & 10 deletions src/Workspaces/Core/MSBuild/MSBuild/MSBuildProjectLoader.Worker.cs
View file
Open in desktop
Expand Up @@ -2,16 +2,20 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

# nullable enable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Build.Framework;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.MSBuild.Build;
Expand Down Expand Up @@ -49,7 +53,7 @@ private partial class Worker
/// <summary>
/// Progress reporter.
/// </summary>
private readonly IProgress<ProjectLoadProgress> _progress;
private readonly IProgress<ProjectLoadProgress>? _progress;

/// <summary>
/// Provides options for how failures should be reported when loading requested project files.
Expand All @@ -66,7 +70,6 @@ private partial class Worker
/// because it was requested.
/// </summary>
private readonly bool _preferMetadataForReferencesOfDiscoveredProjects;

private readonly Dictionary<ProjectId, ProjectFileInfo> _projectIdToFileInfoMap;
private readonly Dictionary<ProjectId, List<ProjectReference>> _projectIdToProjectReferencesMap;
private readonly Dictionary<string, ImmutableArray<ProjectInfo>> _pathToDiscoveredProjectInfosMap;
Expand All @@ -80,8 +83,8 @@ private partial class Worker
ImmutableArray<string> requestedProjectPaths,
string baseDirectory,
ImmutableDictionary<string, string> globalProperties,
ProjectMap projectMap,
IProgress<ProjectLoadProgress> progress,
ProjectMap? projectMap,
IProgress<ProjectLoadProgress>? progress,
DiagnosticReportingOptions requestedProjectOptions,
DiagnosticReportingOptions discoveredProjectOptions,
bool preferMetadataForReferencesOfDiscoveredProjects)
Expand All @@ -104,7 +107,7 @@ private partial class Worker
_projectIdToProjectReferencesMap = new Dictionary<ProjectId, List<ProjectReference>>();
}

private async Task<TResult> DoOperationAndReportProgressAsync<TResult>(ProjectLoadOperation operation, string projectPath, string targetFramework, Func<Task<TResult>> doFunc)
private async Task<TResult> DoOperationAndReportProgressAsync<TResult>(ProjectLoadOperation operation, string projectPath, string? targetFramework, Func<Task<TResult>> doFunc)
{
var watch = _progress != null
? Stopwatch.StartNew()
Expand All @@ -117,7 +120,7 @@ private async Task<TResult> DoOperationAndReportProgressAsync<TResult>(ProjectLo
}
finally
{
if (_progress != null)
if (_progress != null && watch != null)
{
watch.Stop();
_progress.Report(new ProjectLoadProgress(projectPath, operation, targetFramework, watch.Elapsed));
Expand Down Expand Up @@ -297,9 +300,9 @@ private Task<ProjectInfo> CreateProjectInfoAsync(ProjectFileInfo projectFileInfo
var assemblyName = GetAssemblyNameFromProjectPath(projectPath);

var parseOptions = GetLanguageService<ISyntaxTreeFactoryService>(language)
.GetDefaultParseOptions();
?.GetDefaultParseOptions();
var compilationOptions = GetLanguageService<ICompilationFactoryService>(language)
.GetDefaultCompilationOptions();
?.GetDefaultCompilationOptions();

return Task.FromResult(
ProjectInfo.Create(
Expand Down Expand Up @@ -329,6 +332,11 @@ private Task<ProjectInfo> CreateProjectInfoAsync(ProjectFileInfo projectFileInfo
// parse command line arguments
var commandLineParser = GetLanguageService<ICommandLineParserService>(projectFileInfo.Language);

if (commandLineParser is null)
{
throw new Exception($"Unable to find a '{nameof(ICommandLineParserService)}' for '{projectFileInfo.Language}'");
}

var commandLineArgs = commandLineParser.Parse(
arguments: projectFileInfo.CommandLineArgs,
baseDirectory: projectDirectory,
Expand Down Expand Up @@ -409,11 +417,16 @@ private static string GetAssemblyNameFromProjectPath(string projectFilePath)
private IEnumerable<AnalyzerReference> ResolveAnalyzerReferences(CommandLineArguments commandLineArgs)
{
var analyzerService = GetWorkspaceService<IAnalyzerService>();
if (analyzerService is null)
{
throw new Exception($"Unable to find '{nameof(IAnalyzerService)}'");
}

var analyzerLoader = analyzerService.GetLoader();

foreach (var path in commandLineArgs.AnalyzerReferences.Select(r => r.FilePath))
{
string fullPath;
string? fullPath;

if (PathUtilities.IsAbsolute(path))
{
Expand All @@ -429,7 +442,7 @@ private IEnumerable<AnalyzerReference> ResolveAnalyzerReferences(CommandLineArgu
return commandLineArgs.ResolveAnalyzerReferences(analyzerLoader);
}

private static ImmutableArray<DocumentInfo> CreateDocumentInfos(IReadOnlyList<DocumentFileInfo> documentFileInfos, ProjectId projectId, Encoding encoding)
private static ImmutableArray<DocumentInfo> CreateDocumentInfos(IReadOnlyList<DocumentFileInfo> documentFileInfos, ProjectId projectId, Encoding? encoding)
{
var results = ImmutableArray.CreateBuilder<DocumentInfo>();

Expand Down Expand Up @@ -482,6 +495,9 @@ private void CheckForDuplicateDocuments(ImmutableArray<DocumentInfo> documents,
var paths = new HashSet<string>(PathUtilities.Comparer);
foreach (var doc in documents)
{
if (doc.FilePath is null)
continue;

if (paths.Contains(doc.FilePath))
{
var message = string.Format(WorkspacesResources.Duplicate_source_file_0_in_project_1, doc.FilePath, projectFilePath);
Expand All @@ -494,12 +510,14 @@ private void CheckForDuplicateDocuments(ImmutableArray<DocumentInfo> documents,
}
}

[return: MaybeNull]
private TLanguageService GetLanguageService<TLanguageService>(string languageName)
where TLanguageService : ILanguageService
=> _workspaceServices
.GetLanguageServices(languageName)
.GetService<TLanguageService>();

[return: MaybeNull]
private TWorkspaceService GetWorkspaceService<TWorkspaceService>()
where TWorkspaceService : IWorkspaceService
=> _workspaceServices
Expand Down