Default to no fsc #6747

som-snytt · 2018-06-06T21:39:23Z

Let the script runner default to not firing up a compilation daemon.

Accept -howtorun:fsc to mean use the server.

Take -nc to mean shut it down. This is much more convenient than
using fsc -shutdown.

Edit: Use -Yscriptrunner flavor, where flavor is default, resident, shutdown, or a class name.

For shutdown to run, use scala -Yscriptrunner shutdown -e "". Without -e or a file, it goes to REPL. An empty filename "" is not sufficient.

SethTisue · 2018-06-06T22:36:14Z

are you spying on Scala team meetings? 👀 we just discussed/suggested this this morning.

in the longer run it might be nice to just rip up all the compilation-daemon stuff and throw it away, but we're running out of time for would-be-nices for 2.13.

SethTisue · 2018-06-06T22:37:48Z

👍 to defaulting to -nc, but I'm less certain about the change to be more aggressive with the shutting-down. not sure it makes sense to rock the boat there, maybe better to leave well enough alone? not sure.

lrytz · 2018-06-07T09:04:25Z

src/compiler/scala/tools/nsc/GenericRunnerSettings.scala

      "how",
      "how to run the specified code",
-      List("object", "script", "jar", "repl", "guess"),
+      List("object", "script", "jar", "repl", "fsc", "guess"),


I think -howtorun:fsc is not easy to understand. Maybe script-compdaemon is more clear?

I considered -howtorun:resident because fsc is a brand name. When fsc is a module, it will need the brand exposure, but resident is generic; ideally, there will be multiple REPL frontends and backends, and some of the backends will support resident flag. Or am I getting ahead of myself?

I like script in the name because that's "how it runs", as a script, just in a special way. alternatively add a -compdaemon setting, but i think we want to get rid of it entirely, so i would not add something.

I also couldn't find out how to negate the negation, i.e. -nc:false or so, is that possible?

I think -nc:false just works.

Ah... Here's what tripped me up:

➜ sandbox git:(2.13.x) scala -nc Test.scala hi ➜ sandbox git:(2.13.x) fsc -shutdown [No compilation server running.] ➜ sandbox git:(2.13.x) scala Test.scala -nc hi ➜ sandbox git:(2.13.x) fsc -shutdown [Compile server exited] ➜ sandbox git:(2.13.x) fsc -shutdown [No compilation server running.] ➜ sandbox git:(2.13.x)

I'm doing a quick refactor of ScriptRunner to break out the functionality, and then -Yscriptrunner:com.github.somsnytt.BestScriptRunnerEver.

I remember there's a comment in settings.process about handling trailing options; I'll look if it's broken for Generic...Settings.

lrytz · 2018-06-07T09:09:25Z

src/compiler/scala/tools/nsc/GenericRunnerSettings.scala

      "-nc",
-      "do not use the fsc compilation daemon") withAbbreviation "-nocompdaemon" withPostSetHook((x: BooleanSetting) => {_useCompDaemon = !x.value })
+      "do not use the fsc compilation daemon and shut it down if it is running").withAbbreviation("-nocompdaemon")
+      //.withDeprecationMessage("scripts use cold compilation by default; use -howtorun:fsc to start the compilation daemon")


I'm in favor of adding the deprecation warning, so we can remove -nc eventually

I'll need a better story for replacing fsc -shutdown. Besides -howtorun:shutdown. Not really sure anyone ever, anywhere, has ever used -howtorun; I think I tweaked it once. Maybe it should be renamed to -whattorun.

lrytz

LGTM, thanks!

lrytz · 2018-06-08T07:29:03Z

src/repl-frontend/scala/tools/nsc/MainGenericRunner.scala

          ObjectRunner.runAndCatch(settings.classpathURLs, thingToRun, command.arguments)
        case AsScript if isE =>
-          Right(ScriptRunner.runCommand(settings, combinedCode, thingToRun +: command.arguments))
+          Right(!ScriptRunner(settings).runScriptText(combinedCode, thingToRun +: command.arguments).isDefined)


why do we discard the exception here?

You never let me get away with anything. I was worn down by the refactor at that point. And by figuring out what to call it besides runCommand. I'll take another look at how the values bubble up; there were other spots like CommonRunner IIRC.

Perhaps along with the rebase?

Let the script runner default to not firing up a compilation daemon. Accept `-howtorun:fsc` to mean use the server. Take `-nc` to mean shut it down. This is much more convenient than using `fsc -shutdown`.

ScriptRunner is an interface, with standard implementations for immediate compilation and resident compile server. A daemon killer knows how to shutdown the compile server.

Accept default, resident, shutdown, or custom class with a constructor that takes settings. `-nc` is a deprecated alias.

We don't seem to care if something ran unsuccessfully without throwing. Also, leverage NonFatal when throwing yourself on a hand grenade.

som-snytt · 2018-06-12T17:48:24Z

It could use another round. In ScriptRunner, withCompiledScript and runCompiled return boolean instead of passing back the exception; the handler body is also boolean-valued. Those could be flipped to Option[Bomb], but rebasing incurs a merge from my previous helpful efforts.

som-snytt · 2018-06-12T17:53:43Z

src/compiler/scala/tools/nsc/util/Exceptional.scala

-  // partest still uses the old name.  once we re-in-source partest we
-  // can get rid of this, but in the meantime, we'd rather not branch
-  // partest just because one method got renamed
-  def unwrap(x: Throwable): Throwable = rootCause(x)


Merit badge territory! 🎖️

som-snytt · 2018-10-24T18:39:20Z

src/repl-frontend/scala/tools/nsc/MainGenericRunner.scala

          JarRunner.runJar(settings, thingToRun, command.arguments)
        case Error =>
-          Right(false)
+          None


This changes the exit value of scala Junk from 1 to 0. howToRun sees no such class.

Are there conditions where the other runners did not throw but would return Some(false) for failure? Check failed compilation, for example.

scala-jenkins added this to the 2.13.0-M5 milestone Jun 6, 2018

lrytz reviewed Jun 7, 2018

View reviewed changes

som-snytt force-pushed the issue/nc branch from 253f8ac to 8432bb2 Compare June 7, 2018 22:20

lrytz approved these changes Jun 8, 2018

View reviewed changes

som-snytt added 4 commits June 12, 2018 10:28

Default to no fsc

c749292

Let the script runner default to not firing up a compilation daemon. Accept `-howtorun:fsc` to mean use the server. Take `-nc` to mean shut it down. This is much more convenient than using `fsc -shutdown`.

Refactor ScriptRunner

58b5e32

ScriptRunner is an interface, with standard implementations for immediate compilation and resident compile server. A daemon killer knows how to shutdown the compile server.

Yscriptrunner specifies how to run script

d6a918a

Accept default, resident, shutdown, or custom class with a constructor that takes settings. `-nc` is a deprecated alias.

Use Option[Throwable] for Either[Thr,Boolean]

bd6490b

We don't seem to care if something ran unsuccessfully without throwing. Also, leverage NonFatal when throwing yourself on a hand grenade.

som-snytt force-pushed the issue/nc branch from 8432bb2 to bd6490b Compare June 12, 2018 17:42

som-snytt commented Jun 12, 2018

View reviewed changes

adriaanm merged commit 211bc44 into scala:2.13.x Jun 14, 2018

som-snytt deleted the issue/nc branch June 14, 2018 16:17

SethTisue added the release-notes worth highlighting in next release notes label Jun 15, 2018

martijnhoekstra mentioned this pull request Jun 19, 2018

2.13: scala-dist-smoketest started failing recently scala/scala-dev#524

Closed

som-snytt commented Oct 24, 2018

View reviewed changes

som-snytt restored the issue/nc branch June 26, 2020 20:48

SethTisue mentioned this pull request Jul 30, 2020

2.12.12 regression in Bash: fscbad option: '-Ytrack-dependencies' scala/bug#12102

Closed

som-snytt deleted the issue/nc branch December 20, 2020 00:56

som-snytt mentioned this pull request Dec 22, 2022

Runner error status on bad class or jar name #10249

Merged

Default to no fsc #6747

Default to no fsc #6747

Uh oh!

Conversation

som-snytt commented Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SethTisue commented Jun 6, 2018

Uh oh!

SethTisue commented Jun 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrytz Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrytz Jun 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrytz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

som-snytt commented Jun 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

som-snytt commented Jun 6, 2018 •

edited

Loading

SethTisue commented Jun 6, 2018 •

edited

Loading

lrytz Jun 7, 2018 •

edited

Loading

lrytz Jun 7, 2018 •

edited

Loading