Skip to content

fix/scripts: Propagate runner JVM version and skip testing invalid compiler commits #19656

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 8, 2024
Merged

fix/scripts: Propagate runner JVM version and skip testing invalid compiler commits #19656

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d940c2
[scripts/bisect] Propagte JVM version of scala-cli runner to sbt and …
WojciechMazur Feb 9, 2024
2548779
[scripts/bisect] Skip bisecting commits for which compiler cannot be …
WojciechMazur Feb 9, 2024
37e0b07
[script/bisect] Use JVM 17 by default in bisect script
WojciechMazur Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions project/scripts/bisect.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//> using jvm 17 // Maximal JDK version which can be used with all Scala 3 versions, can be overriden via command line arguments '--jvm=21'
/*
This script will bisect a problem with the compiler based on success/failure of the validation script passed as an argument.
It starts with a fast bisection on released nightly builds.
Expand Down Expand Up @@ -124,6 +125,7 @@ object ValidationScript:

def tmpScalaCliScript(command: String, args: Seq[String]): File = tmpScript(s"""
|#!/usr/bin/env bash
|export JAVA_HOME=${sys.props("java.home")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason to override JAVA_HOME instead of passing the java version to scala-cli via --jvm option?
Also, this won't be taken into account when using a custom validation script. Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that user of custom validation script is an advanced user who understands not only a tested code, but also the environment in which it is being run. Especially since custom bisect script might use sbt or other build tool. We could add custom JAVA_HOME to custom scripts by populating env variables of created process, but it would require a rewrite for parts of the code. I'd try this approach and check how it behaves.

The JAVA_HOME was used instead of --jvm argument to make sure that we exactly the same JVM version. We would not be able to pass a path in --jvm argument, and extraction of --jvm argument might be difficult.

|scala-cli ${command} -S "$$1" --server=false ${args.mkString(" ")}
|""".stripMargin
)
Expand Down Expand Up @@ -242,8 +244,10 @@ class CommitBisect(validationScript: File, shouldFail: Boolean, bootstrapped: Bo
val bisectRunScript = raw"""
|scalaVersion=$$(sbt "print ${scala3CompilerProject}/version" | tail -n1)
|rm -rf out
|sbt "clean; set every doc := new File(\"unused\"); set scaladoc/Compile/resourceGenerators := (\`${scala3Project}\`/Compile/resourceGenerators).value; ${scala3Project}/publishLocal"
|${validationCommandStatusModifier}${validationScript.getAbsolutePath} "$$scalaVersion"
|export JAVA_HOME=${sys.props("java.home")}
|(sbt "clean; set every doc := new File(\"unused\"); set scaladoc/Compile/resourceGenerators := (\`${scala3Project}\`/Compile/resourceGenerators).value; ${scala3Project}/publishLocal" \
| || (echo "Failed to build compiler, skip $$scalaVersion"; git bisect skip) \
|) && ${validationCommandStatusModifier}${validationScript.getAbsolutePath} "$$scalaVersion"
""".stripMargin
"git bisect start".!
s"git bisect bad $fistBadHash".!
Expand Down