Switch back to ASM 5.0 #4
Conversation
|
Was it 6.0 or 6.1 that has the issue with changing the ordering? |
|
6.1. 6.0 is fine, but I'm using 5.0.4 because FG 2.2 uses a version of 5 and I'd rather avoid bumping the version there just to be safe. (ASM 6.0 would be used with FG 2.3) |
|
So we need two branches, one for FG2.2 and 2.3? |
|
I don't think we would need to. FG 2.3 (and 2.2) both currently target MCInjector 3.4-SNAPSHOT, with 2.2 referencing it directly and 2.3 doing shade('de.oceanlabs.mcp:mcinjector:3.4-SNAPSHOT'){
exclude group: 'org.ow2.asm', module: 'asm-debug-all'
}and also referencing another thing that uses ASM 6.0. This setup is definitely a bit janky, but it will continue to work here (and the same thing is needed for Srg2Source and launchwrapper which also both haven't updated (?)). |
|
Everything FG is jankey, but what specifically are you referring to S2S and LaunchWrapper? S2S shouldn't be using ASM, and LaunchWrapper shouldn't be hard depped in FG itself. If both branches reference 3.4 where is this actually being pulled in then? |
|
I'm referring to this. SRG2Source does reference it and it looks like it's used for ClassTree. LaunchWrapper is listed as a The exclude stuff was added in MinecraftForge/ForgeGradle@bac87b1, for reference. 3.4 targeted ASM 5.0.4, so that exclude is there for that. When this is merged, both ForgeGradle branches would switch to targeting 3.6.1. |
|
Alright, again dep management is a bit of a PITA. We shadow all deps into the FG jar, and the artifact names changed causing us to use both versions, which is why I made the excludes. |
This is necessary because FG's patching approach is fairly brittle, so even a technically non-breaking upgrade to one of its deps (e.g. ASM 6.0 -> 6.1) can cause it to break. See ModCoderPack/MCInjector#4 for an example.
This is necessary because FG's patching approach is fairly brittle, so even a technically non-breaking upgrade to one of its deps (e.g. ASM 6.0 -> 6.1) can cause it to break. See ModCoderPack/MCInjector#4 for an example.
This is necessary because FG's patching approach is fairly brittle, so even a technically non-breaking upgrade to one of its deps (e.g. ASM 6.0 -> 6.1) can cause it to break. See ModCoderPack/MCInjector#4 for an example.
This is necessary because FG's patching approach is fairly brittle, so even a technically non-breaking upgrade to one of its deps (e.g. ASM 6.0 -> 6.1) can cause it to break. See ModCoderPack/MCInjector#4 for an example.
This is necessary because FG's patching approach is fairly brittle, so even a technically non-breaking upgrade to one of its deps (e.g. ASM 6.0 -> 6.1) can cause it to break. See ModCoderPack/MCInjector#4 for an example.
2 participants
This switches back to ASM version 5.0 while still maintaining the parameter annotation changes (#3).
This code is a bit hacky, because ASM actually did not provide a direct way to change the number of annotations until version 6.1. However, we cannot use ASM 6.1 yet due to another change (reording of how attributes are output, which breaks forge's binary patches when used in ForgeGradle's
mergeJartask). Fortunately(ish) ASM had a system to mark extra annotation entries as synthetic (it actually generated and then removed those to work around the same javac "bug" that proguard tries to fix). We can simply add the same marker, and then ASM will remove the corresponding values when writing the parameter annotation tables.This hack does not work in ASM 6.1. While I could have merged both together, I decided not to (and instead to explicitly make the code fail on ASM 6.1) because if ASM 6.1 is used for other reasons, then
mergeJarwill break. In 1.13, I expect that we will switch to a newer version of MCInjector, and we can also switch to ASM 6.1 (and the previous 6.1-only implementation) at that time.