crashinvaders / gdx-lml

:octocat: LibGDX utility libraries.
Apache License 2.0
14 stars 1 forks source link

Classpath scanning in-scan-ity and an irregular regex #2

Closed tommyettinger closed 1 year ago

tommyettinger commented 1 year ago

Let me start off by saying thanks for maintaining the LML-related repos! I also need to start off by saying it is really hard to actually track down the deepest errors here, and this is likely related to or even entirely a bug somewhere in gdx-liftoff.

Usually I get notified of an exception in gdx-liftoff by a catch block in the application startup, which assumes the exception was thrown by the classpath scanner and launches the fallback classpath scanner, and that always fails on Windows (I'll get back to this in a second). The errors I get look like

Exception in thread "main" com.badlogic.gdx.utils.GdxRuntimeException: Unable to scan classpath.
    at com.github.czyzby.autumn.nongwt.scanner.FallbackDesktopClassScanner.findClassesAnnotatedWith(FallbackDesktopClassScanner.java:58)
    at com.github.czyzby.autumn.context.ContextInitializer.initiateMetaComponents(ContextInitializer.java:361)
    at com.github.czyzby.autumn.context.ContextInitializer.initiate(ContextInitializer.java:291)
    at com.github.czyzby.autumn.mvc.application.AutumnApplication.initiateContext(AutumnApplication.java:78)
    at com.github.czyzby.autumn.mvc.application.AutumnApplication.create(AutumnApplication.java:65)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Window.initializeListener(Lwjgl3Window.java:416)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Window.update(Lwjgl3Window.java:366)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.loop(Lwjgl3Application.java:192)
    at com.badlogic.gdx.backends.lwjgl3.Lwjgl3Application.<init>(Lwjgl3Application.java:166)
    at gdx.liftoff.MainKt.main(main.kt:148)
    at gdx.liftoff.MainKt.main(main.kt)
Caused by: java.util.regex.PatternSyntaxException: Unexpected internal error near index 1
\
    at java.base/java.util.regex.Pattern.error(Pattern.java:2038)
    at java.base/java.util.regex.Pattern.compile(Pattern.java:1799)
    at java.base/java.util.regex.Pattern.<init>(Pattern.java:1440)
    at java.base/java.util.regex.Pattern.compile(Pattern.java:1079)
    at java.base/java.lang.String.split(String.java:3149)
    at java.base/java.lang.String.split(String.java:3195)
    at com.github.czyzby.autumn.nongwt.scanner.FallbackDesktopClassScanner.getBinaryClassName(FallbackDesktopClassScanner.java:103)
    at com.github.czyzby.autumn.nongwt.scanner.FallbackDesktopClassScanner.extractFromBinaries(FallbackDesktopClassScanner.java:73)
    at com.github.czyzby.autumn.nongwt.scanner.FallbackDesktopClassScanner.findClassesAnnotatedWith(FallbackDesktopClassScanner.java:56)
    ... 10 more
Caused by: java.util.regex.PatternSyntaxException: Unexpected internal error near index 1

But these stacktraces don't include the original exception, which could be anything that wasn't caught during normal program flow. The issue on Windows is helpfully noticed by IntelliJ IDEA, which points this out in the file FallbackDesktopClassScanner.java : idea64_3oDrlSrnc9 Because the file separator is the backslash char on Windows, it tries to replace the regex "\\", which won't work because it isn't a complete regex. Changing the classFolders initialization like this should fix this part of the error:

final String[] classFolders = classPathFile.getPath().split(Pattern.quote(File.separator));

There's still something wrong with the original classpath scanner (not the fallback), and I can't easily tell what it is. It seems like the issue may be related to an uncaught exception (or more likely, a Throwable like a NoSuchMethodError), forcing the classpath scan to stop. The only way I've found that fixes the issue is to use a dep on "io.github.lukehutch:fast-classpath-scanner:2.21" instead of the newer class. This may very well be a Liftoff issue at this point. I have actually updated my fork to include the regex fix, the change to the lukehutch FCS version, and updates that address breaking changes in libGDX 1.12.0. I can PR my changes, but you may want to revert the FCS change if the current classpath scanner works everywhere else. I can just keep using my fork.

The reason the classpath issue is in-scan-ity instead of a sane issue is that I have no idea how to track down why the lukehutch FCS version works in Liftoff, but the newer class graph code doesn't. Some information about the error is being swallowed up. Let me know if there's anything I can do to test this!

metaphore commented 1 year ago

Thanks for the details. Interesting case indeed.

Regarding that regex error, a good find, thanks! I'll patch it up.

About that class scanner thing, at this point, I can't extensively test the Autumn libs and thus would go with support for both class scanners. I can revive the old FCS-based scanner and add an optional Autumn lib (if it compiles). That way we still can at least move on with the working applications. And if something comes up related to the issue, we can revisit and research/fix it.

metaphore commented 1 year ago

And speaking of any future improvements or patches, just feel free to write directly to master. I think it would make our lives much easier that way.

tommyettinger commented 1 year ago

Supporting both class scanners seems like the best solution; good idea! I can try to PR the 1.12.0 changes, they're just muddled in with other changes.

metaphore commented 1 year ago

libGDX 1.12.0 migration is done and the old fcs implementation added as a separate lib. I'd appreciate it if you could test the latest snapshot version and let me know if it works for gdx-liftoff (both the main LML/Autumn libs and the class scanner one).

So the two new libs are:

Please use the 1.10.1.12.0-SNAPSHOT version (you'll need the snapshot repos declared for that):

repositories {
  // ...
  maven { url "https://oss.sonatype.org/content/repositories/snapshots/" }
  maven { url "https://s01.oss.sonatype.org/content/repositories/releases/" }
}

dependencies {
  // ...
  implementation "com.crashinvaders.lml:gdx-autumn-desktop-fcs:1.10.1.12.0-SNAPSHOT"
}
tommyettinger commented 1 year ago

I'm using these now and they seem to work just fine! You can close whenever you want to -- maybe now, or maybe when there's a release? This seems tidily wrapped up.

metaphore commented 1 year ago

Yeah, there have been some major changes to Gradle config for this release and I wanted to test it a bit more. It all works fine with my apps, but I don't use the old FCS implementation. So did you actually try to move gdx-liftoff to the 1.10.1.12.0-SNAPSHOT libs?

tommyettinger commented 1 year ago

Yep! Not in a release yet, but it runs and makes projects currently: https://github.com/tommyettinger/gdx-liftoff/blob/master/gradle.properties#L9

metaphore commented 1 year ago

Lovely! Thanks! Then I'm going to release this version of LML to prod. I will let you know when the lib is available on Maven Central

metaphore commented 1 year ago

The 1.10.1.12.0 artifacts are available on Maven Central. Please also update the LML libs' version for the liftoff templates.