Skip to content

Commit

Permalink
fix(jardiff): handle cases where jardiff can not be packed
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tschulte committed Aug 31, 2017
1 parent 7d1dd6f commit 374360c
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,34 +93,49 @@ 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)
}
}

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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 374360c

Please sign in to comment.