Remove betasty directory on successful compilation backgroundTasks

[jchyb]

Fix to scalameta/metals#6628
Usually it was fine to always put betasty files at the end of the classpath, but in the reproduction for some reason despite the code being correct, those files were still being used, which resulted in an incoherent state in bloop (we obtained only betasty files on what was being considered non-best effort compilation). This lead to trying to apply incremental compilation there, producing singular betasty files, and losing information about symbols from other files, producing errors.

The fix is very simple, but so far I have been unable to properly add any tests for it, as the backgroundTasks do not seem to be run in the tests suites.

[tgodzik]

Looks like this indeed does help in case of the repository I sent your way, but the Scala compiler is failing with cryptic stuff like:

Could not verify that the method argument is transitively initialized (Hot). It was found to be the original object of type (class SymDenotation) where initialization checking started. Only transitively initialized arguments may be passed to methods (except constructors).
Non initialized field(s): variable myFlags, variable myPrivateWithin, variable myAnnotations, variable myParamss, variable myTargetName. Calling trace:
├── class SymDenotation private[SymDenotations] (	[ SymDenotations.scala:36 ]
│   ^
├── if (Config.checkNoSkolemsInInfo) assertNoSkolems(initInfo)	[ SymDenotations.scala:53 ]
│                                    ^^^^^^^^^^^^^^^^^^^^^^^^^
├── def assertNoSkolems(tp: Type): Unit =	[ SymDenotations.scala:1622 ]
│   ^
├── assert(!hasSkolems(tp), s"assigning type $tp containing skolems to $this")	[ SymDenotations.scala:1624 ]
│   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
├── 	[ Predef.scala:10 ]
│                                                                   ^^^^^^^
└── assert(!hasSkolems(tp), s"assigning type $tp containing skolems to $this")	[ SymDenotations.scala:1624 ]
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

any idea why?

[jchyb]

This is still pretty unfinished, however I am almost done with the working version. After just this change the incremental compiler interactions can be incorrect, so I also implemented a thing where we properly restart the compilation after the first unsuccessful one (this was also incorrect before, but it was not really noticable, due to us still having betasty we could use from the previous compilation - I'll try to explain this specific issue and the fix later, after I push the code here). Also one race condition I noticed caused a crash from one of the reports we got this week, so that also should be fixed.

With that said none of these fixes fix the behavior in the compiler unfortunately. The main issue there seems to be with the java source files, which I am pretty sure we do not handle yet in best effort compilation :(. When I tested the best effort compilation in the compiler I ended up running into a compiler crash and errors related to missing java symbols - now I can't be 100% sure whether the crash was also caused by the java files, but the compiler is not always able to recover gracefully from the missing symbols, so that might be it... I am sure there is some roundabout way to work with those even without updating the compiler, however it might take more time (I'll work on it next week).

[tgodzik]

When I tested the best effort compilation in the compiler I ended up running into a compiler crash and errors related to missing java symbols - now I can't be 100% sure

Wouldn't that be caused by the problems with compilation? So if the Scala source do not compile and produce classfiles, the Java sources which use them will not be able to compile either. Especially that the compilation mode in the compiler is "mixed". I would say the Java compilation issues are not relevant to the actual problem.

[jchyb]

Also, obviously it should always recover after fixing the issues, no idea why it does not do that in the compiler codebase, so I'll probably start investigating with this.

EDIT: I wrote this before I saw the comment above, but maybe it answers the doubt there? The java stuff should not matter for successful compilations, it only produces additional fake errors/other issues when the compilation is failing.

[jchyb]

(I pushed it but I still want to recheck it/ clean it up, so I am leaving this as a draft for now)

[jchyb]

In the meantime let me quickly explain why we do those recompilations here now:
Let's say we have a Utils module, consisting of:

• UtilsA.scala
• UtilsB.scala

Lets say the first Utils compilation would give us:
UtilsA.class/tasty... and UtilsB.class/tasty... <- we copy everything to client dir and to readonlyDir
We add an error to UtilsB.scala. Now we have from incremental compilation (without any data on what else to recompile) UtilsB.betasty.
Previously we would first copy everything (including betasty!) from readonlyDir, and copy UtilsB.betasty, giving an effect of complete best effort recompilation, but actually compiling only UtilsB.betasty. This was an issue on my part, that I did not notice. Now suddenly, after these fixes, we no longer have those outdated betasty in readonly, so we need to recompile the whole Utils, which we do in this PR. It's worth noting again that the previous behavior was also incorrect, but it was harder to notice.

[jchyb]

Changes here are not strictly related to best effort, but not returning failedProblems in the case of compiler crash caused those annoying leftover errors in the problem tab in the compiler codebase (I can't see why it wouldn't behave the same way even for crashes unrelated to best effort). I also figured that it might be useful to also log the stacktraces of those crashes, although maybe a different log level would be better (debug?).

[jchyb]

About the compiler crash itself (as seen while using best effort in the compiler codebase), the fix for that will have to be in the compiler (I already have a preliminary fix working there)