Concrete references to com.google.re2j.* classes means re2j is non-optional or the android r8 shrinker throws an error

[mikehardy]

( 👋 originally posted directly as a comment on the PR so that it could be tied directly with a click to the code referenced - ideally as a help for maintainer(s) - thank you for jsoup, we appreciate the library!)

---

This was discovered while attempting to adopt the new jsoup version in the AnkiDroid project, where we have at a non-trivial development cost implemented "release mode" emulator testing after having shrinker errors in the past. It surfaced the problem

ankidroid/Anki-Android#19985
https://github.com/ankidroid/Anki-Android/actions/runs/20636686551/job/59315193515#step:8:298

An initial attempt to solve the issue was made by simply adding the observationally-non-optional re2j transitive but that is unsatisfactory as we have also implemented an APK size comparison tool to see the impact of such changes, and re2j appears to add approximately 70 kilobytes to our APK size. That is not huge but is best avoided for something that is in our use case not going to result in a user-perceivable benefit

Can this intended-to-be-optional transitive be made actually optional here? Perhaps through reflective usage in the Re2jRegex helper similar to the "has re2j" detection elsewhere?

Thanks!

Originally posted by @mikehardy in #2407

[prajwalch]

I got the same error. For now, I am downgrading Jsoup to v1.21.2 to pass the build.

[mikehardy]

An effective workaround seems to be adding a stanza in proguard-rules.pro like so:

# Ignore intended-to-be-optional re2j classes - only needed if using re2j for jsoup regex
# jsoup safely falls back to JDK regex if re2j not on classpath, but has concrete re2j refs
-dontwarn com.google.re2j.**

in my analysis of the PR here, the hasRe2j method safely / reflectively checks if it is on the classpath prior to attempting to use any of the re2j classes, and is a gate to any use of the re2j classes, so they will not ever be used despite being concretely referenced in another file.

My testing indicated it worked as expected (no crash, re2j not on classpath so must have fallen back gracefully as intended to JDK regex)

[prajwalch]

An effective workaround seems to be adding a stanza in proguard-rules.pro like so:

# Ignore intended-to-be-optional re2j classes - only needed if using re2j for jsoup regex
# jsoup safely falls back to JDK regex if re2j not on classpath, but has concrete re2j refs
-dontwarn com.google.re2j.**

in my analysis of the PR here, the hasRe2j method safely / reflectively checks if it is on the classpath prior to attempting to use any of the re2j classes, and is a gate to any use of the re2j classes, so they will not ever be used despite being concretely referenced in another file.

My testing indicated it worked as expected (no crash, re2j not on classpath so must have fallen back gracefully as intended to JDK regex)

Thanks for the workaround. Hopefully, this will be fixed on upstream.

[jhy]

Hi there, thanks for the report. Just a first take as I am traveling for the next few days and haven't looked at it in depth.

This is definitely an optional dependency, not "intended-to-be-optional", just plain optional. As you note, the only concrete references to the re2j classes are in a jsoup class, that is only initialized if the re2j classes are available. This follows the Java classloader spec that allows this style of optional loading, without having to load everything by reflection.

We have similar patterns for e.g. the JDK 11 HttpClient already.

So on first blush, the r8 shrinker appears to not be tree shaking correctly to see that these won't be used.

It would be good if we can find why that the path to use the HttpClient is not hitting this and if we can use a similar path here. (Is it because it's in a v11 part of our MJAR, and Android doesn't see any of that? Or just an earlier default exclude rule?)

Alternatively if there are other suggestions I am also keen to hear. Perhaps we can simply document the proguard rule. Ideally we won't have a bunch of reflection code wrapping up the entire r2j interface.

[mikehardy]

No rush on my behalf @jhy, I hope you enjoy the travel. Mostly commenting to say that unfortunately I think I've reached the frontier of my direct experience in this area with the combo of release mode testing (that is, using the shrinker and adding proguard rules while testing same) vs classloading subtleties

So likely won't be able to push this forward directly

...but if you are ever curious for a quick test or verification of a final solution and don't have an easy way to do it yourself you can always propose a PR in the Anki-Android repo and it'll auto-run a release mode test for you with whatever changed jsoup library you may want to try. It reproduced there immediately in our android emulator test workflow. Standard ./gradle/libs.versions.toml type dependency management / Anki-Android/proguard.rules.pro file setup - same files visible in the PR github has cross-linked to this issue if that's useful

Cheers

[jhy]

OK, I have added the dontwarn rule into jsoup's jar file (META-INF/proguard/org.jsoup_jsoup.pro), as specified in Android's Optimization for library authors doc.

And validated in an optimized Android build without the re2j library that the R8 error no longer occurs.