Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shaded multi-release ASM classes are missing nested classes #1724

Closed
Marcono1234 opened this issue Nov 3, 2024 · 9 comments
Closed

Shaded multi-release ASM classes are missing nested classes #1724

Marcono1234 opened this issue Nov 3, 2024 · 9 comments
Assignees
Labels
Milestone

Comments

@Marcono1234
Copy link
Contributor

Marcono1234 commented Nov 3, 2024

Version

byte-buddy-1.15.9.jar

Description

The latest Byte Buddy versions include classes for both Java 5 (regular location in JAR) and Java 8 (as multi-release classes under META-INF/versions/9), see #1657 and #1719.

The problem is that the multi-release shaded ASM classes (under META-INF/versions/9/net/bytebuddy/jar/asm/) seem to be incomplete, these classes are missing:

  • Attribute$Set.class
  • SymbolTable$Entry.class
  • SymbolTable$LabelEntry.class

So the advice in #1719 of omitting either the Java 5 or Java 8 classes to avoid duplication does not work; you encounter a ClassNotFoundException: net.bytebuddy.jar.asm.SymbolTable$Entry.

Maybe the problem is that these include patterns don't contain $ to also match nested classes (respectively an escaped \$)?

<include>org/objectweb/asm/[a-zA-Z\.]+</include>
<include>org/objectweb/asm/signature/[a-zA-Z\.]+</include>
<include>org/objectweb/asm/commons/.*Remapper.*</include>
<include>org/objectweb/asm/commons/ModuleHashesAttribute.class</include>

@raphw
Copy link
Owner

raphw commented Nov 3, 2024

Yes, indeed. The regex does not include the $ symbol, so the inner classes only exist in the Java 5 version. My bad, I fixed this now and will do a new release.

@Marcono1234
Copy link
Contributor Author

Thanks! Though it looks like your fix might contain a typo which is now causing another issue: f164ddd#r148674133 (haven't verified it yet though)

@raphw raphw self-assigned this Nov 3, 2024
@raphw raphw added the build label Nov 3, 2024
@raphw raphw added this to the 1.15.8 milestone Nov 3, 2024
@raphw
Copy link
Owner

raphw commented Nov 3, 2024

It just includes a character that will never be there, so it has no effect, but it is better to fix it of course. I assume my IDE auto-completed something there. Thanks for pointing it out!

@Marcono1234
Copy link
Contributor Author

Marcono1234 commented Nov 3, 2024

I was a bit unsure how the [...]]* will be parsed, but I just tested it now and it seems the pattern is parsed as:

  • one required char from a-zA-Z0-9$\.
  • zero or more ]
"ModuleHashesAttribute.class".matches("ModuleHashesAttribute[a-zA-Z0-9$\\.]]*.class")
// => false

(but for example ModuleHashesAttribute0.class, with trailing 0, matches)

Thanks for the follow-up fix!

@Marcono1234
Copy link
Contributor Author

Seems to be fixed in 1.15.10, thanks! (feel free to close this issue if you want)

Another thing I noticed: The multi-release directory seems to be missing these classes, is that intended?

net/bytebuddy/build/CachedReturnPlugin$Advice$Object.class
net/bytebuddy/build/CachedReturnPlugin$Advice$boolean.class
net/bytebuddy/build/CachedReturnPlugin$Advice$byte.class
net/bytebuddy/build/CachedReturnPlugin$Advice$char.class
net/bytebuddy/build/CachedReturnPlugin$Advice$double.class
net/bytebuddy/build/CachedReturnPlugin$Advice$float.class
net/bytebuddy/build/CachedReturnPlugin$Advice$int.class
net/bytebuddy/build/CachedReturnPlugin$Advice$long.class
net/bytebuddy/build/CachedReturnPlugin$Advice$short.class

@raphw
Copy link
Owner

raphw commented Nov 6, 2024

That is true. Those classes are templating classes, used for build plugins, but they are never loaded. If one wanted to use the standard build plugins, one would need to keep those.

@Marcono1234
Copy link
Contributor Author

one would need to keep those

Do you mean a user would have to manually keep those? That might not be obvious, and might cause issues when users only keep META-INF/versions/9 to reduce the JAR size (as recommended in the README).

Would it be possible and make sense to copy those classes to META-INF/versions/9 too?

(Side note: I am not affected by this currently, but I assume other users might be?)

@Marcono1234
Copy link
Contributor Author

Marcono1234 commented Nov 8, 2024

but they are never loaded

Or do you mean they are never needed at runtime, regardless of how Byte Buddy is used? I guess in that case it would not matter in most cases whether they exist in META-INF/versions/9 or not.

@raphw
Copy link
Owner

raphw commented Nov 9, 2024

They are never loaded but used as advice templates when the plugin is used. I thought about this and decided to remove the entire precompilation and rather generate the advice on demand dynamically. This makes the implementation independent of the underlying class loader, too, what always was a minor concern of mine. And this resolves this issue, too. I think the issue is save to ignore for 99.9% of users, so I will not rush a release, but it is already added to master.

@raphw raphw closed this as completed Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants