Bumping sbt to 1.4.4 breaks repl

[prolativ]

After opening REPL with sbt repl and typing some code:

• typed characters disappear while navigating with left/right arrow keys
• the entire typed line disappears after pressing the return key (even if it's not the end of a definition or a statement)
• the entire typed line disappears after pressing TAB for code completion

This seems to happen for all sbt 1.4.x versions

[smarter]

Assigning @adpi2 since he's working on sbt related stuff (#10541) which might fix the problem.

[smarter]

Also sbt/sbt#6181

[eatkins]

What is sbt repl? In https://github.com/scala/scala3-example-project, I tried the latest sbt and sbt console worked fine.

[smarter]

It's a command defined in the dotty project itself to run the repl of the compiler we're compiling instead of the one we're using to compile ourselves: https://github.com/lampepfl/dotty/blob/31751b9a12170cd972070a2f7f10447b0ae0901a/project/Build.scala#L607

[eatkins]

Yeah, I figured it was something like that. Your problem is here: https://github.com/lampepfl/dotty/blob/31751b9a12170cd972070a2f7f10447b0ae0901a/project/Build.scala#L586 I'm guessing runProcess does something like inherit io. To make this work with sbt 1.4.x, you'll probably need to wire up runProcess to use Redirect.PIPE (https://docs.oracle.com/javase/8/docs/api/java/lang/ProcessBuilder.Redirect.html). At any rate, interactive forked processes are not really well supported in sbt 1.4.x so you should feel free to open an issue about that.

Your other option is to rewrite the task to not fork and run in-process in a separate classloader.

[smarter]

I'm guessing runProcess does something like inherit io.

Indeed: https://github.com/lampepfl/dotty/blob/31751b9a12170cd972070a2f7f10447b0ae0901a/sbt-dotty/src/dotty/tools/sbtplugin/DottyIDEPlugin.scala#L169-L177

But I tried replacing every occurence of INHERIT with PIPE and it didn't make any difference.

[smarter]

... but actually, runCompilerMain doesn't call runProcess, the code you pointed to is never called from the repl task. Instead the runCompilerMain does:
https://github.com/lampepfl/dotty/blob/31751b9a12170cd972070a2f7f10447b0ae0901a/project/Build.scala#L683
Is there something wrong with runMain in sbt 1.4?

[eatkins]

Yeah, I was groggy and my manual code navigation skills failed me. AFAICT, runMain works ok for many use cases, but not for the dotty repl use case. I have a fix for the dotty repl in sbt/sbt#6185. In addition to the suggestions in the commit message, I would also suggest that you not fork by default. It is much more likely to work well since forked run with user input wasn't tested much in 1.4.x development.

[smarter]

I have a fix for the dotty repl in sbt/sbt#6185.

That was quick, thanks!

I would also suggest that you not fork by default.

I think I like forking because it gives me peace of mind: I get a blank JVM where I have some ideas of how to debug things, whereas I'm really out of my comfort zone when I have to deal with the classloader mille-feuilles I get from sbt :).
Anyway, I just gave it a try again at your suggestion and I can report that indeed it fixes the repl issues, but unfortunately the classloader business is still a show-stopper, if I run:

> scala3-compiler/run tests/pos/HelloWorld.scala -d temp
> scala3-compiler/run -classpath temp -decompile HelloWorld.tasty

I get:

java.lang.NoSuchMethodError: scala.quoted.Quotes$Reflection.PackageClauseTypeTest$(Lscala/quoted/Quotes$Reflection;)Lscala/reflect/TypeTest; while compiling HelloWorld

The problem (I'm guessing) is that scala3-compiler depends on scala3-library which are both defined in the current sbt project, but scala3-compiler has to be compiled with an older scala3-compiler and thus an older scala3-library, it's a class from that older library without some newly added method that ends up being classloaded, probably because the scala instance classloader is a parent of the classloader sbt uses to run tasks. Now, I see that sbt/sbt#6185 adds an inProcessTopClassLoader key which sounds like it's related to all of this, but I have no idea how exactly: it's really hard to understand how to use this sort of things without being deeply familiar with sbt internals.

[eatkins]

whereas I'm really out of my comfort zone when I have to deal with the classloader mille-feuilles I get from sbt :).

I get that. The reason that I'm suggesting not forking is that forking is barely tested by the sbt developers so even though it may feel better to you, it's not actually the strategy that is most likely to lead to success. Because sbt is so complex, there are only a small number of people who know it well enough to diagnose and debug certain issues. I am not going to be around forever, but there are things that I can fix in 10 minutes that could easily take days or even weeks for pretty much anyone else. But when I fix something for you, you learn something about sbt which makes you more able to help yourself and others in the future.

Anyway, I just gave it a try again at your suggestion and I can report that indeed it fixes the repl issues, but unfortunately the classloader business is still a show-stopper

So maybe for now the solution is to just not fork in the repl project. My understanding is that there is a lot of scala 3 integration work planned for sbt 1.5.0 so that they are ready to launch together early next year. The classpath stuff can be worked through as part of the integration work that @adpi2 is doing.

[smarter]

The reason that I'm suggesting not forking is that forking is barely tested by the sbt developers so even though it may feel better to you, it's not actually the strategy that is most likely to lead to success.

That is unfortunate given that forking is necessary when one needs to use javaOptions, and I may be wrong but I feel like I've seen many scala projects enable forking (as a quick check, out of the 38 projects in the dotty community build, 8 seem to use forking, that's actually less than I expected).

So maybe for now the solution is to just not fork in the repl project.

Unfortunately even if I disable forking only for the repl task, we're likely to run into the same sort of classloader issues: in the repl users can call macros, and to execute a macro the compiler classloads user-written code which can refer to library code, so if the classloader parent contains an old version of the library code things can go wrong.

[smarter]

(in general, the classloader situation for macros isn't great at all, perhaps we could do better if we could coordinate more with sbt: https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/util/ClasspathFromClassloader.scala)

[prolativ]

A possible workaround: scala3-compiler-bootstrapped/console doesn't have this problem but seems to provide the same functionality as repl