-
-
Notifications
You must be signed in to change notification settings - Fork 420
Move the semanticdb generation logic to compile and make sure it's shared where possible
#5841
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
base: main
Are you sure you want to change the base?
Move the semanticdb generation logic to compile and make sure it's shared where possible
#5841
Conversation
…ration-on-compile # Conflicts: # libs/javalib/src/mill/javalib/SemanticDbJavaModule.scala # libs/javalib/src/mill/javalib/UnresolvedPath.scala
….com/arturaz/mill into fix/semanticdb-generation-on-compile
…ration-on-compile # Conflicts: # libs/daemon/server/src/mill/server/Server.scala # libs/javalib/src/mill/javalib/SemanticDbJavaModule.scala
…spClientsNeedSemanticDb runtime value
…ration-on-compile' into fix/semanticdb-generation-on-compile
…ration-on-compile # Conflicts: # example/scalalib/linting/3-acyclic/build.mill # example/scalalib/spark/1-hello-spark/build.mill # example/scalalib/spark/3-semi-realistic/build.mill # integration/ide/bsp-server/resources/snapshots/logging
|
TBH, the current state (of this PR) looks way to complicated and makes too much assumptions for my taste. But I admit, the current codebase (in this context) is already far from being simple. I'd like to split the problem and provide clear solutions separated from each other. Issue 1: Bad compilation performance
Issue 2: Decide when we need semanticDB data
Proposal for Issue 1:Disclaimer: proposed task names are not final but choosen to make the concept clear
Ideas for Issue 2:
|
This PR optimizes and reworks the compilation pipeline, with regards to SemanticDB generation.
Pre-PR situation
compileandsemanticDbDetailedtasks.compileperformed the compilation without semantic DB plugin.semanticDbDetailedperformed the compilation with the semantic DB plugin, but did not reuse thecompile's output.Which meant that if any of these happened:
compileand thensemanticDbData.compileand then any other task that required the semantic db, or vice-versa.The compilation would have been performed twice, wasting CPU cycles and worsening the developer experience.
Post PR situation
Mill now smartly chooses whether
compileproduces semanticdb data or not. semanticdb is produced if:compilewas directly invoked by a task that needs semanticdb.Implementation details
Introduction of
MILL_BSP_OUTPUT_DIRPreviously you could use
MILL_OUTPUT_DIRenvironment variable to set both regular and bsp mill's output directory to a certain folder.Because regular mill now needs to know the location of the BSP folder, having one variable is problematic:
MILL_OUTPUT_DIR=out_bsp ./mill --bsp ...MILL_OUTPUT_DIR=out_bsp ./mill ...changes the regular mill out folder.Thus
MILL_BSP_OUTPUT_DIRis introduced, which allows you to:MILL_BSP_OUTPUT_DIR=out_bsp ./mill --bsp ...MILL_BSP_OUTPUT_DIR=out_bsp ./mill ... # this still uses the regular out/ folder, but knows where bsp mill out/ folder is locatedBuildCtx.bspSemanticDbSessionsFolderFolder in the filesystem where Mill's BSP sessions that require semanticdb store an indicator file (name = process PID, contents are irrelevant) to communicate to main Mill daemon and other BSP sessions that there is at least one Mill session that will need the semanticdb.
The reasoning is that if at least one of Mill's clients requests semanticdb, then there is no point in running regular
compilewithout semanticdb, as eventually we will have to rerun it with semanticdb, and thus we should compile with semanticdb upfront to avoid paying the price of compling twice (without semanticdb and then with it).CompilationResult.semanticDbFilesBecause we can't change the return type of
compiledue to binary compatibility,semanticDbFilesfield was added toCompilationResultandcompilefills it in if the compilation happened with semanticdb enabled.Removal of
CompileForand related tasksThese are not needed anymore with the single
compiletask.Removal of separate
SemanticDbJavaModule.semanticDbDataDetailedtaskIt's functionality was merged to
compileInternal, which takes acompileSemanticDbparameter.There also was a lot of code duplication between
compileandsemanticDbDataDetailedtasks, for both java and scala modules.Replace the implicit
Task.destusage inZincWorkerwith explicitcompileToargumentThis makes it clearer what the parameter is used for and allows to reuse the same value in
SemanticDbJavaModule.enhanceCompilationResultWithSemanticDbinvocation.Misc changes
JavaModule#resolveRelativeToOutinstance method toUnresolvedPath.resolveRelativeToOut.Serverto provide better debugging output if the server cannot be launched.testScala212Versionupdated to2.12.20because new semanticdb plugin is not provided for the ancient2.12.6version that was used.Fixes: #5744