-
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
Pass runningProject
to GetUpdateAsync
and get projectToRebuild and projectToRestart back
#75503
base: main
Are you sure you want to change the base?
Changes from all commits
f274dd3
9363f55
81466b7
5374001
2d4236b
2deda3d
16fa2fd
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 |
---|---|---|
|
@@ -74,6 +74,127 @@ | |
|
||
return builder.ToImmutableAndClear(); | ||
} | ||
|
||
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. Not sure why this is shown as added. Did you rebase against the latest main? 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. Not quite follow, what you mean here 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. Looks like the code was copied from https://github.com/dotnet/roslyn/pull/75503/files#diff-95af090e3bc672c6352c9245c5b43e6d1561613fc81afee422e462dc9f17a49fR266. 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. Yeah that's true, would be better to find out a way to clean it up 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. Let me look into that |
||
private IEnumerable<Project> GetProjectsContainingBlockingRudeEdits(Solution solution) | ||
=> RudeEdits | ||
.Where(static e => e.Severity == DiagnosticSeverity.Error && e.ProjectId is not null) | ||
.Select(static e => e.ProjectId) | ||
.Distinct() | ||
.OrderBy(static id => id) | ||
.Select(solution.GetRequiredProject); | ||
Check failure on line 84 in src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs#L84
Check failure on line 84 in src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs#L84
|
||
|
||
/// <summary> | ||
/// Returns projects that need to be rebuilt and/or restarted due to blocking rude edits in order to apply changes. | ||
/// </summary> | ||
/// <param name="isRunningProject">Identifies projects that have been launched.</param> | ||
/// <param name="projectsToRestart">Running projects that have to be restarted.</param> | ||
/// <param name="projectsToRebuild">Projects whose source have been updated and need to be rebuilt.</param> | ||
public void GetProjectsToRebuildAndRestart( | ||
Solution solution, | ||
Func<Project, bool> isRunningProject, | ||
ISet<Project> projectsToRestart, | ||
ISet<Project> projectsToRebuild) | ||
{ | ||
var graph = solution.GetProjectDependencyGraph(); | ||
|
||
// First, find all running projects that transitively depend on projects with rude edits. | ||
// These will need to be rebuilt and restarted. In order to rebuilt these projects | ||
// all their transitive references must either be free of source changes or be rebuilt as well. | ||
// This may add more running projects to the set of projects we need to restart. | ||
// We need to repeat this process until we find a fixed point. | ||
|
||
using var _1 = ArrayBuilder<Project>.GetInstance(out var traversalStack); | ||
|
||
projectsToRestart.Clear(); | ||
projectsToRebuild.Clear(); | ||
|
||
foreach (var projectWithRudeEdit in GetProjectsContainingBlockingRudeEdits(solution)) | ||
{ | ||
if (AddImpactedRunningProjects(projectsToRestart, projectWithRudeEdit)) | ||
{ | ||
projectsToRebuild.Add(projectWithRudeEdit); | ||
} | ||
} | ||
|
||
// At this point the restart set contains all running projects directly affected by rude edits. | ||
// Next, find projects that were successfully updated and affect running projects. | ||
|
||
if (ModuleUpdates.Updates.IsEmpty || projectsToRestart.IsEmpty()) | ||
{ | ||
return; | ||
} | ||
|
||
// The set of updated projects is usually much smaller then the number of all projects in the solution. | ||
// We iterate over this set updating the reset set until no new project is added to the reset set. | ||
// Once a project is determined to affect a running process, all running processes that | ||
// reference this project are added to the reset set. The project is then removed from updated | ||
// project set as it can't contribute any more running projects to the reset set. | ||
// If an updated project does not affect reset set in a given iteration, it stays in the set | ||
// because it may affect reset set later on, after another running project is added to it. | ||
|
||
using var _2 = PooledHashSet<Project>.GetInstance(out var updatedProjects); | ||
using var _3 = ArrayBuilder<Project>.GetInstance(out var updatedProjectsToRemove); | ||
foreach (var update in ModuleUpdates.Updates) | ||
{ | ||
updatedProjects.Add(solution.GetRequiredProject(update.ProjectId)); | ||
} | ||
|
||
using var _4 = ArrayBuilder<Project>.GetInstance(out var impactedProjects); | ||
|
||
while (true) | ||
{ | ||
Debug.Assert(updatedProjectsToRemove.Count == 0); | ||
|
||
foreach (var updatedProject in updatedProjects) | ||
{ | ||
if (AddImpactedRunningProjects(impactedProjects, updatedProject) && | ||
impactedProjects.Any(projectsToRestart.Contains)) | ||
{ | ||
projectsToRestart.AddRange(impactedProjects); | ||
updatedProjectsToRemove.Add(updatedProject); | ||
projectsToRebuild.Add(updatedProject); | ||
} | ||
|
||
impactedProjects.Clear(); | ||
} | ||
|
||
if (updatedProjectsToRemove is []) | ||
{ | ||
// none of the remaining updated projects affect restart set: | ||
break; | ||
} | ||
|
||
updatedProjects.RemoveAll(updatedProjectsToRemove); | ||
updatedProjectsToRemove.Clear(); | ||
} | ||
|
||
return; | ||
|
||
bool AddImpactedRunningProjects(ICollection<Project> impactedProjects, Project initialProject) | ||
{ | ||
Debug.Assert(traversalStack.Count == 0); | ||
traversalStack.Push(initialProject); | ||
|
||
var added = false; | ||
|
||
while (traversalStack.Count > 0) | ||
{ | ||
var project = traversalStack.Pop(); | ||
if (isRunningProject(project)) | ||
{ | ||
impactedProjects.Add(project); | ||
added = true; | ||
} | ||
|
||
foreach (var referencingProjectId in graph.GetProjectsThatDirectlyDependOnThisProject(project.Id)) | ||
{ | ||
traversalStack.Push(solution.GetRequiredProject(referencingProjectId)); | ||
} | ||
} | ||
|
||
return added; | ||
} | ||
} | ||
} | ||
|
||
public static readonly EmitSolutionUpdateResults Empty = new() | ||
|
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.
Shouldn't be updated without updating Microsoft.VisualStudio.SDK. Probably better to bump Microsoft.VisualStudio.SDK in a separate PR.
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.
How to determine the right VS SDK version to use. I saw the current version is
17.10.234-preview.1
but that one is not available on nuget.org?