Modify the method used for a ZipEntry when changing compression level.
When writing a zip entry that's stored, we must pre-calculate the sizes and CRC in order
for Other Zip Consuming Tools to be happy.
Test Plan:
`buck test --all`
Build prod apps and verify that the `RepackZipStep` was actually effective.
diff --git a/src/com/facebook/buck/zip/CustomZipEntry.java b/src/com/facebook/buck/zip/CustomZipEntry.java
index cf324dc..062badb 100644
--- a/src/com/facebook/buck/zip/CustomZipEntry.java
+++ b/src/com/facebook/buck/zip/CustomZipEntry.java
@@ -30,16 +30,32 @@
public CustomZipEntry(ZipEntry other) {
super(other);
+ setDefaultMethodIfMethodUnset();
}
public CustomZipEntry(String name) {
super(name);
+ setDefaultMethodIfMethodUnset();
+ }
+
+ private void setDefaultMethodIfMethodUnset() {
+ if (getMethod() == -1) {
+ setMethod(DEFLATED);
+ }
}
public void setCompressionLevel(int compressionLevel) {
Preconditions.checkArgument(
compressionLevel >= NO_COMPRESSION && compressionLevel <= BEST_COMPRESSION);
this.compressionLevel = compressionLevel;
+
+ // We need to update the underlying method declared
+ setMethod(compressionLevel == NO_COMPRESSION ? STORED : DEFLATED);
+
+ // Reset the various fields that need to be updated.
+ setCrc(0);
+ setSize(0);
+ setCompressedSize(0);
}
public int getCompressionLevel() {
diff --git a/src/com/facebook/buck/zip/EntryAccounting.java b/src/com/facebook/buck/zip/EntryAccounting.java
index 16ddd5b..81bcc53 100644
--- a/src/com/facebook/buck/zip/EntryAccounting.java
+++ b/src/com/facebook/buck/zip/EntryAccounting.java
@@ -76,10 +76,6 @@
entry.setTime(clock.currentTimeMillis());
}
- if (method == Method.DEFLATE) {
- flags |= DATA_DESCRIPTOR_FLAG;
- }
-
if (entry instanceof CustomZipEntry) {
deflater.setLevel(((CustomZipEntry) entry).getCompressionLevel());
}
@@ -160,6 +156,35 @@
}
public long writeLocalFileHeader(OutputStream out) throws IOException {
+ if (method == Method.DEFLATE) {
+ flags |= DATA_DESCRIPTOR_FLAG;
+
+ // See http://www.pkware.com/documents/casestudies/APPNOTE.TXT (section 4.4.4)
+ // Essentially, we're about to set bits 1 and 2 to indicate to tools such as zipinfo which
+ // level of compression we're using. If we've not set a compression level, then we're using
+ // the default one, which is right. It turns out. For your viewing pleasure:
+ //
+ // +----------+-------+-------+
+ // | Level | Bit 1 | Bit 2 |
+ // +----------+-------+-------+
+ // | Fastest | 0 | 1 |
+ // | Normal | 0 | 0 |
+ // | Best | 1 | 0 |
+ // +----------+-------+-------+
+ if (entry instanceof CustomZipEntry) {
+ int level = ((CustomZipEntry) entry).getCompressionLevel();
+ switch (level) {
+ case Deflater.BEST_COMPRESSION:
+ flags |= (1 << 1);
+ break;
+
+ case Deflater.BEST_SPEED:
+ flags |= (1 << 2);
+ break;
+ }
+ }
+ }
+
try (ByteArrayOutputStream stream = new ByteArrayOutputStream()) {
ByteIo.writeInt(stream, ZipEntry.LOCSIG);
diff --git a/src/com/facebook/buck/zip/RepackZipEntriesStep.java b/src/com/facebook/buck/zip/RepackZipEntriesStep.java
index eb9850d..06e9bc6 100644
--- a/src/com/facebook/buck/zip/RepackZipEntriesStep.java
+++ b/src/com/facebook/buck/zip/RepackZipEntriesStep.java
@@ -21,12 +21,16 @@
import com.facebook.buck.step.Step;
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSet;
+import com.google.common.hash.Hashing;
import com.google.common.io.ByteStreams;
import java.io.BufferedInputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.logging.Level;
@@ -89,8 +93,24 @@
if (entries.contains(customEntry.getName())) {
customEntry.setCompressionLevel(compressionLevel);
}
+
+ InputStream toUse;
+ // If we're using STORED files, we must pre-calculate the CRC.
+ if (customEntry.getMethod() == ZipEntry.STORED) {
+ try (ByteArrayOutputStream bos = new ByteArrayOutputStream()) {
+ ByteStreams.copy(in, bos);
+ byte[] bytes = bos.toByteArray();
+ customEntry.setCrc(Hashing.crc32().hashBytes(bytes).padToLong());
+ customEntry.setSize(bytes.length);
+ customEntry.setCompressedSize(bytes.length);
+ toUse = new ByteArrayInputStream(bytes);
+ }
+ } else {
+ toUse = in;
+ }
+
out.putNextEntry(customEntry);
- ByteStreams.copy(in, out);
+ ByteStreams.copy(toUse, out);
out.closeEntry();
}
diff --git a/test/com/facebook/buck/zip/ZipOutputStreamTest.java b/test/com/facebook/buck/zip/ZipOutputStreamTest.java
index 06453d1..aaad0a3 100644
--- a/test/com/facebook/buck/zip/ZipOutputStreamTest.java
+++ b/test/com/facebook/buck/zip/ZipOutputStreamTest.java
@@ -49,6 +49,7 @@
import java.util.Calendar;
import java.util.Date;
import java.util.List;
+import java.util.zip.Deflater;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;
import java.util.zip.ZipInputStream;
@@ -343,8 +344,12 @@
entry = new CustomZipEntry("stored");
entry.setCompressionLevel(NO_COMPRESSION);
+ byte[] bytes = "stored".getBytes();
+ entry.setSize(bytes.length);
+ entry.setCrc(Hashing.crc32().hashBytes(bytes).padToLong());
out.putNextEntry(entry);
- out.write("stored".getBytes());
+
+ out.write(bytes);
entry = new CustomZipEntry("best");
entry.setCompressionLevel(BEST_COMPRESSION);
@@ -360,13 +365,28 @@
assertNotEquals(entry.getCompressedSize(), entry.getSize());
entry = in.getNextEntry();
+ ByteStreams.copy(in, ByteStreams.nullOutputStream());
assertEquals("stored", entry.getName());
assertEquals(entry.getCompressedSize(), entry.getSize());
entry = in.getNextEntry();
+ ByteStreams.copy(in, ByteStreams.nullOutputStream());
assertEquals("best", entry.getName());
ByteStreams.copy(in, ByteStreams.nullOutputStream());
assertThat(entry.getCompressedSize(), lessThan(defaultCompressedSize));
}
}
+
+ @Test
+ public void shouldChangeMethodWhenCompressionLevelIsChanged() {
+ CustomZipEntry entry = new CustomZipEntry("cake");
+ assertEquals(ZipEntry.DEFLATED, entry.getMethod());
+ assertEquals(Deflater.DEFAULT_COMPRESSION, entry.getCompressionLevel());
+
+ entry.setCompressionLevel(NO_COMPRESSION);
+ assertEquals(ZipEntry.STORED, entry.getMethod());
+
+ entry.setCompressionLevel(BEST_COMPRESSION);
+ assertEquals(ZipEntry.DEFLATED, entry.getMethod());
+ }
}
