Cargo

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. Text File
    CARGO-727-core.patch
    23/Aug/10 6:00 AM
    3 kB
    Anton Khitrenovich
  2. Text File
    CARGO-727-extensions.patch
    23/Aug/10 6:00 AM
    6 kB
    Anton Khitrenovich

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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.
Hide
Permalink
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

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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: