Add support for adding extra classpaths and system properties to embedded container implementations

Vincent Massol added a comment - 27/Jul/06 3:10 PM Update isssue as we know that it's not implemented by design. Also it's a new feature and not a bug

Vincent Massol added a comment - 27/Jul/06 3:11 PM Nicer formatting

Sharmarke Aden added a comment - 28/Jul/06 1:43 PM This patch is for maven 2 plugin. The patch adds support for adding extra classpath entries to embedded containers. Note that this patch also resolves the issue with system properties not being passed to embedded containers ( http://jira.codehaus.org/browse/CARGO-396 ).

Eric Brown added a comment - 04/Sep/06 6:56 PM cargo_maven2plugin_C-396_and_C-397.patch fixes CARGO-396 system-properties issue, but jetty ignores the embeddedClasspath it properly sets. That is why (I believe) AbstractCargoMojo.java called Thread.currentThread().setContextClassLoader(...). So anyway, using cargo_maven2plugin_C-396_and_C-397.patch did not expand jetty's classpath appropriately when I added <configuration><container><dependencies><dependency>.... So the bug COULD be considered to be both in cargo-maven2-plugin (for not setting the embedded classpath) properly and in the jetty portion of cargo for not using the embedded classpath. Or, we could consider it to be entirely in the cargo-maven2-plugin for both not setting the embedded classpath and using it before jetty is launched. I prefer the second option because it makes fixing it for me easier. (Although I'd rather see a mechanism other than Thread.currentThread().setContextClassLoader(...) to add the classpath as with more work, I think there are better ways. It is just I think that could be a future ticket/improvement. I and hopefully others can use this sooner.) The attached patch moves Thread.currentThread().setContextClassLoader() to a place where it will be used by the code that sets the embedded classpath. Appropriate unit tests have also been included (for both 396 and 397).

Sharmarke Aden added a comment - 05/Sep/06 3:59 PM Hmmm...That's weird. I've been using my patch in my environment for a month or so now and it has works fine. Looking back I do see that the classloader is set for the container but Thread.currentThread().setContextClassLoader() is never invoked. I need to investigate why my code worked. Nice catch Eric...and kudos for the Unit tests

Eric Brown added a comment - 08/Sep/06 6:26 PM I'm fairly certain that I tried your patch with jetty4x-jetty6x and was unable to get the dependencies to work, but most of my testing was with jetty4x. It could very well also be some subtle thing you're doing with your pom.xml that I'm not. ... <plugin> <groupId> org.topazproject.patch.cargo </groupId> <artifactId> cargo-maven2-plugin </artifactId> <configuration> <configuration> <deployables> <deployable> <groupId> ${pom.parent.groupId} </groupId> <artifactId> ${pom.parent.artifactId}-webapp </artifactId> <type> war </type> </deployable> <!-- We need the articles, users and preferences webapps too --> <deployable> <groupId> org.topazproject.ws </groupId> <artifactId> ws-articles-webapp </artifactId> <type> war </type> </deployable> <deployable> <groupId> org.topazproject.ws </groupId> <artifactId> ws-users-webapp </artifactId> <type> war </type> </deployable> <deployable> <groupId> org.topazproject.ws </groupId> <artifactId> ws-pap-webapp </artifactId> <type> war </type> </deployable> </deployables> </configuration> </configuration> <executions> <execution> <id> jetty-start </id> </execution> </executions> </plugin> <pluginManagement> <plugins> <plugin> <groupId> org.topazproject.patch.cargo </groupId> <artifactId> cargo-maven2-plugin </artifactId> <version> 0.2 </version> <configuration> <wait> false </wait> <container> <containerId> jetty4x </containerId> <type> embedded </type> <output> ${project.build.directory}/jetty.log </output> <log> ${project.build.directory}/cargo.log </log> <dependencies> <dependency> <groupId> log4j </groupId> <artifactId> log4j </artifactId> </dependency> <dependency> <location> ${user.home}/.m2/topaz/container-classes </location> </dependency> <dependency> <location> ${basedir}/src/it/container-classes </location> </dependency> </dependencies> </container> <configuration> <home> ${project.build.directory}/jetty </home> <properties> <cargo.servlet.port> 9997 </cargo.servlet.port> <cargo.logging> high </cargo.logging> </properties> <deployables> <deployable> <groupId> ${pom.parent.groupId} </groupId> <artifactId> ${pom.parent.artifactId}-webapp </artifactId> <type> war </type> </deployable> </deployables> </configuration> </configuration> <executions> <execution> <id> jetty-start </id> <phase> pre-integration-test </phase> <goals> <goal> start </goal> </goals> </execution> <!-- doesn't work properly (yet) <execution> <id> jetty-stop </id> <phase> post-integration-test </phase> <goals> <goal> stop </goal> </goals> </execution> --> </executions> </plugin> ...

Vincent Massol added a comment - 16/Sep/06 10:28 AM Aden, let me know when you think this patch should be applied. From Eric's comments I gather you need to find out why it's not working for him, right?

Vincent Massol added a comment - 16/Sep/06 10:29 AM hmm... re-reading the comments I see Eric has provided a new patch. Eric, is it ready for inclusion? Aden, is Eric's patch also working for you? Thanks

Sharmarke Aden added a comment - 19/Sep/06 5:21 PM My application worked because I had code in the app that picked up my config file from either the classpath or a system property. Apparently a system property was being set somewhere else therefore the classpath was not being checked for the config file resource. I have just tried Eric's patch and it works for me. The classpath is being set correctly and resources can be picked up.