Fix plus Workaround for 13760

[philwalk]

superceded by #14252

This fixes the problem that script exceptions are discarded by MainGenericRunner, when running from a compiled jar.

This PR displays all exceptions on the Console when running from a jar file.

It also provides a workaround for avoiding the exception (the problem described by #13760), which is relevant to all java versions later than 8.

`java.lang.ClassNotFoundException: java.sql.Date`

Currently, when running a script for which a previously compiled jar is present, the jar file is executed, even if the -save option has not been specified (this behavior differs from scala2 behavior).

This PR executes from the jar only if the -save option is specified at runtime. Problem scripts (i.e., with references to java.sql.Date) can now selectively disable the -save option.

Adding -save to SCALA_OPTS will typically reduce script startup time by a couple of seconds. This PR provides per-script granularity for overriding the '-save' option. Scripts that would otherwise throw an exception when running from a compiled jar may specify -nosave in the hashbang line. This avoids having to remove the -save option from SCALA_OPTS.

The -nosave option can be added to the hashbang line, as in these two example hashbang lines:

The first version requires /usr/bin/env version 8.30 or later:

#!/usr/bin/env -S scala -nosave

This version uses the path to the scala wrapper:

#!/opt/scala3/bin/scala -nosave

[philwalk]

The failed test seems to be the result of a time zone difference: the test expected stdout to contain 1969-12-31 whereas the script actually printed 1970-01-01. I have an update, but will wait for additional feedback before pushing changes.

[BarkingBad]

I just took a quick glance at the changes, will focus more on the issue in the evening. My question is why there are 16 commits in this PR, could you rebase it to keep only relevant commits to be merged? Nonetheless, thanks for your fixes :)

[BarkingBad]

Why do we need this? There is save = false by default afaik

[philwalk]

Why do we need this? There is save = false by default afaik

The purpose is to permit a script to opt-out of the -save option if necessary, without having to disable the -save option for all scripts (by removing -save from SCALA_OPTS). There is a new command line option -nosave that will reset save = false after SCALA_OPTS have been processed. The new option can be added to the hashbang line. Our new BashScriptTests test case verifies that it can override the -save option.

[BarkingBad]

Ok, sounds good

[BarkingBad]

Is this needed? Nio is newer and usually more respected that old io API, so I am curious

[philwalk]

Is this needed? Nio is newer and usually more respected that old io API, so I am curious

This is not needed, but it now matches how File instanced str created everywhere else in MainGenericRunner, and it's also more efficient (by directly initializing a File rather than creating a Path and then converting it to a File). However, perhaps we should leave it like it was.

[BarkingBad]

If you feel like File is OK, let it be then, I won't argue

[BarkingBad]

Isn't it returning Unit instead of Option[Throwable]?

[BarkingBad]

I guess it was my fault that I don't assign the throwable and then print it if something went wrong. I think it should be done that way, then we will reuse Exception from line 228

[philwalk]

So something like this?

          val res = if mainClass.nonEmpty then
            ObjectRunner.runAndCatch(newClasspath :+ File(targetJar).toURI.toURL, mainClass, settings.scriptArgs)
          else
            Some(IllegalArgumentException(s"No main class defined in manifest in jar: $precompiledJar"))
          errorFn("", res)

[BarkingBad]

Yes, something like that seems reasonable

[philwalk]

philwalk wants to merge 16 commits

I'm not sure why this happened. My sandbox got messed up, so I directly edited BashScriptsTests in the browser. All of the commits seem to be superfluous, having no effect on the code changes, AFAICT.

Ah, I think I know what my mistake was, but I'm not sure what the best way is to correct it, other than to close this PR and start over. Suggestions are welcome. I have a new branch that has only the bare minimum of changes, is there a way to point this PR to the new branch?

I will plan to close this PR and create a new one, when the test resources are less busy this evening.

BTW, it seems that a proper fix to the underlying problem might be to use a different ClassLoader somehow, similar to this: #11658. But the changes in this PR seem to be necessary even so.

[philwalk]

To cleanup, this PR will be replaced by a new new one, when the test resources are less busy this evening.

[BarkingBad]

...is there a way to point this PR to the new branch?

It's maybe not intuitive, but quite straightforward. You could just simply hard reset your current branch to the new branch and force push :)

Thanks for your input :D