try to make ZipExtraField change in zip backwards compatible for subclasses that override zipFile. PR 48541

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@900082 13f79535-47bb-0310-9956-ffa450edef68

WHATSNEW

@@ -9,9 +9,14 @@ Changes that could break older environments:
 Fixed bugs:
 -----------
 
- * <fixcrlf> now tries earlier to delete the created temporary files.
+ * <fixcrlf> now tries to delete the created temporary files earlier.
    Bugzilla Report 48506.
 
+ * the implementation of <zip> had been changed in a way that broke
+   the jarjar links task and protentially other third-party subclasses
+   as well.
+   Bugzilla Report 48541.
+
 Other changes:
 --------------
 

build.xml

@@ -1825,6 +1825,8 @@ see ${build.junit.reports} / ${antunit.reports}
             unless="tests.and.ant.share.classloader"/>
           <exclude name="${ant.package}/DefaultLoggerTest.java"
             unless="tests.and.ant.share.classloader"/>
+          <exclude name="${taskdefs.package}/ZipExtraFieldTest.java"
+            unless="tests.and.ant.share.classloader"/>
 
           <!-- can only run if cvs is installed on your machine
                enable by setting the property have.cvs

src/main/org/apache/tools/ant/taskdefs/Jar.java

@@ -473,7 +473,7 @@ public class Jar extends Zip {
                super.zipFile(is, zOut,
                              "META-INF/services/" + service.getType(),
                              System.currentTimeMillis(), null,
-                             ZipFileSet.DEFAULT_FILE_MODE, null);
+                             ZipFileSet.DEFAULT_FILE_MODE);
            } finally {
                // technically this is unnecessary since
                // Service.getAsStream returns a ByteArrayInputStream
@@ -581,7 +581,7 @@ public class Jar extends Zip {
         try {
             super.zipFile(bais, zOut, MANIFEST_NAME,
                           System.currentTimeMillis(), null,
-                          ZipFileSet.DEFAULT_FILE_MODE, null);
+                          ZipFileSet.DEFAULT_FILE_MODE);
         } finally {
             // not really required
             FileUtils.close(bais);
@@ -669,7 +669,7 @@ public class Jar extends Zip {
             new ByteArrayInputStream(baos.toByteArray());
         try {
             super.zipFile(bais, zOut, INDEX_NAME, System.currentTimeMillis(),
-                          null, ZipFileSet.DEFAULT_FILE_MODE, null);
+                          null, ZipFileSet.DEFAULT_FILE_MODE);
         } finally {
             // not really required
             FileUtils.close(bais);
@@ -689,8 +689,7 @@ public class Jar extends Zip {
      * @throws IOException on error
      */
     protected void zipFile(InputStream is, ZipOutputStream zOut, String vPath,
-                           long lastModified, File fromArchive, int mode,
-                           ZipExtraField[] extra)
+                           long lastModified, File fromArchive, int mode)
         throws IOException {
         if (MANIFEST_NAME.equalsIgnoreCase(vPath))  {
             if (isFirstPass()) {
@@ -705,8 +704,7 @@ public class Jar extends Zip {
             if (index && vPath.indexOf("/") == -1) {
                 rootEntries.addElement(vPath);
             }
-            super.zipFile(is, zOut, vPath, lastModified, fromArchive, mode,
-                          extra);
+            super.zipFile(is, zOut, vPath, lastModified, fromArchive, mode);
         }
     }
 

src/main/org/apache/tools/ant/taskdefs/Zip.java

@@ -1674,25 +1674,33 @@ public class Zip extends MatchingTask {
         }
     }
 
+    /*
+     * This is a hacky construct to extend the zipFile method to
+     * support a new parameter (extra fields to preserve) without
+     * breaking subclasses that override the old method signature.
+     */
+    private static ThreadLocal currentZipExtra = new ThreadLocal() {
+            protected Object initialValue() {
+                return null;
+            }
+        };
+
     /**
-     * Adds a new entry to the archive, takes care of duplicates as well.
-     *
-     * @param in the stream to read data for the entry from.  The
-     * caller of the method is responsible for closing the stream.
-     * @param zOut the stream to write to.
-     * @param vPath the name this entry shall have in the archive.
-     * @param lastModified last modification time for the entry.
-     * @param fromArchive the original archive we are copying this
-     * entry from, will be null if we are not copying from an archive.
-     * @param mode the Unix permissions to set.
-     *
-     * @since Ant 1.5.2
-     * @throws IOException on error
+     * Provides the extra fields for the zip entry currently being
+     * added to the archive - if any.
+     * @since Ant 1.8.0
      */
-    protected void zipFile(InputStream in, ZipOutputStream zOut, String vPath,
-                           long lastModified, File fromArchive, int mode)
-        throws IOException {
-        zipFile(in, zOut, vPath, lastModified, fromArchive, mode, null);
+    protected final ZipExtraField[] getCurrentExtraFields() {
+        return (ZipExtraField[]) currentZipExtra.get();
+    }
+
+    /**
+     * Sets the extra fields for the zip entry currently being
+     * added to the archive - if any.
+     * @since Ant 1.8.0
+     */
+    protected final void setCurrentExtraFields(ZipExtraField[] extra) {
+        currentZipExtra.set(extra);
     }
 
     /**
@@ -1706,16 +1714,13 @@ public class Zip extends MatchingTask {
      * @param fromArchive the original archive we are copying this
      * entry from, will be null if we are not copying from an archive.
      * @param mode the Unix permissions to set.
-     * @param extra ZipExtraFields to add
      *
-     * @since Ant 1.8.0
+     * @since Ant 1.5.2
      * @throws IOException on error
      */
     protected void zipFile(InputStream in, ZipOutputStream zOut, String vPath,
-                           long lastModified, File fromArchive,
-                           int mode, ZipExtraField[] extra)
+                           long lastModified, File fromArchive, int mode)
         throws IOException {
-
         // fromArchive is used in subclasses overriding this method
 
         if (entries.contains(vPath)) {
@@ -1784,6 +1789,7 @@ public class Zip extends MatchingTask {
             }
 
             ze.setUnixMode(mode);
+            ZipExtraField[] extra = getCurrentExtraFields();
             if (extra != null) {
                 ze.setExtraFields(extra);
             }
@@ -1802,6 +1808,34 @@ public class Zip extends MatchingTask {
         addedFiles.addElement(vPath);
     }
 
+    /**
+     * Adds a new entry to the archive, takes care of duplicates as well.
+     *
+     * @param in the stream to read data for the entry from.  The
+     * caller of the method is responsible for closing the stream.
+     * @param zOut the stream to write to.
+     * @param vPath the name this entry shall have in the archive.
+     * @param lastModified last modification time for the entry.
+     * @param fromArchive the original archive we are copying this
+     * entry from, will be null if we are not copying from an archive.
+     * @param mode the Unix permissions to set.
+     * @param extra ZipExtraFields to add
+     *
+     * @since Ant 1.8.0
+     * @throws IOException on error
+     */
+    protected void zipFile(InputStream in, ZipOutputStream zOut, String vPath,
+                           long lastModified, File fromArchive,
+                           int mode, ZipExtraField[] extra)
+        throws IOException {
+        try {
+            setCurrentExtraFields(extra);
+            zipFile(in, zOut, vPath, lastModified, fromArchive, mode);
+        } finally {
+            setCurrentExtraFields(null);
+        }
+    }
+
     /**
      * Method that gets called when adding from <code>java.io.File</code> instances.
      *

src/tests/junit/org/apache/tools/ant/taskdefs/ZipExtraFieldTest.java

@@ -0,0 +1,92 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package org.apache.tools.ant.taskdefs;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.InputStream;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Iterator;
+
+import junit.framework.TestCase;
+
+import org.apache.tools.ant.types.ResourceCollection;
+import org.apache.tools.ant.types.resources.ZipResource;
+import org.apache.tools.zip.JarMarker;
+import org.apache.tools.zip.ZipEntry;
+import org.apache.tools.zip.ZipExtraField;
+import org.apache.tools.zip.ZipFile;
+
+public class ZipExtraFieldTest extends TestCase {
+
+    public void testPreservesExtraFields() throws IOException {
+        File f = File.createTempFile("ziptest", ".zip");
+        f.delete();
+        ZipFile zf = null;
+        try {
+            Zip testInstance = new Zip();
+            testInstance.setDestFile(f);
+            final ZipResource r = new ZipResource() {
+                    public String getName() {
+                        return "x";
+                    }
+                    public boolean isExists() {
+                        return true;
+                    }
+                    public boolean isDirectory() {
+                        return false;
+                    }
+                    public long getLastModified() {
+                        return 1;
+                    }
+                    public InputStream getInputStream() {
+                        return new ByteArrayInputStream(new byte[0]);
+                    }
+                    public ZipExtraField[] getExtraFields() {
+                        return new ZipExtraField[] {
+                            new JarMarker()
+                        };
+                    }
+                };
+            testInstance.add(new ResourceCollection() {
+                    public boolean isFilesystemOnly() { return false; }
+                    public int size() { return 1; }
+                    public Iterator iterator() {
+                        ArrayList l = new ArrayList();
+                        l.add(r);
+                        return l.iterator();
+                    }
+                });
+            testInstance.execute();
+
+            zf = new ZipFile(f);
+            ZipEntry ze = zf.getEntry("x");
+            assertNotNull(ze);
+            assertEquals(1, ze.getExtraFields().length);
+            assertTrue(ze.getExtraFields()[0] instanceof JarMarker);
+        } finally {
+            ZipFile.closeQuietly(zf);
+            if (f.exists()) {
+                f.delete();
+            }
+        }
+    }
+
+}