Bug 39655 - Manifest task does not merge Class-Path in update mode and overwrites the old contents
Summary: Manifest task does not merge Class-Path in update mode and overwrites the old...
Status: RESOLVED FIXED
Alias: None
Product: Ant
Classification: Unclassified
Component: Core tasks (show other bugs)
Version: 1.6.5
Hardware: All All
: P2 enhancement (vote)
Target Milestone: 1.8.0
Assignee: Ant Notifications List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2006-05-24 20:40 UTC by Vassilis Touloumtzoglou
Modified: 2009-08-20 06:52 UTC (History)
0 users



Attachments
main/org/apache/tools/ant/taskdefs/Manifest.java (2.16 KB, patch)
2006-05-24 20:43 UTC, Vassilis Touloumtzoglou
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vassilis Touloumtzoglou 2006-05-24 20:40:20 UTC
the mode="update" the manifest task is supposed to merge sections of manifest properties including 
the main section.
There seems to be some code to handle the Class-Path attribute separately but guess what, it doesn't 
work...

There's discussion that multiple Class-Path attributes should be supported but the utility of this feature 
is questionable. Ideally an attribute should be added to disambiguate the bahaviour of the Manifest 
task (something like multipleClasspathProps="true|false" should do).

In the meantime here's my fix for this:
--- main/org/apache/tools/ant/taskdefs/Manifest.java    2005-06-02 15:19:58.000000000 +0200
+++ /Volumes/vassilistouloumtzoglou/dev/workspace/apache-ant/src/main/org/apache/tools/ant/
taskdefs/Manifest.java       2006-05-15 21:08:35.000000000 +0200
@@ -430,11 +430,19 @@
                     if (classpathAttribute == null) {
                         classpathAttribute = new Attribute();
                         classpathAttribute.setName(ATTRIBUTE_CLASSPATH);
+                        if (getAttribute(ATTRIBUTE_CLASSPATH) != null) {
+                               for (Enumeration attribEnum = getAttribute(ATTRIBUTE_CLASSPATH).getValues(); 
attribEnum.hasMoreElements();) {
+                                       String value = (String)attribEnum.nextElement();
+                                       String previousValue = (classpathAttribute.getValue() != null ? 
classpathAttribute.getValue() : "");
+                                       classpathAttribute.setValue(previousValue + " " + value);
+                               }
+                        }
                     }
                     Enumeration cpe = attribute.getValues();
                     while (cpe.hasMoreElements()) {
                         String value = (String) cpe.nextElement();
-                        classpathAttribute.addValue(value);
+                        String previousValue = (classpathAttribute.getValue() != null ? 
classpathAttribute.getValue() : "");
+                        classpathAttribute.setValue(previousValue + " " + value);
                     }
                 } else {
                     // the merge file always wins
@@ -629,7 +637,12 @@
                 return;
             }
             String attributeKey = attribute.getKey();
-            attributes.put(attributeKey, attribute);
+            if (attributes.contains(attributeKey)) {
+               Attribute principalAttribute = (Attribute)attributes.get(attributeKey);
+               principalAttribute.addValue(attribute.getValue());
+            } else {
+               attributes.put(attributeKey, attribute);
+            }
             if (!attributeIndex.contains(attributeKey)) {
                 attributeIndex.addElement(attributeKey);
             }
Comment 1 Vassilis Touloumtzoglou 2006-05-24 20:43:39 UTC
Created attachment 18349 [details]
main/org/apache/tools/ant/taskdefs/Manifest.java

main/org.apache.tools.ant.taskdefs.Manifest.java patch
It's a core task so it's in ant.jar
Comment 2 Stefan Bodewig 2009-08-20 02:20:54 UTC
The code for Class-Path isn't there to merge Class-Path attributes from different manifests but to merge different Class-Path attributes inside the same section of the same manifest - so it is not really related to merging at all.

See the warning generated in line 622 of svn revision 802486

                        warnings.addElement("Multiple Class-Path attributes "
                            + "are supported but violate the Jar "
                            + "specification and may not be correctly "
                            + "processed in all environments");

Merging of manifests as done by the tasks update-mode means add new attributes and overwrite existing attributes, including Class-Path and this is what the current code does.

See the comment in line 487 of said revision

                // the merge file *always* wins, even for Class-Path

I don't recall why Ant supports multiple Class-Path attributes but this really is the only purpose of the existing code.

As you said, it may be useful to have a separate option to merge Class-Path attributes and your patch contains most of what would be needed to implement it.  Am looking into it.
Comment 3 Stefan Bodewig 2009-08-20 06:52:21 UTC
svn revision 806174 introduces two separate attributes for the merging and "flattening" of Class-Path attributes.