Guice: Stop shading ASM?

[ronshapiro]

Per https://github.com/google/guice/pull/1169#issuecomment-395095124, maybe it's time to stop shading ASM?

That will allow people to bring their own copy of ASM if they so choose, and will unblock them when new versions of the JDK come out and Guice hasn't updated yet. We'd obviously provide a default in our maven pom for those that aren't ahead of the game. This could be helpful as the JDK continues its 6-month release cadence.

[sameb]

sgtm, i believe the reason it's shaded is because historically asm was very binary incompatible with prior releases... but i think they changed that as of recent releases.

[mkurz]

That comment from @mcculls might be relevant here as well:

Note that the build already produces a version of Guice that doesn't bundle asm or cglib:

http://repo1.maven.org/maven2/com/google/inject/guice/4.2.0/guice-4.2.0-classes.jar

This is a jar of the Guice classes before any JarJar'ing.

Also if you don't need AOP then there's the "no_aop" jar which doesn't use asm/cglib at all:

http://repo1.maven.org/maven2/com/google/inject/guice/4.2.0/guice-4.2.0-no_aop.jar

[cushon]

unblock them when new versions of the JDK come out and Guice hasn't updated yet

I don't think thing it's sufficient to not shade ASM, since the API level will also need to be updated to support new class file versions:

https://github.com/google/guice/blob/35caaec1b4af0f20ab8b3fae8a1fc98afc0fd13e/core/src/com/google/inject/internal/util/LineNumbers.java#L144

[davido]

I don't think thing it's sufficient to not shade ASM, since the API level will also need to be updated to support new class file versions:[...]

And for experimental support of JDK11 in Guice it could only use experimental Opcodes.ASM7_EXPERIMENTAL: [1].

https://gitlab.ow2.org/asm/asm/blob/master/asm/src/main/java/org/objectweb/asm/Opcodes.java#L55

[davido]

And for experimental support of JDK11 in Guice it could only use experimental Opcodes.ASM7_EXPERIMENTAL: [1].

Done in this PR #1203. With this diff Gerrit can be built with JDK11, with Bazel and with VanillaJavaBuilder (https://gerrit-review.googlesource.com/c/gerrit/+/194040):

$ bazel build --host_javabase=:jdk11 --host_java_toolchain=//:toolchain_vanilla --java_toolchain=//:toolchain_vanilla //:release
[...]
INFO: Analysed target //:release (0 packages loaded).
INFO: Found 1 target...
Target //:release up-to-date:
  bazel-bin/release.war
INFO: Elapsed time: 0.431s, Critical Path: 0.19s

[mkurz]

BTW: asm 6.2.1 is available

[sameb]

as others have mentioned, not shading asm won't actually resolve this since the code will need to change (both in guice & cglib) to support the newer JDK features.

[mkurz]

@sameb Since you are here:
Given that Java 11 will be released next week and a couple of days later ASM 7.0 will be released as well - how likely will it be you upgrade to ASM 7 and then release a new Guice version?
I work on the Play Framework and there is a new release planned within the next couple of weeks. However we would need a new Guice version with AMS 7 so that the new release also run on Java 11.

Any ETA?

[sameb]

if you ping this thread the day asm7 is released, i'll try to release a new cglib & guice same day or next day (assuming no unexpected problems arise).

[mkurz]

@sameb Great, thanks! I will ping you for sure :smile:

[mkurz]

Ping @sameb - ASM 7 is out! See https://github.com/google/guice/issues/1205#issuecomment-433649551

[mkurz]

ASM 8.0.1 available.