Allow registering additional packages in the transformable layer for generated classes #90
Description
A regression due to the introduction of module-based transformation has occurred where synthetic classes cannot be placed into a package that does not already exist in the transformable module layer.
The ModuleClassLoader will only pass class load requests to transformers for classes in packages it already knows about, so Mixin's strategy of requesting a class in a package it controls will not work unchanged.
Some options for solutions are:
- launch plugins need to be restricted to only adding classes to existing packages
- launch plugins and/or transformation services should be able to provide additional packages when the transformable module layer is built.
Java has added a hidden classes feature which allows adding new classes to a module at runtime, but that is often not suitable for bytecode transformations because hidden classes cannot be directly referenced, and is such unlikely to be a solution for this issue.
Initial discussion from #non-api-modding
``` Smelly² — 09/29/2021 I'm trying to port my mixins over to 1.17.1, and I'm having a really weird crash happening when the game is running The logs say there were no errors when processing one of my specific mixins, but later in game I'm getting this: java.lang.NoClassDefFoundError: org/spongepowered/asm/synthetic/args/Args$1 Does anybody know what would cause this? I also noticed earlier in the logs it says, [mixin/]: ArgsClassGenerator assigning org.spongepowered.asm.synthetic.args.Args$1 for descriptor (JLjava/util/List;Ljava/util/Optional;)V But I'm guessing that step didn't go as planned? nessie❦ — 09/29/2021 i suggest giving us more information like full log files and your mixin and potentially your buildscript Mumfrey — 09/30/2021 Args$1 is a mixin-generated synthetic subclass of Args which is created when you use the @ModifyArgs injector It seems like mixin never gets the request to generate the class though, as it seems like it's failing earlier in modlauncher I think we might need @cpw on this one it seems on the surface like SJH is deciding the class doesn't exist and then instead of ML passing it through the rest of the transformers/plugins with isEmpty set, it's just bombing out before that but that's just what I can glean from reading the stack trace either way, the culprit in your code will be modifyargs, but it's nothing you've done wrong Smelly² — 09/30/2021 Yeah, I suspected something like that to be the cause cpw — 09/30/2021 ? ZekerZhayard (ping) — 09/30/2021 We think there is currently no way to generate a non-existent class through LaunchPlugin or TransformationService 👀 cpw — 09/30/2021 is it a bug? that should work... LlamaLad7 — 09/30/2021 doesn't seem to when Mixin tries to make args subclasses we get ArgsClassGenerator assigning org.spongepowered.asm.synthetic.args.Args$1 for descriptor (Lcom/mojang/authlib/yggdrasil/YggdrasilAuthenticationService;Lnet/minecraft/client/main/GameConfig;)Vand then a CNFE
Mumfrey — 09/30/2021
and I confirmed this morning the request for that class never hits the service
what's weird is that inner classes which get repatriated, effectively generated, seem to work fine
cpw — 09/30/2021
is it going into the wrong classloader?
ZekerZhayard (ping) — 09/30/2021
https://github.com/MinecraftForge/securejarhandler/blob/main/src/main/java/cpw/mods/cl/ModuleClassLoader.java#L127-L134
TCL seems to only look for classes with existing package names, and throws CFNE directly in other cases
cpw — 09/30/2021
yeah. you won't be able to birth a class into a package that's not already associated with a module
module package lists are fixed at module creation time. there's no way around that.
Mumfrey — 09/30/2021
ok but I changed the package it generates the classed in to the same as the Args class itself, and it still bombs
shouldn't org.spongepowered.asm.mixin.injection.invoke.arg be associated with a module?
since a real class exists in it
cpw — 09/30/2021
it should be.
Mumfrey — 09/30/2021
if I set the package it generates the classes in to net.minecraft.client it works, if I use any existing mixin package it bombs
if I use an arbitrary package it bombs, eg. foo.bar
I did wonder if it's because my Mixin is dev-time and is being loaded from a flat dir rather than jar, but I tried using org.objectweb.asm and it bombs too
ZekerZhayard (ping) — 09/30/2021
I think only the package located in TCL is available, both asm and mixin are located in MCL
cpw — 09/30/2021
it's probably because mixin is loaded at the non-transformable level
you have to target a package in a module in the GAME layer
Mumfrey — 09/30/2021
tbh I have no idea how the module hackery works
cpw — 09/30/2021
yeah. its complicated for sure.
Mumfrey — 09/30/2021
all I can tell you is mixin synthesizes two types of class: repatriated inner classes, and args classes
cpw — 09/30/2021
i basically partitioned it into four layers. The top layer is where mixin and shit loads.
Mumfrey — 09/30/2021
the args classes are the ones that break because the transformer never gets the chance to generate them
cpw — 09/30/2021
the bottom layer (GAME) is where transforming happens
so yeah, if your targetting a package NOT in GAME layer, you won't have much success
Mumfrey — 09/30/2021
so how can that package be added to the GAME layer?
cpw — 09/30/2021
good question. that might be a thing we need to add to the API
to allow you to generate synthetic packages
probably, it'll be a new target type for the thingy thing.
Mumfrey — 09/30/2021
well the args subclasses are generated in a package of their own so that they don't conflict with anything, so just that one package needs to be permitted
or find some way to enable the old behaviour
cpw — 09/30/2021
old behaviour won't be possible sadly
not without a giant ctrl-z
and then resolving all the bullshit j16 added differently
anyway, i'll try and add it when i finally get out of this giant burnout
Mumfrey — 09/30/2021
so there's no way for it to fail over for nonexistent classes?
<details>