From aab8adb884480383b2be99146d0fb2843dc00fd2 Mon Sep 17 00:00:00 2001 From: Justin Date: Mon, 6 Jul 2020 15:30:10 -0600 Subject: [PATCH] Add simple patch checker and cleanup patches (#6851) * Add simple patch checker and cleanup patches * Address comments * move task implementation * genPatches is now finalized by checkPatches * the S2S artifacts are automatically removed * added class and method access checking --- .gitignore | 1 - build.gradle | 14 +- buildSrc/.gitignore | 2 + .../forge/tasks/CheckPatches.groovy | 213 ++++++++++++++++++ .../renderer/FirstPersonRenderer.java.patch | 7 - .../client/renderer/FogRenderer.java.patch | 6 +- .../entity/EntityRendererManager.java.patch | 10 +- .../entity/monster/ShulkerEntity.java.patch | 12 +- .../client/CClientSettingsPacket.java.patch | 2 +- .../play/server/SJoinGamePacket.java.patch | 10 - .../play/server/SRespawnPacket.java.patch | 10 - .../gen/IWorldGenerationBaseReader.java.patch | 8 - 12 files changed, 234 insertions(+), 61 deletions(-) create mode 100644 buildSrc/.gitignore create mode 100644 buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy delete mode 100644 patches/minecraft/net/minecraft/network/play/server/SJoinGamePacket.java.patch delete mode 100644 patches/minecraft/net/minecraft/network/play/server/SRespawnPacket.java.patch delete mode 100644 patches/minecraft/net/minecraft/world/gen/IWorldGenerationBaseReader.java.patch diff --git a/.gitignore b/.gitignore index 68c45dd69..86327bac2 100644 --- a/.gitignore +++ b/.gitignore @@ -27,7 +27,6 @@ /projects/**/run/ /projects/**/*.launch /repo/ -/buildSrc src/generated/resources/.cache/ #Patch rejects diff --git a/build.gradle b/build.gradle index 3efd96642..7d3497f6a 100644 --- a/build.gradle +++ b/build.gradle @@ -29,6 +29,7 @@ import java.util.zip.ZipInputStream import java.util.zip.ZipOutputStream import java.security.MessageDigest import java.net.URL +import net.minecraftforge.forge.tasks.* import net.minecraftforge.gradle.common.task.ArchiveChecksum import net.minecraftforge.gradle.common.task.DownloadMavenArtifact import net.minecraftforge.gradle.common.task.ExtractInheritance @@ -138,7 +139,7 @@ project(':forge') { apply plugin: 'net.minecraftforge.gradle.patcher' apply plugin: 'net.minecrell.licenser' apply plugin: 'de.undercouch.download' - + compileJava.sourceCompatibility = compileJava.targetCompatibility = sourceCompatibility = targetCompatibility = '1.8' // Need this here so eclipse task generates correctly. group = 'net.minecraftforge' @@ -892,6 +893,17 @@ project(':forge') { } } + task checkPatchesAndFix(type: CheckPatches) { + patchDir = file("$rootDir/patches") + autoFix = true + } + + task checkPatches(type: CheckPatches) { + patchDir = file("$rootDir/patches") + autoFix = false + } + project(':forge').getTasks().getByName('genPatches').finalizedBy(checkPatches) + task launcherJson(dependsOn: ['signUniversalJar', 'signLauncherJar']) { inputs.file universalJar.archivePath inputs.file { launcherJar.archivePath } diff --git a/buildSrc/.gitignore b/buildSrc/.gitignore new file mode 100644 index 000000000..abc78ba75 --- /dev/null +++ b/buildSrc/.gitignore @@ -0,0 +1,2 @@ +/.gradle/ +/build/ diff --git a/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy new file mode 100644 index 000000000..31ff7995f --- /dev/null +++ b/buildSrc/src/main/groovy/net/minecraftforge/forge/tasks/CheckPatches.groovy @@ -0,0 +1,213 @@ +package net.minecraftforge.forge.tasks + +import org.gradle.api.DefaultTask +import org.gradle.api.tasks.Input +import org.gradle.api.tasks.InputDirectory +import org.gradle.api.tasks.TaskAction + +import java.nio.file.Files +import java.util.regex.Pattern + +public class CheckPatches extends DefaultTask { + @InputDirectory File patchDir + @Input boolean autoFix = false + + @TaskAction + protected void exec() { + def hasS2SArtifact = [ + "patches/minecraft/net/minecraft/client/renderer/ViewFrustum.java.patch", + "patches/minecraft/net/minecraft/data/BlockModelDefinition.java.patch" + ] + + def verified = true; + project.fileTree(patchDir).each { patch -> + def patchPath = project.rootDir.toPath().relativize(patch.toPath()).toString() + verified &= verifyPatch(patch, autoFix, patchPath, hasS2SArtifact.contains(patchPath)) + } + + if (!verified) + throw new RuntimeException('One or more patches failed verification. Check the log for errors.') + } + + def verifyPatch(patch, fix, patchPath, hasS2SArtifact) { + def hunk_start_pattern = Pattern.compile('^@@ -[0-9,]* \\+[0-9,]* @@$') + def white_space_pattern = Pattern.compile('^[+\\-]\\s*$') + def import_pattern = Pattern.compile('^[+\\-]\\s*import.*') + def field_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^=;]*)(=.*)?;\\s*$') + def method_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final)?([^(]*)[(]([^)]*)?[)]\\s*[{]\\s*$') + def class_pattern = Pattern.compile('^[+\\-][\\s]*((public|protected|private)[\\s]*)?(static[\\s]*)?(final[\\s]*)?(class|interface)([^{]*)[{]\\s*$') + + def accessMap = [("private"):0, (null):1, ("protected"):2, ("public"):3] + + def hasProblem = false; + def lines = patch.readLines() + + def hunksStart = 0 + def onlyWhiteSpace = false + + def didFix = false + def newLines = [] + def whiteSpaceErrors = [] + + // First two lines are file name ++/-- and we do not care + newLines.add(lines[0] + '\n') + newLines.add(lines[1] + '\n') + + int i = 2 + for (; i < lines.size(); ++i) { + def line = lines[i] + newLines.add(line + '\n') + + if (hunk_start_pattern.matcher(line).find()) { + if (onlyWhiteSpace) { + if (fix || hasS2SArtifact) { + logger.lifecycle("Removing white space hunk starting at {}, file: {}", hunksStart + 1, patchPath) + def toRemove = i - hunksStart; + while (toRemove-- > 0) + newLines.remove(newLines.size() - 1) + didFix = true + } + else { + logger.lifecycle("Patch contains only white space hunk starting at {}, file: {}", hunksStart + 1, patchPath) + hasProblem = true + } + } + else { + whiteSpaceErrors.each { logger.lifecycle(it) } + whiteSpaceErrors.clear() + } + hunksStart = i + onlyWhiteSpace = true + continue + } + + if (line.startsWithAny('+','-')) { + def prefixChange = false + def prevLine = lines[i - 1] + + if (line.charAt(0) == (char)'+' && prevLine.charAt(0) == (char)'-') { + def prevTrim = prevLine.substring(1).replaceAll("\\s", "") + def currTrim = line.substring(1).replaceAll("\\s", "") + + if (prevTrim.equals(currTrim)) { + prefixChange = true + } + + def pMatcher = field_pattern.matcher(prevLine) + def cMatcher = field_pattern.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // = ... + pMatcher.group(5) == cMatcher.group(5) && // field name + pMatcher.group(3) == cMatcher.group(3) && // static + (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + logger.lifecycle("Patch contains access changes or final removal at {}, file: {}", i + 1, patchPath) + hasProblem = true + } + + pMatcher = method_pattern.matcher(prevLine) + cMatcher = method_pattern.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // params + pMatcher.group(5) == cMatcher.group(5) && // void name + pMatcher.group(3) == cMatcher.group(3) && // static + (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + logger.lifecycle("Patch contains access changes or final removal at {}, file: {}", i + 1, patchPath) + hasProblem = true + } + + pMatcher = class_pattern.matcher(prevLine) + cMatcher = class_pattern.matcher(line) + + if (pMatcher.find() && cMatcher.find() && + pMatcher.group(6) == cMatcher.group(6) && // ClassName<> extends ... + pMatcher.group(5) == cMatcher.group(5) && // class | interface + pMatcher.group(3) == cMatcher.group(3) && // static + (accessMap[pMatcher.group(2)] < accessMap[cMatcher.group(2)] || pMatcher.group(4) != cMatcher.group(4))) { + logger.lifecycle("Patch contains access changes or final removal at {}, file: {}", i + 1, patchPath) + hasProblem = true + } + } + + if (line.charAt(0) == (char)'-' && i + 1 < lines.size()) { + def nextLine = lines[i + 1] + if (nextLine.charAt(0) == (char)'+') { + def nextTrim = nextLine.substring(1).replaceAll("\\s", "") + def currTrim = line.substring(1).replaceAll("\\s", "") + + if (nextTrim.equals(currTrim)) { + prefixChange = true + } + } + } + + def isWhiteSpaceChange = white_space_pattern.matcher(line).find() + + if (!prefixChange && !isWhiteSpaceChange) { + onlyWhiteSpace = hasS2SArtifact && import_pattern.matcher(line).find() + } + else if (isWhiteSpaceChange) { + def prevLineChange = prevLine.startsWithAny('+','-') + def nextLineChange = i + 1 < lines.size() && lines[i + 1].startsWithAny('+','-') + + if (!prevLineChange && !nextLineChange) { + whiteSpaceErrors.add(String.format("Patch contains white space change in valid hunk at %d (cannot auto fix), file: %s", i + 1, patchPath)) + } + } + + if (line.contains("\t")) { + if (!fix) { + logger.lifecycle("Patch contains tabs on line {}, file: {}", i + 1, patchPath) + hasProblem = true + } + else { + logger.lifecycle("Fixing tabs on line {}, file: {}", i + 1, patchPath) + line = line.replaceAll('\t', ' ') + newLines.remove(newLines.size() - 1) + newLines.add(line + '\n') + didFix = true + } + } + + if (import_pattern.matcher(line).find() && !hasS2SArtifact) { + logger.lifecycle("Patch contains import change on line {}, file: {}", i + 1, patchPath) + hasProblem = true + } + } + } + + if (onlyWhiteSpace) { + if (fix || hasS2SArtifact) { + logger.lifecycle("Removing white space hunk starting at {}, file: {}", hunksStart + 1, patchPath) + def toRemove = i - hunksStart; + while (toRemove-- > 0) + newLines.remove(newLines.size() - 1) + didFix = true + } + else { + logger.lifecycle("Patch contains only white space hunk starting at {}, file: {}", hunksStart + 1, patchPath) + hasProblem = true + } + } + else { + whiteSpaceErrors.each { error -> logger.lifecycle(error) } + } + + if (didFix) { + if (newLines.size() <= 2) { + logger.lifecycle("Patch is now empty removing, file: {}", patchPath) + Files.delete(patch.toPath()) + } + else { + if (!hasS2SArtifact) + logger.lifecycle("*** Updating patch file. Please run setup then genPatches again. ***") + patch.withWriter('UTF-8') { writer -> + newLines.each { l -> writer.write(l) } + } + } + } + + return !hasProblem + } +} diff --git a/patches/minecraft/net/minecraft/client/renderer/FirstPersonRenderer.java.patch b/patches/minecraft/net/minecraft/client/renderer/FirstPersonRenderer.java.patch index 298538ff9..10ee29516 100644 --- a/patches/minecraft/net/minecraft/client/renderer/FirstPersonRenderer.java.patch +++ b/patches/minecraft/net/minecraft/client/renderer/FirstPersonRenderer.java.patch @@ -81,10 +81,3 @@ } if (this.field_187469_f < 0.1F) { -@@ -430,6 +440,5 @@ - } else { - this.field_187471_h = 0.0F; - } -- - } - } diff --git a/patches/minecraft/net/minecraft/client/renderer/FogRenderer.java.patch b/patches/minecraft/net/minecraft/client/renderer/FogRenderer.java.patch index 73db7dc3d..1b5fc5099 100644 --- a/patches/minecraft/net/minecraft/client/renderer/FogRenderer.java.patch +++ b/patches/minecraft/net/minecraft/client/renderer/FogRenderer.java.patch @@ -49,13 +49,11 @@ if (fluidstate.func_206884_a(FluidTags.field_206959_a)) { float f = 1.0F; f = 0.05F; -@@ -231,8 +250,8 @@ +@@ -231,6 +250,7 @@ RenderSystem.fogEnd(f3); RenderSystem.fogMode(GlStateManager.FogMode.LINEAR); RenderSystem.setupNvFogDistance(); + net.minecraftforge.client.ForgeHooksClient.onFogRender(p_228372_1_, p_228372_0_, partialTicks, f3); } -- - } - public static void func_228373_b_() { + } diff --git a/patches/minecraft/net/minecraft/client/renderer/entity/EntityRendererManager.java.patch b/patches/minecraft/net/minecraft/client/renderer/entity/EntityRendererManager.java.patch index 81f0fdcfa..a316bf42b 100644 --- a/patches/minecraft/net/minecraft/client/renderer/entity/EntityRendererManager.java.patch +++ b/patches/minecraft/net/minecraft/client/renderer/entity/EntityRendererManager.java.patch @@ -1,6 +1,6 @@ --- a/net/minecraft/client/renderer/entity/EntityRendererManager.java +++ b/net/minecraft/client/renderer/entity/EntityRendererManager.java -@@ -187,13 +187,15 @@ +@@ -187,7 +187,10 @@ this.field_178637_m = new PlayerRenderer(this); this.field_178636_l.put("default", this.field_178637_m); this.field_178636_l.put("slim", new PlayerRenderer(this, true)); @@ -11,13 +11,7 @@ for(EntityType entitytype : Registry.field_212629_r) { if (entitytype != EntityType.field_200729_aH && !this.field_78729_o.containsKey(entitytype)) { throw new IllegalStateException("No renderer registered for " + Registry.field_212629_r.func_177774_c(entitytype)); - } - } -- - } - - public EntityRenderer func_78713_a(T p_78713_1_) { -@@ -449,4 +451,8 @@ +@@ -449,4 +452,8 @@ public FontRenderer func_78716_a() { return this.field_78736_p; } diff --git a/patches/minecraft/net/minecraft/entity/monster/ShulkerEntity.java.patch b/patches/minecraft/net/minecraft/entity/monster/ShulkerEntity.java.patch index 1071e36f9..69253abc9 100644 --- a/patches/minecraft/net/minecraft/entity/monster/ShulkerEntity.java.patch +++ b/patches/minecraft/net/minecraft/entity/monster/ShulkerEntity.java.patch @@ -25,11 +25,9 @@ Optional optional1 = Optional.of(new BlockPos(p_70107_1_, p_70107_3_, p_70107_5_)); if (!optional1.equals(optional)) { this.field_70180_af.func_187227_b(field_184701_b, optional1); -@@ -282,7 +284,14 @@ - BlockPos blockpos1 = blockpos.func_177982_a(8 - this.field_70146_Z.nextInt(17), 8 - this.field_70146_Z.nextInt(17), 8 - this.field_70146_Z.nextInt(17)); +@@ -283,6 +285,12 @@ if (blockpos1.func_177956_o() > 0 && this.field_70170_p.func_175623_d(blockpos1) && this.field_70170_p.func_175723_af().func_177746_a(blockpos1) && this.field_70170_p.func_226665_a__(this, new AxisAlignedBB(blockpos1))) { Direction direction = this.func_234299_g_(blockpos1); -+ if (direction != null) { + net.minecraftforge.event.entity.living.EnderTeleportEvent event = new net.minecraftforge.event.entity.living.EnderTeleportEvent(this, blockpos1.func_177958_n(), blockpos1.func_177956_o(), blockpos1.func_177952_p(), 0); + if (net.minecraftforge.common.MinecraftForge.EVENT_BUS.post(event)) direction = null; @@ -40,11 +38,3 @@ this.field_70180_af.func_187227_b(field_184700_a, direction); this.func_184185_a(SoundEvents.field_187791_eX, 1.0F, 1.0F); this.field_70180_af.func_187227_b(field_184701_b, Optional.of(blockpos1)); -@@ -306,7 +315,6 @@ - this.field_70760_ar = 0.0F; - this.field_70761_aq = 0.0F; - } -- - } - - public void func_184206_a(DataParameter p_184206_1_) { diff --git a/patches/minecraft/net/minecraft/network/play/client/CClientSettingsPacket.java.patch b/patches/minecraft/net/minecraft/network/play/client/CClientSettingsPacket.java.patch index 24535c2a5..2a2ad07b6 100644 --- a/patches/minecraft/net/minecraft/network/play/client/CClientSettingsPacket.java.patch +++ b/patches/minecraft/net/minecraft/network/play/client/CClientSettingsPacket.java.patch @@ -6,6 +6,6 @@ } + + public String getLanguage() { -+ return this.field_149530_a; ++ return this.field_149530_a; + } } diff --git a/patches/minecraft/net/minecraft/network/play/server/SJoinGamePacket.java.patch b/patches/minecraft/net/minecraft/network/play/server/SJoinGamePacket.java.patch deleted file mode 100644 index d3a8aa966..000000000 --- a/patches/minecraft/net/minecraft/network/play/server/SJoinGamePacket.java.patch +++ /dev/null @@ -1,10 +0,0 @@ ---- a/net/minecraft/network/play/server/SJoinGamePacket.java -+++ b/net/minecraft/network/play/server/SJoinGamePacket.java -@@ -21,6 +21,7 @@ - private boolean field_149204_b; - private GameType field_149205_c; - private GameType field_241785_e_; -+ private int dimensionInt; - private Set> field_240811_e_; - private IDynamicRegistries.Impl field_240812_f_; - private RegistryKey field_240813_g_; diff --git a/patches/minecraft/net/minecraft/network/play/server/SRespawnPacket.java.patch b/patches/minecraft/net/minecraft/network/play/server/SRespawnPacket.java.patch deleted file mode 100644 index 770416d5f..000000000 --- a/patches/minecraft/net/minecraft/network/play/server/SRespawnPacket.java.patch +++ /dev/null @@ -1,10 +0,0 @@ ---- a/net/minecraft/network/play/server/SRespawnPacket.java -+++ b/net/minecraft/network/play/server/SRespawnPacket.java -@@ -15,6 +15,7 @@ - public class SRespawnPacket implements IPacket { - private RegistryKey field_240822_a_; - private RegistryKey field_149088_a; -+ private int dimensionInt; - private long field_229746_b_; - private GameType field_149087_c; - private GameType field_241787_e_; diff --git a/patches/minecraft/net/minecraft/world/gen/IWorldGenerationBaseReader.java.patch b/patches/minecraft/net/minecraft/world/gen/IWorldGenerationBaseReader.java.patch deleted file mode 100644 index 1be61e104..000000000 --- a/patches/minecraft/net/minecraft/world/gen/IWorldGenerationBaseReader.java.patch +++ /dev/null @@ -1,8 +0,0 @@ ---- a/net/minecraft/world/gen/IWorldGenerationBaseReader.java -+++ b/net/minecraft/world/gen/IWorldGenerationBaseReader.java -@@ -8,4 +8,5 @@ - boolean func_217375_a(BlockPos p_217375_1_, Predicate p_217375_2_); - - BlockPos func_205770_a(Heightmap.Type p_205770_1_, BlockPos p_205770_2_); -+ - }