robert burrell donkin wrote:
after some investigation, the issue which turned up with RC8 was
determined not to be a bug. notes have been added to the troubleshooting
documentation.
RC10 is available from:
http://people.apache.org/~rdonkin/commons-logging. please download,
check and then vote.

<snip>

Sorry to be jumping into this at the last minute. I've done a code review of /src/java and /xdocs and compared them to the 1.0.4 release. As you might have seen I committed some spelling corrections to the xdocs. The added documents are excellent by the way!

There were a couple of questions that popped up along the way, which I will deal with below, one file at a time. Nothing big, mostly documentation stuff, so nothing to hold up a release.

The attached patch file fixes a couple of them, while others need input from you. I didn't want to commit the patch before I had checked it with the rest of you.

I hope to be doing some practical tests in applet environments tomorrow.


\commons-logging-1.1-RC10-src\RELEASE-NOTES.txt

Lines 147-150:
"Previous releases of commons-logging-api.jar contained the Jdk14Logger class;
this is now deprecated. It will be removed from the API jar in some future
release. If your application needs this jar, then instead of
upgrading to commons-logging-api-1.1.jar, upgrade to commons-logging-1.1.jar."

I don't see why we advice this. The jar still contains Jdk14Logger. Should we encourage people to switch jars now or when JCL 2 comes out?

Lines 173-174:
"File commons-logging-api-nn.jar provides no adapters to external logging
libraries, just the internally implemented SimpleLog and NoOpLog classes."

This is not entirely true. The Jdk14Logger is there as well.

There is nothing in the release notes about the fact that AvalonLogger no longer implements Serializable. Do we need that?


\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\AvalonLogger.java

Line 251: Javadoc for trace(Object message, Throwable t) was changed like this: @see org.apache.commons.logging.Log#trace(java.lang.Object, java.lang.Throwable) ---> @see org.apache.commons.logging.Log#debug(Object, Throwable)
  Should be changed back.
  (done in patch)


\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\Jdk13LumberjackLogger.java

Line 42: A since-tag of "1.1" was added, but the class existed in SCM when the previous version was released, but was not included in the jar. Should the since tag be there?


\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\LogFactoryImpl.java

Line 121: The sentence seems to not have been finished...
Lines 173-175: Could use the constants that were defined in lines 74-78.
Line 761,1322: Missing indentation
  (done in patch)
Line 1252: Should be removed, copy-and-paste error
  (done in patch)


\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\impl\SimpleLog.java

Line 398: The javadoc below
  @see org.apache.commons.logging.Log#trace(Object, Throwable)
should be
  @see org.apache.commons.logging.Log#trace(Object)
  (done in patch)

\commons-logging-1.1-RC10-src\src\java\org\apache\commons\logging\LogFactory.java
Line 142: Should be changed
  </code></pre>  --->  </pre></code>
  (done in patch)
Lines 335-340, 1130, 1219-1271: Indentation is wrong (is using tab-characters)
  (done in patch)
Line 574: Should this be converted into a @todo?
Line 1172: LogFactory.class.getClassLoader.load(name) ---> LogFactory.class.getClassLoader().load(name)
  (done in patch)
Line 1217: The last part of the JavaDocs are cut in the middle of a sentence. What should it say? Lines 1462, 1466: The file doesn't have to be called FACTORY_PROPERTIES, the name is given by the parameter "fileName". The method is private and is only called once with fileName=FACTORY_PROPERTIES. Should this be changed? Line 1477: OUTPUT_PROPERTY ---> DIAGNOSTICS_DEST_PROPERTY or perhaps "org.apache.commons.logging.diagnostics.dest". Which one is best?
Line 1655: Missing a space at the end of the string (after the colon)
  (done in patch)


\commons-logging-1.1-RC10-src\xdocs\guide.xml
Line 150: Remove " in each file".
  (done in patch)


\commons-logging-1.1-RC10-src\xdocs\tech.xml
Line 649:
"JCL uses the context classloader to use the <code>Log</code> implementation."
What should it be?
"JCL uses the context classloader to get|load the <code>Log</code> implementation."


\commons-logging-1.1-RC10-src\xdocs\troubleshooting.xml
Line 91:
"Each diagnostic message is prefixed with details of the class being logger in a standard format."
What should it be?
"Each diagnostic message is prefixed with details of the class being logged|loaded|diagnosed in a standard format."

--
Dennis Lundberg
Index: xdocs/guide.xml
===================================================================
--- xdocs/guide.xml     (revision 398696)
+++ xdocs/guide.xml     (arbetskopia)
@@ -147,7 +147,7 @@
 When such a file exists, every entry in the properties file becomes an 
"attribute"
 of the LogFactory. When there is more than one such file in the classpath, 
releases
 of commons-logging prior to 1.1 simply use the first one found. From release 
1.1,
-each file may define a <code>priority</code> key in each file, and the file 
with
+each file may define a <code>priority</code> key, and the file with
 the highest priority is used (no priority definition implies priority of zero).
 When multiple files have the same priority, the first one found is used.
 </p>
Index: src/java/org/apache/commons/logging/impl/AvalonLogger.java
===================================================================
--- src/java/org/apache/commons/logging/impl/AvalonLogger.java  (revision 
398661)
+++ src/java/org/apache/commons/logging/impl/AvalonLogger.java  (arbetskopia)
@@ -248,7 +248,7 @@
      * 
      * @param message to log.
      * @param t log this cause.
-     * @see org.apache.commons.logging.Log#debug(Object, Throwable)
+     * @see org.apache.commons.logging.Log#trace(Object, Throwable)
      */
     public void trace(Object message, Throwable t) {
         if (getLogger().isDebugEnabled()) 
getLogger().debug(String.valueOf(message), t);
Index: src/java/org/apache/commons/logging/impl/LogFactoryImpl.java
===================================================================
--- src/java/org/apache/commons/logging/impl/LogFactoryImpl.java        
(revision 398661)
+++ src/java/org/apache/commons/logging/impl/LogFactoryImpl.java        
(arbetskopia)
@@ -758,8 +758,7 @@
      * or if no adapter at all can be instantiated
      */
     private Log discoverLogImplementation(String logCategory)
-    throws LogConfigurationException
-    {
+            throws LogConfigurationException {
         if (isDiagnosticsEnabled()) {
             logDiagnostic("Discovering a Log implementation...");
         }
@@ -1248,8 +1247,7 @@
             current = current.getParent();
         }
        
-       // scan c2's ancestors to find c1
-        // scan c1's ancestors to find c2
+        // scan c2's ancestors to find c1
         current = c2;
         while (current != null) {
             if (current == c1)
@@ -1319,7 +1317,7 @@
      * should not be recovered from.
      */
     private void handleFlawedHierarchy(ClassLoader badClassLoader, Class 
badClass)
-    throws LogConfigurationException {
+            throws LogConfigurationException {
 
         boolean implementsLog = false;
         String logInterfaceName = Log.class.getName();
Index: src/java/org/apache/commons/logging/impl/SimpleLog.java
===================================================================
--- src/java/org/apache/commons/logging/impl/SimpleLog.java     (revision 
398661)
+++ src/java/org/apache/commons/logging/impl/SimpleLog.java     (arbetskopia)
@@ -395,7 +395,7 @@
      * <code>org.apache.commons.logging.impl.SimpleLog.LOG_LEVEL_TRACE</code>.
      *
      * @param message to log
-     * @see org.apache.commons.logging.Log#trace(Object, Throwable)
+     * @see org.apache.commons.logging.Log#trace(Object)
      */
     public final void trace(Object message) {
 
Index: src/java/org/apache/commons/logging/LogFactory.java
===================================================================
--- src/java/org/apache/commons/logging/LogFactory.java (revision 398661)
+++ src/java/org/apache/commons/logging/LogFactory.java (arbetskopia)
@@ -139,7 +139,7 @@
      * <strong>Note:</strong> <code>LogFactory</code> will print:
      * <code><pre>
      * [ERROR] LogFactory: Load of custom hashtable failed</em>
-     * </code></pre>
+     * </pre></code>
      * to system error and then continue using a standard Hashtable.
      * </p>
      * <p>
@@ -328,16 +328,16 @@
         } catch (Throwable t) {
             // ignore
             if (!WEAK_HASHTABLE_CLASSNAME.equals(storeImplementationClass)) {
-                   // if the user's trying to set up a custom implementation, 
give a clue
-                   if (isDiagnosticsEnabled()) {
-                       // use internal logging to issue the warning
-                       logDiagnostic("[ERROR] LogFactory: Load of custom 
hashtable failed");
-                       } else {
-                           // we *really* want this output, even if 
diagnostics weren't
+                // if the user's trying to set up a custom implementation, 
give a clue
+                if (isDiagnosticsEnabled()) {
+                    // use internal logging to issue the warning
+                    logDiagnostic("[ERROR] LogFactory: Load of custom 
hashtable failed");
+                } else {
+                    // we *really* want this output, even if diagnostics 
weren't
                     // explicitly enabled by the user.
-                           System.err.println("[ERROR] LogFactory: Load of 
custom hashtable failed");
-                       }
-               }
+                    System.err.println("[ERROR] LogFactory: Load of custom 
hashtable failed");
+                }
+            }
         }
         if (result == null) {
             result = new Hashtable();
@@ -1127,7 +1127,7 @@
                         // to their native logging system. 
                         // 
                         String msg = 
-                               "The application has specified that a custom 
LogFactory implementation should be used but " +
+                            "The application has specified that a custom 
LogFactory implementation should be used but " +
                             "Class '" + factoryClass + "' cannot be converted 
to '"
                             + LogFactory.class.getName() + "'. ";
                         if (implementsLogFactory) {
@@ -1169,7 +1169,7 @@
              * classLoader was unable to load factoryClass.
              *
              * In either case, we call Class.forName, which is equivalent
-             * to LogFactory.class.getClassLoader.load(name), ie we ignore
+             * to LogFactory.class.getClassLoader().load(name), ie we ignore
              * the classloader parameter the caller passed, and fall back
              * to trying the classloader associated with this class. See the
              * javadoc for the newFactory method for more info on the 
@@ -1216,59 +1216,59 @@
      * @param logFactoryClass <code>Class</code> which may implement 
<code>LogFactory</code>
      * @return true if the <code>Class</code> is assignable from the 
      */
-       private static boolean implementsLogFactory(Class logFactoryClass) {
-               boolean implementsLogFactory = false;
-               if (logFactoryClass != null) {
-                       try {
-                           ClassLoader logFactoryClassLoader = 
logFactoryClass.getClassLoader();
-                           if (logFactoryClassLoader == null) {
-                               logDiagnostic("[CUSTOM LOG FACTORY] was loaded 
by the boot classloader");
-                           } else {
-                               logHierarchy("[CUSTOM LOG FACTORY] ", 
logFactoryClassLoader);
-                               Class factoryFromCustomLoader 
-                                       = 
Class.forName("org.apache.commons.logging.LogFactory", false, 
logFactoryClassLoader);
-                               implementsLogFactory = 
factoryFromCustomLoader.isAssignableFrom(logFactoryClass);
-                               if (implementsLogFactory) {
-                                       logDiagnostic("[CUSTOM LOG FACTORY] " + 
logFactoryClass.getName() 
-                                                       + " implements 
LogFactory but was loaded by an incompatible classloader.");
-                               } else {
-                                       logDiagnostic("[CUSTOM LOG FACTORY] " + 
logFactoryClass.getName() 
-                                                       + " does not implement 
LogFactory.");
-                               }
-                           }
-                       } catch (SecurityException e) {
-                               //
-                               // The application is running within a hostile 
security environment.
-                               // This will make it very hard to diagnose 
issues with JCL.
-                               // Consider running less securely whilst 
debugging this issue.
-                               //
-                               logDiagnostic("[CUSTOM LOG FACTORY] 
SecurityException thrown whilst trying to determine whether " +
-                                               "the compatibility was caused 
by a classloader conflict: " 
-                                               + e.getMessage());
-                       } catch (LinkageError e) {
-                               //
-                               // This should be an unusual circumstance.
-                               // LinkageError's usually indicate that a 
dependent class has incompatibly changed.
-                               // Another possibility may be an exception 
thrown by an initializer.
-                               // Time for a clean rebuild?
-                               //
-                               logDiagnostic("[CUSTOM LOG FACTORY] 
LinkageError thrown whilst trying to determine whether " +
-                                               "the compatibility was caused 
by a classloader conflict: " 
-                                               + e.getMessage());
-                       } catch (ClassNotFoundException e) {
-                               //
-                               // LogFactory cannot be loaded by the 
classloader which loaded the custom factory implementation.
-                               // The custom implementation is not viable 
until this is corrected. 
-                               // Ensure that the JCL jar and the custom class 
are available from the same classloader.
-                               // Running with diagnostics on should give 
information about the classloaders used 
-                               // to load the custom factory.
-                               // 
-                               logDiagnostic("[CUSTOM LOG FACTORY] LogFactory 
class cannot be loaded by classloader which loaded the " +
-                                               "custom LogFactory 
implementation. Is the custom factory in the right classloader?");
-                       }
-               }
-               return implementsLogFactory;
-       }
+    private static boolean implementsLogFactory(Class logFactoryClass) {
+        boolean implementsLogFactory = false;
+        if (logFactoryClass != null) {
+            try {
+                ClassLoader logFactoryClassLoader = 
logFactoryClass.getClassLoader();
+                if (logFactoryClassLoader == null) {
+                    logDiagnostic("[CUSTOM LOG FACTORY] was loaded by the boot 
classloader");
+                } else {
+                    logHierarchy("[CUSTOM LOG FACTORY] ", 
logFactoryClassLoader);
+                    Class factoryFromCustomLoader
+                        = 
Class.forName("org.apache.commons.logging.LogFactory", false, 
logFactoryClassLoader);
+                    implementsLogFactory = 
factoryFromCustomLoader.isAssignableFrom(logFactoryClass);
+                    if (implementsLogFactory) {
+                        logDiagnostic("[CUSTOM LOG FACTORY] " + 
logFactoryClass.getName()
+                                        + " implements LogFactory but was 
loaded by an incompatible classloader.");
+                    } else {
+                        logDiagnostic("[CUSTOM LOG FACTORY] " + 
logFactoryClass.getName()
+                                        + " does not implement LogFactory.");
+                    }
+                }
+            } catch (SecurityException e) {
+                //
+                // The application is running within a hostile security 
environment.
+                // This will make it very hard to diagnose issues with JCL.
+                // Consider running less securely whilst debugging this issue.
+                //
+                logDiagnostic("[CUSTOM LOG FACTORY] SecurityException thrown 
whilst trying to determine whether " +
+                                "the compatibility was caused by a classloader 
conflict: "
+                                + e.getMessage());
+            } catch (LinkageError e) {
+                //
+                // This should be an unusual circumstance.
+                // LinkageError's usually indicate that a dependent class has 
incompatibly changed.
+                // Another possibility may be an exception thrown by an 
initializer.
+                // Time for a clean rebuild?
+                //
+                logDiagnostic("[CUSTOM LOG FACTORY] LinkageError thrown whilst 
trying to determine whether " +
+                                "the compatibility was caused by a classloader 
conflict: "
+                                + e.getMessage());
+            } catch (ClassNotFoundException e) {
+                //
+                // LogFactory cannot be loaded by the classloader which loaded 
the custom factory implementation.
+                // The custom implementation is not viable until this is 
corrected.
+                // Ensure that the JCL jar and the custom class are available 
from the same classloader.
+                // Running with diagnostics on should give information about 
the classloaders used
+                // to load the custom factory.
+                //
+                logDiagnostic("[CUSTOM LOG FACTORY] LogFactory class cannot be 
loaded by classloader which loaded the " +
+                                "custom LogFactory implementation. Is the 
custom factory in the right classloader?");
+            }
+        }
+        return implementsLogFactory;
+    }
 
     /**
      * Applets may run in an environment where accessing resources of a loader 
is
@@ -1652,7 +1652,7 @@
             return;
         }        
         if (classLoader != null) {
-            StringBuffer buf = new StringBuffer(prefix + "ClassLoader tree:");
+            StringBuffer buf = new StringBuffer(prefix + "ClassLoader tree: ");
             for(;;) {
                 buf.append(objectId(classLoader));
                 if (classLoader == systemClassLoader) {

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to