-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix plus Workaround for 13760 #14241
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
Changes from all commits
c150972
1da66ba
05b3fd7
0f058e1
621bbec
34c739e
7eacdf2
4fa4e97
4303a16
3ff19fd
2e45931
2dcd722
7a6a6ac
64b2ddc
a0d7fc1
459329e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,6 +81,9 @@ case class Settings( | |
def withSave: Settings = | ||
this.copy(save = true) | ||
|
||
def noSave: Settings = | ||
this.copy(save = false) | ||
|
||
def withModeShouldBePossibleRun: Settings = | ||
this.copy(modeShouldBePossibleRun = true) | ||
|
||
|
@@ -135,6 +138,8 @@ object MainGenericRunner { | |
) | ||
case "-save" :: tail => | ||
process(tail, settings.withSave) | ||
case "-nosave" :: tail => | ||
process(tail, settings.noSave) | ||
case "-with-compiler" :: tail => | ||
process(tail, settings.withCompiler) | ||
case (o @ javaOption(striped)) :: tail => | ||
|
@@ -207,16 +212,18 @@ object MainGenericRunner { | |
case ExecuteMode.Script => | ||
val targetScript = Paths.get(settings.targetScript).toFile | ||
val targetJar = settings.targetScript.replaceAll("[.][^\\/]*$", "")+".jar" | ||
val precompiledJar = Paths.get(targetJar).toFile | ||
val precompiledJar = File(targetJar) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this needed? Nio is newer and usually more respected that old io API, so I am curious There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is not needed, but it now matches how File instanced str created everywhere else in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you feel like |
||
val mainClass = if !precompiledJar.isFile then "" else Jar(targetJar).mainClass.getOrElse("") | ||
val jarIsValid = mainClass.nonEmpty && precompiledJar.lastModified >= targetScript.lastModified | ||
val jarIsValid = mainClass.nonEmpty && precompiledJar.lastModified >= targetScript.lastModified && settings.save | ||
if jarIsValid then | ||
// precompiledJar exists, is newer than targetScript, and manifest defines a mainClass | ||
sys.props("script.path") = targetScript.toPath.toAbsolutePath.normalize.toString | ||
val scalaClasspath = ClasspathFromClassloader(Thread.currentThread().getContextClassLoader).split(classpathSeparator) | ||
val newClasspath = (settings.classPath.flatMap(_.split(classpathSeparator).filter(_.nonEmpty)) ++ removeCompiler(scalaClasspath) :+ ".").map(File(_).toURI.toURL) | ||
if mainClass.nonEmpty then | ||
ObjectRunner.runAndCatch(newClasspath :+ File(targetJar).toURI.toURL, mainClass, settings.scriptArgs) | ||
ObjectRunner.runAndCatch(newClasspath :+ File(targetJar).toURI.toURL, mainClass, settings.scriptArgs).foreach { (t: Throwable) => | ||
t.printStackTrace() | ||
} | ||
Comment on lines
+224
to
+226
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it returning There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, something like that seems reasonable |
||
else | ||
Some(IllegalArgumentException(s"No main class defined in manifest in jar: $precompiledJar")) | ||
else | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#!bin/scala -nosave | ||
|
||
def main(args: Array[String]): Unit = { | ||
println(new java.sql.Date(100L)) | ||
System.err.println("SCALA_OPTS="+Option(System.getenv("SCALA_OPTS")).getOrElse("")) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this? There is
save = false
by default afaikThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
fromSCALA_OPTS
). There is a new command line option-nosave
that will resetsave = false
afterSCALA_OPTS
have been processed. The new option can be added to the hashbang line. Our newBashScriptTests
test case verifies that it can override the-save
option.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, sounds good