From 374360c118e2a7373ee2fa5be7d1b784240bb1aa Mon Sep 17 00:00:00 2001 From: Tobias Schulte Date: Thu, 31 Aug 2017 17:15:59 +0200 Subject: [PATCH] fix(jardiff): handle cases where jardiff can not be packed pack200 is a lossy compression algorithm. Sometimes packing a jardiff alters the class files. This was not handled correctly. Now in that case the fallback is to only gzip the jardiff (using `pack200 --effort=0`). Closes #48 --- .../gradle/jnlp/war/JarDiffTask.groovy | 39 +++++++++----- .../GradleJnlpWarPluginIntegrationSpec.groovy | 53 +++++++++++++++++-- 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/gradle-jnlp-plugin/src/main/groovy/de/gliderpilot/gradle/jnlp/war/JarDiffTask.groovy b/gradle-jnlp-plugin/src/main/groovy/de/gliderpilot/gradle/jnlp/war/JarDiffTask.groovy index c0d86cb..151d1fe 100644 --- a/gradle-jnlp-plugin/src/main/groovy/de/gliderpilot/gradle/jnlp/war/JarDiffTask.groovy +++ b/gradle-jnlp-plugin/src/main/groovy/de/gliderpilot/gradle/jnlp/war/JarDiffTask.groovy @@ -93,24 +93,32 @@ class JarDiffTask extends DefaultTask { File diffJarPacked = new File("${diffJar}.pack.gz") logger.info("packing $diffJar.name with pack200") try { - JavaHomeAware.exec(project, "pack200", "--repack", diffJar) - JavaHomeAware.exec(project, "pack200", diffJarPacked, diffJar) - project.delete(diffJar) - if (diffJarPacked.size() >= newVersion.file.size()) { - // only keep jardiff.pack.gz if smaller than the full file - logger.info("jardiff.pack.gz $diffJarPacked.name is not smaller than $newFile.name") - project.delete(diffJarPacked) - } - return + // do the repack on a tmp file, because this might go wrong and we are stuck with a scrambled file + File tmpJar = tmpFile(diffJar) + // first repack + JavaHomeAware.exec(project, "pack200", "--repack", tmpJar) + // because the repacking might have altered the class files, resulting in a security exception + // when packing again + JavaHomeAware.exec(project, "pack200", diffJarPacked, tmpJar) } catch (e) { + // there was most probably a security exceptionls // the file may be created and have size 0 -- delete project.delete(diffJarPacked) - logger.warn("failed to pack $diffJar.name", e) + logger.info("failed to pack $diffJar.name using default params -- retrying with param --effort=0") + // use the unaltered diffJar as source, using --effort=0 is save, because it does not alter the class files + JavaHomeAware.exec(project, "pack200", "--effort=0", diffJarPacked, diffJar) } + project.delete(diffJar) + if (diffJarPacked.size() >= newVersion.file.size()) { + // only keep jardiff.pack.gz if smaller than the full file + logger.info("$diffJarPacked.name is not smaller than $newVersion.file.name") + project.delete(diffJarPacked) + } + return } if (diffJar.size() >= newVersion.file.size()) { // only keep jardiff if smaller than the full file - logger.info("jardiff $diffJar.name is not smaller than $newFile.name") + logger.info("$diffJar.name is not smaller than $newVersion.file.name") project.delete(diffJar) } } @@ -118,9 +126,16 @@ class JarDiffTask extends DefaultTask { File getJar(File file) { if (file.name.endsWith('.jar')) return file - File jar = new File("$project.buildDir/tmp/jardiff", file.name - '.pack.gz') + File jar = new File(into.parentFile, file.name - '.pack.gz') JavaHomeAware.exec(project, "unpack200", file, jar) return jar } + File tmpFile(File file) { + project.copy { + from file + into into.parentFile + } + return new File(into.parentFile, file.name) + } } diff --git a/gradle-jnlp-plugin/src/test/groovy/de/gliderpilot/gradle/jnlp/war/GradleJnlpWarPluginIntegrationSpec.groovy b/gradle-jnlp-plugin/src/test/groovy/de/gliderpilot/gradle/jnlp/war/GradleJnlpWarPluginIntegrationSpec.groovy index 05232be..d3ddd53 100644 --- a/gradle-jnlp-plugin/src/test/groovy/de/gliderpilot/gradle/jnlp/war/GradleJnlpWarPluginIntegrationSpec.groovy +++ b/gradle-jnlp-plugin/src/test/groovy/de/gliderpilot/gradle/jnlp/war/GradleJnlpWarPluginIntegrationSpec.groovy @@ -327,7 +327,7 @@ class GradleJnlpWarPluginIntegrationSpec extends AbstractJnlpIntegrationSpec { } @Unroll - def '[gradle #gv] jardiffs are created'() { + def '[gradle #gv] jardiffs with pack200 are created'() { given: gradleVersion = gv @@ -348,12 +348,49 @@ class GradleJnlpWarPluginIntegrationSpec extends AbstractJnlpIntegrationSpec { } } ''' - runTasksSuccessfully("build") + def result = runTasksSuccessfully("build") then: + !result.standardOutput.contains("failed to pack") fileExists("war/build/libs/war-1.1.war") fileExists("war/build/tmp/warContent/launch.jnlp") fileExists("war/build/tmp/warContent/lib/${moduleName}__V1.0-myalias__V1.1-myalias.diff.jar.pack.gz") + // but no diff.jar + !fileExists("war/build/tmp/warContent/lib/${moduleName}__V1.0-myalias__V1.1-myalias.diff.jar") + + where: + gv << gradleVersions + } + + @Unroll + def '[gradle #gv] jardiffs without pack200 are created'() { + given: + gradleVersion = gv + + when: + buildFile << "jnlp.usePack200 = false\n" + runTasksSuccessfully('build', 'publish') + version = '1.1' + warBuildFile.text = '''\ + jnlpWar { + versions { + "1.0" "$rootProject.group:$rootProject.name:1.0:webstart@zip" + } + launchers { + "1.1" { + jardiff { + from "1.0" + } + } + } + } + ''' + runTasksSuccessfully("build") + + then: + fileExists("war/build/libs/war-1.1.war") + fileExists("war/build/tmp/warContent/launch.jnlp") + fileExists("war/build/tmp/warContent/lib/${moduleName}__V1.0-myalias__V1.1-myalias.diff.jar") where: gv << gradleVersions @@ -454,14 +491,20 @@ class GradleJnlpWarPluginIntegrationSpec extends AbstractJnlpIntegrationSpec { } } ''' - runTasksSuccessfully("build") + def result = runTasksSuccessfully("build") then: fileExists("war/build/libs/war-1.1.war") fileExists("war/build/tmp/warContent/launch.jnlp") - // somehow xalan does not work, no pack.gz is created + // somehow pack200 of the xalan jardiff does not work with default params + // does not even work with java 7 + System.getProperty("java.specification.version") == "1.7" && + result.standardOutput.contains("failed to create xalan__V2.7.1-myalias__V2.7.2-myalias.diff.jar") || + result.standardOutput.contains("failed to pack xalan__V2.7.1-myalias__V2.7.2-myalias.diff.jar using default params -- retrying with param --effort=0") && + // but the pack200.gz file using --effort=0 is not smaller than the new version .pack.gz + result.standardOutput.contains("xalan__V2.7.1-myalias__V2.7.2-myalias.diff.jar.pack.gz is not smaller than xalan__V2.7.2-myalias.jar.pack.gz") + // therefore we don't have the diff.jar.pack.gz nor the diff.jar !fileExists("war/build/tmp/warContent/lib/xalan__V2.7.1-myalias__V2.7.2-myalias.diff.jar.pack.gz") - // but the diff.jar is bigger than the v2.7.2.jar.pack.gz, therefore no diff !fileExists("war/build/tmp/warContent/lib/xalan__V2.7.1-myalias__V2.7.2-myalias.diff.jar") where: