-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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.
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
Work to unify threading concepts between Workspace and ProjectSystemProjectFactory (part2) #67524
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,6 @@ | |
using System.Collections.Immutable; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Reflection.Metadata.Ecma335; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Host; | ||
|
@@ -78,57 +77,76 @@ public ProjectSystemProjectFactory(Workspace workspace, IFileChangeWatcher fileC | |
|
||
public async Task<ProjectSystemProject> CreateAndAddToWorkspaceAsync(string projectSystemName, string language, ProjectSystemProjectCreationInfo creationInfo, ProjectSystemHostInfo hostInfo) | ||
{ | ||
var id = ProjectId.CreateNewId(projectSystemName); | ||
var projectId = ProjectId.CreateNewId(projectSystemName); | ||
var assemblyName = creationInfo.AssemblyName ?? projectSystemName; | ||
|
||
// We will use the project system name as the default display name of the project | ||
var project = new ProjectSystemProject( | ||
this, | ||
hostInfo, | ||
id, | ||
projectId, | ||
displayName: projectSystemName, | ||
language, | ||
assemblyName: assemblyName, | ||
assemblyName, | ||
creationInfo.CompilationOptions, | ||
creationInfo.FilePath, | ||
creationInfo.ParseOptions); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just removed named options i thought were redundant. |
||
|
||
var versionStamp = creationInfo.FilePath != null | ||
? VersionStamp.Create(File.GetLastWriteTimeUtc(creationInfo.FilePath)) | ||
: VersionStamp.Create(); | ||
|
||
var projectInfo = ProjectInfo.Create( | ||
new ProjectInfo.ProjectAttributes( | ||
projectId, | ||
versionStamp, | ||
name: projectSystemName, | ||
assemblyName, | ||
language, | ||
compilationOutputFilePaths: default, // will be updated when command line is set | ||
SourceHashAlgorithms.Default, // will be updated when command line is set | ||
filePath: creationInfo.FilePath, | ||
telemetryId: creationInfo.TelemetryId), | ||
compilationOptions: creationInfo.CompilationOptions, | ||
filePath: creationInfo.FilePath, | ||
parseOptions: creationInfo.ParseOptions); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pulled creation of this info singleton up out of hte lambda. |
||
|
||
var versionStamp = creationInfo.FilePath != null ? VersionStamp.Create(File.GetLastWriteTimeUtc(creationInfo.FilePath)) | ||
: VersionStamp.Create(); | ||
|
||
await ApplyChangeToWorkspaceAsync(w => | ||
await ApplyChangeToWorkspaceAsync(async w => | ||
{ | ||
var projectInfo = ProjectInfo.Create( | ||
new ProjectInfo.ProjectAttributes( | ||
id, | ||
versionStamp, | ||
name: projectSystemName, | ||
assemblyName: assemblyName, | ||
language: language, | ||
checksumAlgorithm: SourceHashAlgorithms.Default, // will be updated when command line is set | ||
compilationOutputFilePaths: default, // will be updated when command line is set | ||
filePath: creationInfo.FilePath, | ||
telemetryId: creationInfo.TelemetryId), | ||
compilationOptions: creationInfo.CompilationOptions, | ||
parseOptions: creationInfo.ParseOptions); | ||
|
||
// If we don't have any projects and this is our first project being added, then we'll create a new SolutionId | ||
// and count this as the solution being added so that event is raised. | ||
if (w.CurrentSolution.ProjectIds.Count == 0) | ||
{ | ||
w.OnSolutionAdded( | ||
SolutionInfo.Create( | ||
SolutionId.CreateNewId(SolutionPath), | ||
VersionStamp.Create(), | ||
SolutionPath, | ||
projects: new[] { projectInfo }, | ||
analyzerReferences: w.CurrentSolution.AnalyzerReferences) | ||
.WithTelemetryId(SolutionTelemetryId)); | ||
} | ||
else | ||
{ | ||
w.OnProjectAdded(projectInfo); | ||
} | ||
await w.SetCurrentSolutionAsync( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this now uses the new helper, spinning until it 'wins' and actually makes the change to the workspace. this is the primary functional change intended by thsi PR, moving away from manually splatting the OnSolutionAdded call. |
||
useAsync: true, | ||
oldSolution => | ||
{ | ||
// If we don't have any projects and this is our first project being added, then we'll create a | ||
// new SolutionId and count this as the solution being added so that event is raised. | ||
if (oldSolution.ProjectIds.Count == 0) | ||
{ | ||
var solutionInfo = SolutionInfo.Create( | ||
SolutionId.CreateNewId(SolutionPath), | ||
VersionStamp.Create(), | ||
SolutionPath, | ||
projects: new[] { projectInfo }, | ||
analyzerReferences: w.CurrentSolution.AnalyzerReferences).WithTelemetryId(SolutionTelemetryId); | ||
var newSolution = w.CreateSolution(solutionInfo); | ||
|
||
foreach (var project in solutionInfo.Projects) | ||
newSolution = newSolution.AddProject(project); | ||
|
||
return newSolution; | ||
} | ||
else | ||
{ | ||
return oldSolution.AddProject(projectInfo); | ||
} | ||
}, | ||
(oldSolution, newSolution) => | ||
{ | ||
return oldSolution.ProjectIds.Count == 0 | ||
? (WorkspaceChangeKind.SolutionAdded, projectId: null, documentId: null) | ||
: (WorkspaceChangeKind.ProjectAdded, projectId, documentId: null); | ||
}, | ||
onBeforeUpdate: null, | ||
onAfterUpdate: null, | ||
CancellationToken.None).ConfigureAwait(false); | ||
}).ConfigureAwait(false); | ||
|
||
return project; | ||
|
@@ -174,11 +192,11 @@ public void ApplyChangeToWorkspace(Action<Workspace> action) | |
/// <summary> | ||
/// Applies a single operation to the workspace. <paramref name="action"/> should be a call to one of the protected Workspace.On* methods. | ||
/// </summary> | ||
public async ValueTask ApplyChangeToWorkspaceAsync(Action<Workspace> action) | ||
public async ValueTask ApplyChangeToWorkspaceAsync(Func<Workspace, ValueTask> action) | ||
{ | ||
using (await _gate.DisposableWaitAsync().ConfigureAwait(false)) | ||
{ | ||
action(Workspace); | ||
await action(Workspace).ConfigureAwait(false); | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a rename as 'id' was very confusing to me.