VirtusLab / scala-cli

Scala CLI is a command-line tool to interact with the Scala language. It lets you compile, run, test, and package your Scala code (and more!)
https://scala-cli.virtuslab.org
Apache License 2.0
557 stars 130 forks source link

Flyway is unable to find migrations on classpath of project packaged with scala-cli #1986

Open lbialy opened 1 year ago

lbialy commented 1 year ago

Version(s) 0.2.1

Describe the bug Flyway is a SQL migration tool that is capable of loading migrations from classpath resources. This feature is used when delivering application that has a capability to migrate it's own database schema during initialisation phase. Embedding flyway in Scala looks similar to this snippet:

  org.flywaydb.core.Flyway
    .configure()
    .dataSource("jdbc:postgresql://localhost:5432/postgres", "postgres", "postgres")
    .locations(s"classpath:migrations")
    .load()
    .migrate()

In this case the .locations(s"classpath:migrations") informs Flyway to look for sql scripts with Vn prefix (where n is an ordinal) in classpath directory called migrations. This works fine when running Flyway with testcontainers-scala via scala-cli test . but fails when used with scala-cli package ., both with --standalone and --assembly.

To Reproduce Clone this repository: https://github.com/lbialy/scala-cli-classpath-issue, you will need docker to run it (or a standalone postgres running on your localhost:5432 with postgres database, user named postgres with postgres password). You can cross-validate by running scala-cli test . in the repository root - this will run postgres via testcontainers and run migrations on it. Then run ./reproduce.sh script which will in turn package the app (it takes an optional argument, either --standalone or --assembly), run postgres in docker and run the reproducer app against it. All this ceremony is required because Flyway doesn't load migrations without an active jdbc connection, sorry. Flyway fails to find migrations when executed from packaged application.

Expected behaviour Flyway should be able to find migrations on classpath. This works fine with no changes to code at all when exported to sbt + sbt-native-packager. I assume it has to have something to do with how classpath is composed in cousier bootstraped apps / coursier assemblies.

lbialy commented 1 year ago

fyi: there is a small difference between classpath:migrations vs classpath:/migrations but both variants fail with scala-cli package (and both variants work fine in tests!). For some reason classpath:/migrations does not work properly with --assembly and throws on Source.fromResource. Relative path dependent resource loading code on JVM is probably one of the worst things on the whole platform.

victorqrt commented 7 months ago

Hello,

Did anybody get a chance to look into this? I am running into the same issue with scala-cli 1.2.2.

I have a similar reproducer using an sqlite jdbc connection: https://github.com/victorqrt/sc-flyway - this is using a scala wapper to flyway but the issue arises with native java flyway as well. To reproduce, you can build a jar with make, and it does not find the sql scripts in the classpath when run, while scala-cli run . does.

Just like described above, the problem goes away for me if exporting / rewriting my build to sbt or maven.

Thank you for scala-cli!

tgodzik commented 7 months ago

@lbialy did you find a workaround? We haven't managed to properly look at this

lbialy commented 7 months ago

Nope. I recall the issue is around how coursier packages the classpath but it's kinda surprising that even with --assembly things are borked. To troubleshoot it I'd start with a) unzipping the jars and checking that migrations directory is present b) checking what the actual entrypoint is, for bootstrapped jars it's not the main class of the user, it's the main class of coursier-bootstrap package that then downloads all the dependencies and builds a classpath with a separate classloader (this would probably explain why flyway would fail with bootstrapped jars but not assembly ones). Given that you can run assembly with user's main class I guess there's no classloader magic happening there but I'd still check what's inside. c) compare an assembly jar built by sbt-assembly with assembly jar built by scala-cli (unzipped of course) d) finally go inside flyway source and check what is happening when you use "classpath:migrations" parameter to see what code is used to load these files. Given that it's a name of directory there has to be something interesting happening there because ordinarily one can't just list files in a dir on classpath. Code handling this listing of files in that directory might be doing something that trips on some small difference that could be found in c)

victorqrt commented 7 months ago

Comparing unzip -l outputs with the sbt assembly jar, I realized scala-cli doesn't seem to create zip entries for intermediary directories (that is, an entry in the zip ending in /) while sbt does.

Patching the jar produced by scala-cli, using my example repo linked above (my migrations are expected under "classpath:sql"):

make
java -jar sc-flyway.jar
# ...
# migrations not found

# patching zips does not work well with the shell stub scala-cli prepends to the jar, "fix it" beforehand
zip -F sc-flyway.jar --out sc-flyway.zip

# append empty directory to the zip, just to get an entry for it
mkdir sql
zip -g sc-flyway.zip sql/

java -jar sc-flyway.zip 
# 🎉

And boom! migrations are found and applied.

I think it boils down to this usage of ClassLoader::getResources. I didn't dig but that method probably ends up producing URLs based on zip entries?

When using flyway with sql scripts it is nice to be able to specify an single folder in whatever code/config you have and then drop files in there - It's probable other libraries do this and then rely on similar methods to match a zip entry that won't be there when packaging using scala-cli.

Maybe this warrants a fix? It seems adding entries for directories in the produced jar would be enough at least for this use case. Would certainly make for a nicer ux than zip-patching assembly jars :)

In any case, many thanks @lbialy for the pointers. I was comparing the "unzipped" jars before as suggested, digging through manifests, etc but if the problem is what it seems to be, you actually miss it by unzipping...

victorqrt commented 7 months ago

Some more context/discussion on this: https://bugs.eclipse.org/bugs/show_bug.cgi?id=243163#c2

lbialy commented 7 months ago

Glad to hear that! Sorry if I let you on a merry chase a bit. I think we should mirror the behavior sbt has so yeah, fix to scala-cli.

On Thu 18. Apr 2024 at 12:32, victorqrt @.***> wrote:

Some more context/discussion on this: https://bugs.eclipse.org/bugs/show_bug.cgi?id=243163#c2

— Reply to this email directly, view it on GitHub https://github.com/VirtusLab/scala-cli/issues/1986#issuecomment-2063547648, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBVNUUCTI3EMLNP5FM7RE3Y56ONBAVCNFSM6AAAAAAWONMZXSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRTGU2DONRUHA . You are receiving this because you were mentioned.Message ID: @.***>

victorqrt commented 7 months ago

Great! I think this may be a candidate? https://github.com/VirtusLab/scala-cli/blob/81acfafd6968d8dfc6b5e40ba9fbd3384237aa85/modules/build/src/main/scala/scala/build/internal/resource/ResourceMapper.scala#L11C7-L11C71

I can't try and patch right now, If nobody picks it up I might have a look when I get time - Thanks again

lbialy commented 7 months ago

It's a question to @Gedochao and @alexarchambault I think.