Cargo
  1. Cargo
  2. CARGO-727

WAR created by "uberwar" does not contain manifest

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0
    • Fix Version/s: 1.0.3
    • Component/s: Maven2
    • Labels:
      None
    • Complexity:
      Intermediate
    • Number of attachments :
      2

      Description

      Obviously, it is impossible to "merge" manifests from source WARs.
      However, there is no possibility to specify own, even minimal, manifest for merged WAR file.

      I suggest to add this option to plugin configuration.
      For example,

      	<plugin>
      		<groupId>org.codehaus.cargo</groupId>
      		<artifactId>cargo-maven2-plugin</artifactId>
      		<extensions>true</extensions>
      		<configuration>
      			<descriptor>src/assemble/merge.xml</descriptor>
      			<manifestEntries>
      				<Implementation-Version>${majorVersion}</Implementation-Version>
      				<Implementation-Vendor>MyCompany</Implementation-Vendor>
      			</manifestEntries>
      		</configuration>
      	</plugin>
      

      From the code point of view:
      1) JarUtils.createJarFromDirectory():

      • use Manifest as additional parameter, map two-argument call to new function
      • when not null, pass Manifest as second argument to the constructor of JarOutputStream
        2) UberWarMojo/WarArchiveMerger/MergedWarArchive trio:
      • use configuration values to create correct Manifest object
      • send Manifest to JarUtil in MergedWarArchive.store()
      1. CARGO-727-core.patch
        3 kB
        Anton Khitrenovich
      2. CARGO-727-extensions.patch
        6 kB
        Anton Khitrenovich

        Activity

        Hide
        Aleksander Adamowski added a comment -

        I think it would also be a useful option to allow generation of manifest using logic inherited from Maven archiver - the logic which can e.g. add classpath to the manifest:

        http://maven.apache.org/shared/maven-archiver/examples/classpath.html

        In the case of uberwar, the classpath should consist of all transitive dependencies of merged war artifacts (not including the merged wars themselves, obviously).

        Note that this functionality is especially important considering the Java EE 5 specification which strongly suggests that all packaged JARs and WARs in an EAR have their dependencies listed in MANIFEST.MF's Class-Path - for example, see the specification, section EE.8.2.6 on page 158.

        Show
        Aleksander Adamowski added a comment - I think it would also be a useful option to allow generation of manifest using logic inherited from Maven archiver - the logic which can e.g. add classpath to the manifest: http://maven.apache.org/shared/maven-archiver/examples/classpath.html In the case of uberwar, the classpath should consist of all transitive dependencies of merged war artifacts (not including the merged wars themselves, obviously). Note that this functionality is especially important considering the Java EE 5 specification which strongly suggests that all packaged JARs and WARs in an EAR have their dependencies listed in MANIFEST.MF's Class-Path - for example, see the specification, section EE.8.2.6 on page 158.
        Hide
        Aleksander Adamowski added a comment -

        Also, the impossibility of merging manifests isn't that obvious.

        Standard Java API has a ready-made method for that!

        http://download.oracle.com/javase/1.5.0/docs/api/java/util/jar/Manifest.html#read(java.io.InputStream)

        public void read(InputStream is)
                  throws IOException
        Reads the Manifest from the specified InputStream. The entry names and attributes read will be merged in with the current manifest entries.
        

        The standard JAR utility makes use of it:

        jar cmf manifest-addition jar-file input-file(s)
        Let's look at the options and arguments used in this command:

        The c option indicates that you want to create a JAR file.
        The m option indicates that you want to merge information from an existing manifest file into the manifest file of the JAR file you're creating.
        ....

        It seems that it should be even easier than merging JARs, which uberwar is all about...

        Show
        Aleksander Adamowski added a comment - Also, the impossibility of merging manifests isn't that obvious. Standard Java API has a ready-made method for that! http://download.oracle.com/javase/1.5.0/docs/api/java/util/jar/Manifest.html#read(java.io.InputStream ) public void read(InputStream is) throws IOException Reads the Manifest from the specified InputStream. The entry names and attributes read will be merged in with the current manifest entries. The standard JAR utility makes use of it: jar cmf manifest-addition jar-file input-file(s) Let's look at the options and arguments used in this command: The c option indicates that you want to create a JAR file. The m option indicates that you want to merge information from an existing manifest file into the manifest file of the JAR file you're creating. .... It seems that it should be even easier than merging JARs, which uberwar is all about...
        Hide
        Aleksander Adamowski added a comment -

        Another working example to take an example of:

        Ant "JAR" task (http://ant.apache.org/manual/Tasks/jar.html):

        mergeClassPathAttributes

        Whether to merge the Class-Path attributes found in different manifests (if merging manifests). If false, only the attribute of the last merged manifest will be preserved. Since Ant 1.8.0.
        unless you also set flattenAttributes to true this may result in manifests containing multiple Class-Path attributes which violates the manifest specification.

        This seems to be exactly what's needed, with a ready made tested logic, so no need to reinvent the wheel.

        Show
        Aleksander Adamowski added a comment - Another working example to take an example of: Ant "JAR" task ( http://ant.apache.org/manual/Tasks/jar.html): mergeClassPathAttributes Whether to merge the Class-Path attributes found in different manifests (if merging manifests). If false, only the attribute of the last merged manifest will be preserved. Since Ant 1.8.0. unless you also set flattenAttributes to true this may result in manifests containing multiple Class-Path attributes which violates the manifest specification. This seems to be exactly what's needed, with a ready made tested logic, so no need to reinvent the wheel.
        Show
        Aleksander Adamowski added a comment - Hmm seems like the logic should be placed in WarArchiveMerger.java: http://www.google.com/codesearch/p?hl=en#Vzc_g0ZQBa4/trunk/maven/org/codehaus/cargo/cargo-core/api/module/src/main/java/org/codehaus/cargo/module/webapp/merge/MergedWarArchive.java&q=file:WarArchiveMerger.java&d=17 And for a working example, here's the exact Ant source file and line where manifest Class-Path merging takes place: http://fisheye6.atlassian.com/browse/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/Manifest.java?r=HEAD#l533
        Hide
        Anton Khitrenovich added a comment -

        The patch for original issue is attached. Here, UberWarMojo makes use of standard maven-archiver to pack the merged WAR file. As a result, standard <archive> configuration tag is supported (including manifest creation). Basic WAR merging functionality in MergedWarArchive is not affected by this change.

        Show
        Anton Khitrenovich added a comment - The patch for original issue is attached. Here, UberWarMojo makes use of standard maven-archiver to pack the merged WAR file. As a result, standard <archive> configuration tag is supported (including manifest creation). Basic WAR merging functionality in MergedWarArchive is not affected by this change.
        Hide
        Savas Ali Tokmen added a comment -

        Both patches look good to me. We will include this in 1.0.3, I'm now exchanging with Anton to see if he wants to commit this himself or let me do it for him.

        Show
        Savas Ali Tokmen added a comment - Both patches look good to me. We will include this in 1.0.3, I'm now exchanging with Anton to see if he wants to commit this himself or let me do it for him.
        Hide
        Anton Khitrenovich added a comment -

        Plugin configuration in the solution looks different than in original issue description (note the <archive> node).
        For full list of supported elements see maven-archiver reference page.

        <plugin>
        	<groupId>org.codehaus.cargo</groupId>
        	<artifactId>cargo-maven2-plugin</artifactId>
        	<extensions>true</extensions>
        	<configuration>
        		<descriptor>src/assemble/merge.xml</descriptor>
        		<archive>
        			<addMavenDescriptor>false</addMavenDescriptor>
        			<manifest>
        				<addDefaultImplementationEntries>true</addDefaultImplementationEntries>
        				<addDefaultSpecificationEntries>true</addDefaultSpecificationEntries>
        			</manifest>
        		</archive>
        	</configuration>
        </plugin>
        
        Show
        Anton Khitrenovich added a comment - Plugin configuration in the solution looks different than in original issue description (note the <archive> node). For full list of supported elements see maven-archiver reference page. <plugin> <groupId>org.codehaus.cargo</groupId> <artifactId>cargo-maven2-plugin</artifactId> <extensions>true</extensions> <configuration> <descriptor>src/assemble/merge.xml</descriptor> <archive> <addMavenDescriptor>false</addMavenDescriptor> <manifest> <addDefaultImplementationEntries>true</addDefaultImplementationEntries> <addDefaultSpecificationEntries>true</addDefaultSpecificationEntries> </manifest> </archive> </configuration> </plugin>
        Hide
        Anton Khitrenovich added a comment -

        Changes submitted to the repository (rev. ##4261, 4262).

        Show
        Anton Khitrenovich added a comment - Changes submitted to the repository (rev. ##4261, 4262).

          People

          • Assignee:
            Anton Khitrenovich
            Reporter:
            Anton Khitrenovich
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: