Add sloth support#4348
Conversation
|
@lbialy how close are we to publishing a stable version of |
a8e0c42 to
52da2af
Compare
|
Why is it called "lazyvalgrade", what does "grade" mean here? |
|
@SethTisue I believe that's a pun on "lazy val upgrade". |
52da2af to
330d1d0
Compare
|
https://central.sonatype.com/search?q=org.virtuslab+sloth |
|
Do we now have adopt a sloth in a zoo? And have sloth plushies? |
|
We should. 🦥 |
bbac081 to
15645e2
Compare
b243a04 to
714f8a5
Compare
…hey're not put in a JAR
…hPatcher.captureStdio` thread-safe
cb6b21a to
e1317a2
Compare
e1317a2 to
cd093e4
Compare
warcholjakub
left a comment
There was a problem hiding this comment.
Those are the issues I managed to spot, while experimenting with it in the terminal. I'll look at the code next.
| val patchedDest = value(SlothPatcher.patchJarFile(destPath, options, logger)) | ||
| if patchedDest != destPath then os.copy.over(patchedDest, destPath, createFolders = true) |
There was a problem hiding this comment.
Not sure whether this behaviour is intentional, but even if it is, we should probably warn the user.
With preamble:
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> cat Main.scala
//> using scala 3.3.8
object Greetings:
lazy val uno: String = "Heloł"
lazy val dos: String = "Greetings"
@main def run(): Unit =
import Greetings.*
println(s"$uno & $dos")
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> ./mill -i scala --power package --assembly --sloth Main.scala -o app.jar
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> java -jar app.jar
Error: Invalid or corrupt jarfile app.jarWithout preamble:
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade) [1]> ./mill -i scala --power package --assembly --preamble=false --sloth Main.scala -o app2.jar
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> java -jar app2.jar
Heloł & Greetings| private def patchIfJar(path: os.Path, logger: Logger): os.Path = | ||
| if path.ext == "jar" then patchJar(path, logger) | ||
| else path |
There was a problem hiding this comment.
Since we allow package outputs without a .jar extension, we shouldn't rely solely on the extension to determine whether it's a jar. At the very least, we should warn the user when Sloth does not patch an artifact.
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> ./mill -i scala --power package --assembly --force --sloth Main.scala -o app_no_jar_ext
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> java -jar app_no_jar_ext
WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::objectFieldOffset has been called by scala.runtime.LazyVals$ (file:/Users/jwarchol/Documents/GitHub/scala-cli/app_no_jar)
WARNING: Please consider reporting this to the maintainers of class scala.runtime.LazyVals$
WARNING: sun.misc.Unsafe::objectFieldOffset will be removed in a future release
Heloł & GreetingsThere was a problem hiding this comment.
NIT: I've noticed that //> using sloth false results in Sloth being enabled. I think it'd be a better UX, if we handled true & false or informed user, that it doesn't take arguments.
|
I'm wondering about one thing. Let's assume this hypothetical scenario:
Tried to reproduce it, and it indeed failed, but not sure how realistic and common this problem would be. Also I don't really see any straightforward solution. But yeah - just something to think about. EDIT: |
| @@ -97,8 +100,11 @@ object NativeImage { | |||
| ) | |||
|
|
|||
| if cacheData.changed then { | |||
There was a problem hiding this comment.
NIT: cacheData doesn't include the fact whether someone used sloth. So if you build an image without sloth, and then try with it enabled, it won't rebuild the image.
There was a problem hiding this comment.
oooh, this is a good catch
| Using.resource(JarOutputStream(os.write.outputStream(dest), manifest)): jos => | ||
| for | ||
| path <- os.walk(dir) | ||
| if os.isFile(path) | ||
| do | ||
| val relativePath = path.relativeTo(dir).toString.replace('\\', '/') | ||
| val entry = ZipEntry(relativePath) | ||
| entry.setLastModifiedTime(FileTime.fromMillis(os.mtime(path))) | ||
| val content = os.read.bytes(path) | ||
| entry.setSize(content.length) | ||
| jos.putNextEntry(entry) | ||
| jos.write(content) | ||
| jos.closeEntry() |
There was a problem hiding this comment.
We create a manifest in line 117, but if the user has supplied their own, we later try to add it, which causes a duplication error.
|
@warcholjakub Can you elaborate about what do you mean by sloth modifying the dependency library? In what circumstances the verification you mentioned happens? What exactly fails verification? |
I mean something like this: jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> cat SignedLib.scala
//> using scala 3.3.8
package signedlib
object SignedLib:
lazy val message = "hello from signed library"
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> cat Main.scala
import signedlib.SignedLib
@main def main(): Unit =
println(SignedLib.message)
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> ./mill -i scala --power package SignedLib.scala --library -o "signed.jar"
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> jarsigner -keystore "key.p12" -storepass changeit "signed.jar" test
jar signed.
jwarchol@vl-d-1635 ~/D/G/scala-cli (feature/lazyvalgrade)> ./mill -i scala --power run Main.scala --sloth --classpath signed.jar
[...]
Exception in thread "main" java.lang.SecurityException: SHA-384 digest error for Greetings$.class
at java.base/sun.security.util.ManifestEntryVerifier.verify(ManifestEntryVerifier.java:254)
at java.base/java.util.jar.JarVerifier.processEntry(JarVerifier.java:248)
at java.base/java.util.jar.JarVerifier.update(JarVerifier.java:235)
at java.base/java.util.jar.JarVerifier$VerifierStream.read(JarVerifier.java:453)
at java.base/jdk.internal.loader.Resource.getBytes(Resource.java:106)
at java.base/jdk.internal.loader.URLClassPath$JarLoader$1.getBytes(URLClassPath.java:745)
at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:773)
at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:691)
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:620)
at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:578)
at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:490)
at Main$package$.run(Main.scala:9)
at run.main(Main.scala:7)As I said, I'm not sure whether it's a realistic scenario - just smth I noticed. EDIT: Modified the scenario a bit. |
|
I did some research and you're right that the problem exists but fortunately: a) signing is now quite rare b) the whole signing mechanic is contingent on the presence of files. @Gedochao would it be possible to include a step where deps are copied to |
Adds Sloth support behind
--sloth///> using slothhttps://github.com/VirtusLab/sloth
Scala 3, up till 3.7.x series, has been using the (now terminally deprecated)
sun.misc.UnsafeAPI. This in turn results in some ugly warnings on JDK 24+ (and likely errors in the future).Now, while Scala 3.8+ uses a new implementation, and Scala 3.3 allows for the new implementation on JDK 9+ behind the
-Yfuture-lazy-valscompiler flag, the problem is not entirely solved. Even if your app is on the latest Scala, it likely still depends on libraries built with Scala 3.3 (or some other <3.8 version). That in turn brings the problem back. And since it is not widespread to suffix Scala 3 libraries with the minor, as we did in Scala 2 days, it is not entirely easy to spot which libraries will pose a problem.Sloth is a bytecode patcher, which is capable of patching all past lazy val implementations (including the pre-3.3 one) up to the current one. Kudos to @lbialy for developing it.
Behind the
--slothflag, this PR allows to post-process and patch the bytecode of your app/artifacts to silently get rid of the problem.The
--sloth-agentflag runs Sloth as a Java agent instead, allowing it to i.e. run in the JVM your tests live.More details on how it works and what it does at https://github.com/VirtusLab/sloth
This feature is brought to the following sub-commands:
Checklist
scala-cli fmt .)scalafix(./mill -i __.fix)./mill -i 'generate-reference-doc[]'.run)How much have your relied on LLM-based tools in this contribution?
extensively, Cursor + Claude
How was the solution tested?
./mill -i integration.test.jvm '*sun.misc.Unsafe*'