Refactored root modules finder #2648
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Changed `PrefixLogger` to accept a `Logger` instead of a `ColorLogger`. Also added a `quiet` flat to `MainModule.resolve`.
lefou
force-pushed
the
refactor-find-root-modules
branch
from
ce3f846
to
552d312
Compare
lihaoyi
added a commit
that referenced
this pull request
Aug 15, 2023
…in BSP to avoid using the wrong evaluator (#2692) Should fix #2643. The basic problem is previously BSP always used the `frame(0)` evaluator passed in to `updateEvaluator`, which is meant specifically for evaluating user tasks in the root `./build.sc`. However, BSP also querying of modules and tasks both in `build.sc` as well as meta-builds in `mill-build/build.sc` and nested meta-builds. This ends up evaluating the meta-builds in the same `out/` folder as the main build, rather than in the `out/mill-build/` folders they should be evaluated in. This PR avoids using the `Evaluator` given in `updateEvaluator`, and instead preserves the `Evaluator`s we already construct as part of `MillBuildBootstrap` in `mill.bsp.worker.State`, and passes them as a `Seq[Evaluator]` from the enclosing `MillBuildBootstrap` code all the way to the Mill-BSP code so we can associate each `Module` with its respective `Ealuator`. We then use those associated `Evaluator`s to perform the actual evaluation throughout the `Mill{Scala,Java,Jvm,}BuildServer` implementations. There's some additional re-shaping, so that in case a single BSP API call wants to query modules/tasks from multiple evaluators at once we properly group the tasks/modules together according to their respective evaluators for evaluation. By passing the `Seq[Evaluator]` from the enclosing `MillBuildBootstrap`, this also lets us remove the now-redundant call to `MillBuildBootstrap` buried within `State.scala`. We already have all the necessary data structures from the enclosing `MillBuildBootstrap` and its evaluators, so we don't need to re-run the bootstrap process and re-construct all of it from scratch just for BSP I haven't managed to reproduce the original error, so confirming the fix will have to wait until this lands in master and people can try it out. But I have manually tested using VSCode to load and jump around `example/basic/1-simple-scala/` and `example/scalabuilds/10-scala-realistic/`, and it seems all the references and modules work and are set up correctly. So at least we have some confidence that this doesn't cause any regressions. This should also supersede #2648, since we now no longer need to re-run the `MillBuildBootstrap`/`RootModuleFinder` logic at all since we just take the existing list of `Evaluator`s from the enclosing bootstrap process
|
This PR is obsolete. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moved root module finder logic into dedicated
RootModuleFinderobject.Changed
PrefixLoggerto accept aLoggerinstead of aColorLogger.Also added a
quietflag toMainModule.resolve. This is useful, if we need the return value, but do not want the command to print the resolved outcome.