sdk
sdk copied to clipboard
dart-lang
[CP] [stable] Fix analysis server plugins not receiving setContextRoots (or having wrong roots)
Commit(s) to merge
- https://dart-review.googlesource.com/c/sdk/+/381460
- https://dart-review.googlesource.com/c/sdk/+/381481
- https://dart-review.googlesource.com/c/sdk/+/381860
Target
stable
Prepared changelist for beta/stable
https://dart-review.googlesource.com/c/sdk/+/381760
Issue Description
Recent changes to the analysis server result in fewer analysis contexts being created to improve performance. Analysis contexts were also used by analysis server plugins and when no analysis context is created at the location where a plugin should be enabled, it would instead be enabled for the parent context, resulting in the wrong folders having the plugins enabled, and potentially triggering a race condition because the same plugin may be assigned to the same root multiple times.
The result was that analysis server plugins could be nonfunctional, or given the wrong root folders to analyze.
What is the fix
This fix is to ensure that analysis contexts are created at all locations where plugins are enabled (or changed from the parent context).
Why cherry-pick
Without the cherry pick, analysis server plugins may not work at all, or may be enabled for folders that they should not (potentially producing spurious results).
Risk
Medium? I'm not sure what the scale is here. It's not the most trivial cherry-pick, however the changes in behaviour should be limited to only when plugins are used (although the extra check for plugins applies to all contexts) and the change includes several new tests covering the reported error conditions and some additional cases.
(cc @bwilkerson @srawlins @pq in case they have opinions on this)
Issue link(s)
https://github.com/dart-lang/sdk/issues/56475
Extra Info
No response
Summary: Analysis server plugins were not receiving the correct context roots, leading to incorrect plugin behavior and potential race conditions. This fix ensures that analysis contexts are created at all locations where plugins are enabled, resolving the issue.
~~I spotted an issue with the fix here. The fix is simple, but we need some additional tests for those cases. I'll work on a CL for main and then update this cherry-pick with the fix after.~~
The issue noted above has now been fixed in main and included in the cherry-pick CL.
The fix has a very low risk of causing problems (it's safe to land), and a failure to cherry pick the fix will leave some unknown percentage of our users unable to use the plugins they've come to depend on. I think our users will be much better off if we can approve this.
