But first please explain why you think this change is correct. It looks wrong to me, and I think the test was correct. Maybe i don't understand what the test is doing, but I don't think "libfolder/" is in the "manifest classpath" being tested but it shows up in the results. Why should it?
thanks david jencks On Jun 1, 2011, at 2:02 PM, Kevan Miller wrote: > This commit broke the build. See > http://ci.apache.org/builders/geronimo-server-trunk/builds/41 > > Can you fix the test errors? > > --kevan > > On May 30, 2011, at 9:50 AM, genspr...@apache.org wrote: > >> Author: genspring >> Date: Mon May 30 13:50:57 2011 >> New Revision: 1129174 >> >> URL: http://svn.apache.org/viewvc?rev=1129174&view=rev >> Log: >> EJB31 spec EE.8.2.1 : >> A JAR format file (such as a .jar file, .war file, or .rar file) may >> reference a >> .jar file or directory by naming the referenced .jar file or directory in a >> Class-Path header in the referencing JAR file’s Manifest file >> >> Modified: >> >> geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ClassPathUtils.java >> >> geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java >> >> geronimo/server/trunk/plugins/openejb/geronimo-openejb-builder/src/main/java/org/apache/geronimo/openejb/deployment/EjbModuleBuilder.java >> >> Modified: >> geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ClassPathUtils.java >> URL: >> http://svn.apache.org/viewvc/geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ClassPathUtils.java?rev=1129174&r1=1129173&r2=1129174&view=diff >> ============================================================================== >> --- >> geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ClassPathUtils.java >> (original) >> +++ >> geronimo/server/trunk/framework/modules/geronimo-deployment/src/main/java/org/apache/geronimo/deployment/ClassPathUtils.java >> Mon May 30 13:50:57 2011 >> @@ -136,11 +136,13 @@ public class ClassPathUtils { >> if (!targetUri.getPath().endsWith("/")) { >> targetUri = URI.create(targetUri.getPath() + "/"); >> } >> + >> + //1, add the directory to classpath >> + classPath.add(targetUri.getPath()); >> + >> + //2, add *.jar under the directory to classpath >> for (File file : factory.listFiles(targetUri)) { >> - if (file.isDirectory()) { >> - log.debug("Sub directory [" + >> file.getAbsolutePath() + "] in the manifest entry directory is ignored"); >> - continue; >> - } >> + >> if (!file.getName().endsWith(".jar")) { >> log.debug("Only jar files are added to classpath, >> file [" + file.getAbsolutePath() + "] is ignored"); >> continue; >> @@ -149,15 +151,6 @@ public class ClassPathUtils { >> } >> } else { >> if (!pathUri.getPath().endsWith(".jar")) { >> - if (manifestClassLoaderMode == MFCP_STRICT) { >> - problems.add(new DeploymentException( >> - "Manifest class path entries must end >> with the .jar extension (J2EE 1.4 Section 8.2): path= " >> - + path + ", module= " + >> moduleBaseUri)); >> - } else { >> - log.info("The " + manifestClassLoaderMessage + >> " processing mode is in effect.\n" >> - + "Therefore, a manifest classpath >> entry which does not end with .jar, " >> - + pathUri + " is being permitted and >> ignored."); >> - } >> continue; >> } >> classPath.add(targetUri.getPath()); >> @@ -234,6 +227,32 @@ public class ClassPathUtils { >> } >> >> URI targetUri = moduleBaseUri.resolve(pathUri); >> + >> + try { >> + if (factory.isDirectory(targetUri)) { >> + if (!targetUri.getPath().endsWith("/")) { >> + targetUri = URI.create(targetUri.getPath() + "/"); >> + } >> + >> + //1, add the directory to classpath >> + addToClassPath(moduleBaseUri, resolutionUri, targetUri, >> classpath, exclusions, factory, problems); >> + >> + //2, add *.jar under the directory to classpath >> + for (File file : factory.listFiles(targetUri)) { >> + >> + if (!file.getName().endsWith(".jar")) { >> + log.debug("Only jar files are added to >> classpath, file [" + file.getAbsolutePath() + "] is ignored"); >> + continue; >> + } >> + addToClassPath(moduleBaseUri, resolutionUri, >> targetUri.resolve(file.getName()), classpath, exclusions, factory, problems); >> + } >> + } else { >> + if (!pathUri.getPath().endsWith(".jar")) { >> + continue; >> + } >> + addToClassPath(moduleBaseUri, resolutionUri, targetUri, >> classpath, exclusions, factory, problems); >> + } >> + } catch (IOException e) {} >> >> try { >> if (factory.isDirectory(targetUri)) { >> @@ -281,7 +300,7 @@ public class ClassPathUtils { >> >> private static void addToClassPath(URI moduleBaseUri, URI resolutionUri, >> URI targetUri, Collection<String> classpath, Collection<String> exclusions, >> JarFileFactory factory, List<DeploymentException> problems) throws >> DeploymentException { >> String targetEntry = targetUri.toString(); >> - if (exclusions.contains(targetEntry)) { >> + if (exclusions!=null && exclusions.contains(targetEntry)) { >> return; >> } >> URI resolvedUri = resolutionUri.resolve(targetUri); >> @@ -291,6 +310,11 @@ public class ClassPathUtils { >> return; >> } >> classpath.add(classpathEntry); >> + >> + if (!classpathEntry.endsWith(".jar")){ >> + return; >> + } >> + >> JarFile classPathJarFile; >> try { >> classPathJarFile = factory.newJarFile(targetUri); >> >> Modified: >> geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java >> URL: >> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java?rev=1129174&r1=1129173&r2=1129174&view=diff >> ============================================================================== >> --- >> geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java >> (original) >> +++ >> geronimo/server/trunk/plugins/client/geronimo-client-builder/src/main/java/org/apache/geronimo/client/builder/AppClientModuleBuilder.java >> Mon May 30 13:50:57 2011 >> @@ -25,12 +25,15 @@ import java.net.URL; >> import java.util.ArrayList; >> import java.util.Collection; >> import java.util.Collections; >> +import java.util.Enumeration; >> import java.util.HashMap; >> import java.util.LinkedList; >> import java.util.List; >> import java.util.Map; >> +import java.util.Set; >> import java.util.StringTokenizer; >> import java.util.jar.Attributes; >> +import java.util.jar.JarEntry; >> import java.util.jar.JarFile; >> import java.util.jar.Manifest; >> import java.util.zip.ZipEntry; >> @@ -563,12 +566,9 @@ public class AppClientModuleBuilder impl >> appClientModule.setEarContext(appClientDeploymentContext); >> appClientModule.setRootEarContext(appClientDeploymentContext); >> >> - try { >> - >> appClientDeploymentContext.addIncludeAsPackedJar(URI.create(module.getTargetPath()), >> moduleFile); >> - } catch (IOException e) { >> - throw new DeploymentException("Unable to copy app client >> module jar into configuration: " + moduleFile.getName(), e); >> - } >> - if (module.getParentModule() != null) { >> + >> + if (module.getParentModule() != null) { >> + >> Collection<String> libClasspath = >> module.getParentModule().getClassPath(); >> for (String libEntryPath : libClasspath) { >> try { >> @@ -579,12 +579,65 @@ public class AppClientModuleBuilder impl >> } >> } >> module.getClassPath().addAll(libClasspath); >> + >> + >> + Enumeration<JarEntry> ear_entries = earFile.entries(); >> + >> + //Copy non archive files from ear file to appclient >> configuration. These >> + // files are needed when caculating dir classpath in manifest. >> + while (ear_entries.hasMoreElements()) { >> + >> + ZipEntry ear_entry = ear_entries.nextElement(); >> + URI targetPath = >> module.getParentModule().resolve(ear_entry.getName()); >> + >> + if (!ear_entry.getName().endsWith(".jar") && >> !ear_entry.getName().endsWith(".war") >> + && !ear_entry.getName().endsWith(".rar") && >> !ear_entry.getName().startsWith("META-INF")) >> + { >> + appClientDeploymentContext.addFile(targetPath, earFile, >> ear_entry); >> + } >> } >> + >> + Collection<String> appClientModuleClasspaths = >> module.getClassPath(); >> + >> + try { >> + // extract the client Jar file into a standalone packed jar >> file and add the contents to the output >> + URI moduleBase = new URI(module.getTargetPath()); >> + >> appClientDeploymentContext.addIncludeAsPackedJar(moduleBase, moduleFile); >> + // add manifest class path entries to the app client context >> + addManifestClassPath(appClientDeploymentContext, >> appClientModule.getEarFile(), moduleFile, moduleBase); >> + >> + >> + } catch (IOException e) { >> + throw new DeploymentException("Unable to copy app client >> module jar into configuration: " + moduleFile.getName(), e); >> + } catch (URISyntaxException e) { >> + throw new DeploymentException("Unable to get app client >> module base URI " + module.getTargetPath(), e); >> + } >> + >> + appClientModuleClasspaths.add(module.getTargetPath()); >> + EARContext moduleContext = module.getEarContext(); >> + Collection<String> moduleLocations = >> module.getParentModule().getModuleLocations(); >> + URI baseUri = URI.create(module.getTargetPath()); >> + >> moduleContext.getCompleteManifestClassPath(module.getDeployable(), baseUri, >> URI.create("."), appClientModuleClasspaths, moduleLocations); >> + >> + >> + for (String classpath: appClientModuleClasspaths){ >> + appClientDeploymentContext.addToClassPath(classpath); >> + >> + //Copy needed jar from ear to appclient configuration. >> + if (classpath.endsWith(".jar")){ >> + >> + NestedJarFile library = new NestedJarFile(earFile, >> classpath); >> + >> appClientDeploymentContext.addIncludeAsPackedJar(URI.create(classpath), >> library); >> + >> + } >> + } >> + } >> + >> } catch (DeploymentException e) { >> throw e; >> } catch (IOException e) { >> throw new DeploymentException(e); >> - } >> + } >> for (Module connectorModule : appClientModule.getModules()) { >> if (connectorModule instanceof ConnectorModule) { >> >> getConnectorModuleBuilder().installModule(connectorModule.getModuleFile(), >> appClientDeploymentContext, connectorModule, configurationStores, >> targetConfigurationStore, repositories); >> @@ -652,25 +705,12 @@ public class AppClientModuleBuilder impl >> } >> } >> >> - // Create a Module ID Builder defaulting to similar settings to use >> for any children we create >> - ModuleIDBuilder idBuilder = new ModuleIDBuilder(); >> - >> idBuilder.setDefaultGroup(appClientModule.getEnvironment().getConfigId().getGroupId()); >> - >> idBuilder.setDefaultVersion(appClientModule.getEnvironment().getConfigId().getVersion()); >> + >> try { >> try { >> >> //register the message destinations in the app client ear >> context. >> namingBuilders.initContext(appClient, geronimoAppClient, >> appClientModule); >> - // extract the client Jar file into a standalone packed jar >> file and add the contents to the output >> - URI moduleBase = new URI(appClientModule.getTargetPath()); >> - try { >> - >> appClientDeploymentContext.addIncludeAsPackedJar(moduleBase, moduleFile); >> - } catch (IOException e) { >> - throw new DeploymentException("Unable to copy app >> client module jar into configuration: " + moduleFile.getName(), e); >> - } >> - >> - // add manifest class path entries to the app client context >> - addManifestClassPath(appClientDeploymentContext, >> appClientModule.getEarFile(), moduleFile, moduleBase); >> >> // get the classloader >> Bundle appClientClassBundle = >> appClientDeploymentContext.getDeploymentBundle(); >> @@ -896,12 +936,11 @@ public class AppClientModuleBuilder impl >> throw new DeploymentException("Invalid manifest classpath >> entry: jarFile=" + jarFileLocation + ", path=" + path, e); >> } >> >> - if (!pathUri.getPath().endsWith(".jar")) { >> - throw new DeploymentException("Manifest class path entries >> must end with the .jar extension (JAVAEE 5 Section 8.2): jarFile=" + >> jarFileLocation + ", path=" + path); >> - } >> if (pathUri.isAbsolute()) { >> throw new DeploymentException("Manifest class path entries >> must be relative (JAVAEE 5 Section 8.2): jarFile=" + jarFileLocation + ", >> path=" + path); >> } >> + >> + Enumeration<JarEntry> ear_entries = earFile.entries(); >> >> // determine the target file >> URI classPathJarLocation = jarFileLocation.resolve(pathUri); >> @@ -915,23 +954,44 @@ public class AppClientModuleBuilder impl >> if (entry == null) { >> throw new DeploymentException("Cound not find manifest >> class path entry: jarFile=" + jarFileLocation + ", path=" + path); >> } >> - >> + >> try { >> - // copy the file into the output context >> - deploymentContext.addFile(classPathJarLocation, >> earFile, entry); >> + >> + if (!entry.getName().endsWith(".jar")) { >> + >> + while (ear_entries.hasMoreElements()) { >> + ZipEntry ear_entry = ear_entries.nextElement(); >> + URI targetPath = >> jarFileLocation.resolve(ear_entry.getName()); >> + if >> (ear_entry.getName().startsWith(classPathJarLocation.getPath())) >> + deploymentContext.addFile(targetPath, >> earFile, ear_entry); >> + } >> + } else { >> + >> + // copy the file into the output context >> + deploymentContext.addFile(classPathJarLocation, >> earFile, entry); >> + } >> } catch (IOException e) { >> - throw new DeploymentException("Cound not copy manifest >> class path entry into configuration: jarFile=" + jarFileLocation + ", path=" >> + path, e); >> + throw new DeploymentException( >> + "Cound not copy manifest class path entry into >> configuration: jarFile=" + jarFileLocation >> + + ", path=" + path, e); >> } >> >> JarFile classPathJarFile; >> - try { >> - classPathJarFile = new JarFile(classPathFile); >> - } catch (IOException e) { >> - throw new DeploymentException("Manifest class path >> entries must be a valid jar file (JAVAEE 5 Section 8.2): jarFile=" + >> jarFileLocation + ", path=" + path, e); >> - } >> + >> + if (classPathFile.getName().endsWith(".jar")) { >> >> - // add the client jars of this class path jar >> - addManifestClassPath(deploymentContext, earFile, >> classPathJarFile, classPathJarLocation); >> + try { >> + classPathJarFile = new JarFile(classPathFile); >> + } catch (IOException e) { >> + throw new DeploymentException( >> + "Manifest class path entries must be a >> valid jar file (JAVAEE 5 Section 8.2): jarFile=" >> + + jarFileLocation + ", path=" + >> path, e); >> + } >> + >> + // add the client jars of this class path jar >> + addManifestClassPath(deploymentContext, earFile, >> classPathJarFile, classPathJarLocation); >> + >> + } >> } >> } >> } >> >> Modified: >> geronimo/server/trunk/plugins/openejb/geronimo-openejb-builder/src/main/java/org/apache/geronimo/openejb/deployment/EjbModuleBuilder.java >> URL: >> http://svn.apache.org/viewvc/geronimo/server/trunk/plugins/openejb/geronimo-openejb-builder/src/main/java/org/apache/geronimo/openejb/deployment/EjbModuleBuilder.java?rev=1129174&r1=1129173&r2=1129174&view=diff >> ============================================================================== >> --- >> geronimo/server/trunk/plugins/openejb/geronimo-openejb-builder/src/main/java/org/apache/geronimo/openejb/deployment/EjbModuleBuilder.java >> (original) >> +++ >> geronimo/server/trunk/plugins/openejb/geronimo-openejb-builder/src/main/java/org/apache/geronimo/openejb/deployment/EjbModuleBuilder.java >> Mon May 30 13:50:57 2011 >> @@ -362,7 +362,7 @@ public class EjbModuleBuilder implements >> >> >> >> - detectTempClassLoader = >> ClassLoaderUtil.createTempClassLoader(ClassLoaderUtil.createClassLoader(jarPath, >> (URL[])libURLs.toArray(new URL[0]), OpenEJB.class.getClassLoader())); >> + detectTempClassLoader = >> ClassLoaderUtil.createTempClassLoader(ClassLoaderUtil.createClassLoader(jarPath, >> libURLs.toArray(new URL[0]), OpenEJB.class.getClassLoader())); >> >> ResourceFinder finder = new ResourceFinder("", >> detectTempClassLoader, baseUrl); >> >> @@ -397,7 +397,7 @@ public class EjbModuleBuilder implements >> libURLs.clear(); >> libURLs.add(baseUrl); >> >> - ejbModuleTempClassLoader = >> ClassLoaderUtil.createTempClassLoader(ClassLoaderUtil.createClassLoader(jarPath, >> (URL[])libURLs.toArray(new URL[0]), OpenEJB.class.getClassLoader())); >> + ejbModuleTempClassLoader = >> ClassLoaderUtil.createTempClassLoader(ClassLoaderUtil.createClassLoader(jarPath, >> libURLs.toArray(new URL[0]), OpenEJB.class.getClassLoader())); >> >> >> // create the EJB Module >> @@ -670,8 +670,12 @@ public class EjbModuleBuilder implements >> // extract the ejbJar file into a standalone packed jar file >> and add the contents to the output >> >> earContext.addIncludeAsPackedJar(URI.create(module.getTargetPath()), >> moduleFile); >> // add manifest class path entries to the ejb module classpath >> - Set<String> EjbModuleClasspaths = module.getClassPath(); >> - earContext.addManifestClassPath(moduleFile, >> URI.create("."), EjbModuleClasspaths); >> + Collection<String> EjbModuleClasspaths = >> module.getClassPath(); >> + EjbModuleClasspaths.add(module.getTargetPath()); >> + Collection<String> moduleLocations = module.isStandAlone() >> ? null : module.getParentModule() >> + .getModuleLocations(); >> + URI baseUri = URI.create(module.getTargetPath()); >> + >> earContext.getCompleteManifestClassPath(module.getDeployable(), baseUri, >> URI.create("."), EjbModuleClasspaths, moduleLocations); >> >> for (String classpath:EjbModuleClasspaths){ >> earContext.addToClassPath(classpath); >> @@ -728,12 +732,7 @@ public class EjbModuleBuilder implements >> private void doInitContext(EARContext earContext, Module module, Bundle >> bundle) throws DeploymentException { >> EjbModule ejbModule = (EjbModule) module; >> >> - Collection<String> manifestcp = module.getClassPath(); >> - manifestcp.add(module.getTargetPath()); >> - EARContext moduleContext = module.getEarContext(); >> - Collection<String> moduleLocations = >> EARContext.MODULE_LIST_KEY.get(module.getRootEarContext().getGeneralData()); >> - URI baseUri = URI.create(module.getTargetPath()); >> - moduleContext.getCompleteManifestClassPath(module.getDeployable(), >> baseUri, URI.create("."), manifestcp, moduleLocations); >> + >> >> GeronimoEjbInfo ejbInfo = getEjbInfo(earContext, ejbModule, bundle); >> >> >> >
