Rfb support and new JUL logging #159
Conversation
|
It would be nice to get rfb transformers working for plugins. I might need some help with that. |
…class-loader option
|
Plugin transforming is quite a special case, since you would have to delegate plugin classes to launchwrapper (or rfb in this case) but not define them in the lw/rfb classloader. My proposal would be enumerating transformers/plugins manually and having a config of "safe" transformers that can interact with plugins, then call it directly |
I found method https://github.com/GTNewHorizons/RetroFuturaBootstrap/blob/c948dae9fd90c1db88385915c328dfd51971a294/src/main/java/com/gtnewhorizons/retrofuturabootstrap/URLClassLoaderWithUtilities.java#L37 that can be helpful. This method not define class itself, public and can be invoked from PluginClassLoader. Example use from LaunchClassLoader here: https://github.com/GTNewHorizons/RetroFuturaBootstrap/blob/c948dae9fd90c1db88385915c328dfd51971a294/src/main/java/net/minecraft/launchwrapper/LaunchClassLoader.java#L313 |
|
An alternative option is to use rfb to load plugins directly without PluginClassLoader (the plugin must be ready for this). Unfortunately, such plugins will not be able to interact with others from PluginClassLoader |
About that method, seems like it only invokes rfb transformers while skipping normal lw transformers, which can be good and bad? Rfb transformers would be less common and would not cause as many issues as normal transformers, so less manual labor. But how should mixins be handled? I'm open to more opinions on how plugin transformation should be handled at all. |
|
rfb transformers seem safe and are required to work on new versions of Java. We can make options for using rfb and regular transformers the same way remapping is configured now. We can use the runTransformers method via reflection, or if we really need it, create an issue to the authors of RFB. |
|
Enabling rfb and lw transformers did not break the plugins I tested. However, plugins like NBT API and PowerNBT do not work correctly in both staging and rfb-support (this PR) branches. The problem is that |
|
That issue in specific was caused by including unimixin, not sure why, but it breaks reflection on vanilla classes due to reflection trying to find unmapped classes. |
I not using unimixins while testing. |
|
Replace Class.forName to classloader.loadClass works for my test plugin. May be need apply this fix to all plugins by asm transformer? |
|
I tested this PR with GTNH 5.6.1 and popular plugins
Plugins (16): CoreProtect, Chatty, LuckPerms, MultiWorld, WorldEdit, Essentials, EssentialsProtect, EssentialsChat, EssentialsAntiBuild, ClearLag, WorldBorder, TestPlugin, EssentialsSpawn, WorldGuard, Multiverse-Core, Crucible_Server Can you review this PR? |
The logger hack was supposed to fix that by handing over logger control to bukkit console handler |
|
I wonder if it would be possible to be able to switch between rfb and lw with a simple option toggle, it would require changing crucible bootstrapping a bit though, so something for me to do later. |
I try fix this without return logger hack
Probably a bad idea. Rfb contains modified lw classes |
With a modified bootstrapping, it either loads crucible lw or rfb, so it would not have any conflict. |
|
I am testing this pull-request to check for the compats of these changes. [CrashReport from EnderIOAddons]CrashReportjava.lang.NullPointerException: Cannot invoke "net.minecraft.item.ItemStack.func_77973_b()" because "info.loenwind.enderioaddons.recipe.Recipes.capBankCreative" is null
at Launch//info.loenwind.enderioaddons.plant.EioaGrowthRequirement.<init>(EioaGrowthRequirement.java:34)
at Launch//info.loenwind.enderioaddons.plant.EioaCropPlant.<init>(EioaCropPlant.java:40)
at Launch//info.loenwind.enderioaddons.machine.afarm.TileAfarm.registerPlants(TileAfarm.java:138)
at Launch//info.loenwind.enderioaddons.machine.afarm.AgriDetector.registerPlants(AgriDetector.java:35)
at Launch//info.loenwind.enderioaddons.proxy.ClientAndServerProxy.init(ClientAndServerProxy.java:66)
at Launch//info.loenwind.enderioaddons.EnderIOAddons.init(EnderIOAddons.java:80)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at Launch//cpw.mods.fml.common.FMLModContainer.handleModStateEvent(FMLModContainer.java:532)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at Launch//com.google.common.eventbus.EventSubscriber.handleEvent(EventSubscriber.java:74)
at Launch//com.google.common.eventbus.SynchronizedEventSubscriber.handleEvent(SynchronizedEventSubscriber.java:47)
at Launch//com.google.common.eventbus.EventBus.dispatch(EventBus.java:322)
at Launch//com.google.common.eventbus.EventBus.dispatchQueuedEvents(EventBus.java:304)
at Launch//com.google.common.eventbus.EventBus.post(EventBus.java:275)
at Launch//cpw.mods.fml.common.LoadController.sendEventToModContainer(LoadController.java:212)
at Launch//cpw.mods.fml.common.LoadController.propogateStateMessage(LoadController.java:190)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at Launch//com.google.common.eventbus.EventSubscriber.handleEvent(EventSubscriber.java:74)
at Launch//com.google.common.eventbus.SynchronizedEventSubscriber.handleEvent(SynchronizedEventSubscriber.java:47)
at Launch//com.google.common.eventbus.EventBus.dispatch(EventBus.java:322)
at Launch//com.google.common.eventbus.EventBus.dispatchQueuedEvents(EventBus.java:304)
at Launch//com.google.common.eventbus.EventBus.post(EventBus.java:275)
at Launch//cpw.mods.fml.common.LoadController.distributeStateMessage(LoadController.java:119)
at Launch//cpw.mods.fml.common.Loader.initializeMods(Loader.java:739)
at Launch//cpw.mods.fml.server.FMLServerHandler.finishServerLoading(FMLServerHandler.java:97)
at Launch//cpw.mods.fml.common.FMLCommonHandler.onServerStarted(FMLCommonHandler.java:323)
at Launch//net.minecraft.server.dedicated.DedicatedServer.func_71197_b(DedicatedServer.java:270)
at Launch//net.minecraft.server.MinecraftServer.run(MinecraftServer.java:644)
at java.base/java.lang.Thread.run(Thread.java:1583)Seems to be a problem on a mixin over ItemStack. Maybe the change of loading order of mixins caused that. Anyway, EnderIoAddons is not the most stable mod in the World, so maybe is something specific with him that can be fixed on the mod itself. But is already something to keep in mind.
|
|
Well, without enabling the lwTransformer everything seems fine. (tecnically is meanted to be enabled for specific plugins so whatever) Gonna try to fix the compatibility with the EnderIOAddons. |
|
The logging behavior does not exactly match the logging behavior in staging. You can help to fix the remaining issues lwTransformers may not be safe for plugins, so they are not enabled by default |
This PR fixes bugs related to the old version of ASM in mods such as #148
Instead of launchwrapper, https://github.com/GTNewHorizons/RetroFuturaBootstrap is used which provides transformers that fix this and other problems.
About JUL
RFB breaks hacks used by Crucible to implement logging of plugins and mods. These hacks were removed, and a simplified version of the log4j-jul library was written instead. The log4j-jul library itself has a minimum version of 2.1 and is not compatible with log4j 2.0-beta9. Updating to the latest log4j is complicated by the project structure
About SystemClassLoader
RFB supports two launch options - with the argument
-Djava.system.class.loader=com.gtnewhorizons.retrofuturabootstrap.RfbSystemClassLoaderand with the argument-Drfb.skipClassLoaderCheck=true. I don't know which option will be better for Crucible, I didn't notice any difference in behavior during testing.This change may be break something. Requires thorough testing!