Created attachment 22319 [details] Agent.java rewritten with reflection As part of an effort to remove the need to download components manually (outside Maven) I have found that when compiling with Java 5+ it is only Agent.java which need the jmxtools.jar in the compilation class path. The attached patch is a rewrite of Agent.java using reflection so it can compile without the presence of jmxtools.jar. I would appreciate help in getting a test environment running.
Created attachment 22340 [details] Whole file (if patch is bad)
Additionally these dependencies can be removed from pom.xml <dependency> <groupId>com.sun.jdmk</groupId> <artifactId>jmxtools</artifactId> <version>1.2.1</version> <optional>true</optional> </dependency> <dependency> <groupId>com.sun.jmx</groupId> <artifactId>jmxri</artifactId> <version>1.2.1</version> <optional>true</optional> </dependency>
While I like the idea of removing the compile time dependency to those evil Sun jars, I'm not so sure swallowing an exception is such a good idea, from a stylistic point I think wrapping that exception and throwing an IllegalStateException would be better. Lets face it, if the Exception does happen, the use probably wants to know about it (yes there's logging, but is continuing in the face of error a good idea here?) Still, that removal of the Sun jar dependency is tempting! :)
(In reply to comment #3) > While I like the idea of removing the compile time dependency to those evil Sun > jars, I'm not so sure swallowing an exception is such a good idea, from a > stylistic point I think wrapping that exception and throwing an > IllegalStateException would be better. > > Lets face it, if the Exception does happen, the use probably wants to know > about it (yes there's logging, but is continuing in the face of error a good > idea here?) > > Still, that removal of the Sun jar dependency is tempting! :) Yes. From what I understand the try-catch-log.error-return approach is the way it is done elsewhere.
Created attachment 22389 [details] Alternative patch that preserves existing log4j behavior This patch changes the checked exceptions raised by reflection into the equivalent error that would occur with the previous code in the same scenario.
Committed last patch in rev 682879, modified to throw RuntimeException instead of Errors in rev 682998 and 683009 per discussion on log4j-dev.