You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/03/28 23:18:48 UTC

[GitHub] [logging-log4j2] rgoers opened a new pull request #480: Master java11

rgoers opened a new pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480


   Moves master to Java11. 
   
   1. Removes all -java9 modules.
   2. Required version upgrades to lots of stuff.
   3. Log4j-api and Log4j-plugins are true JPMS modules. Not much else is yet. Getting a test-jar created that is usable by down-stream components required doing some heaving hacking since the classes in the test jars being passed to downstream modules cannot be in the same package as the component they originated from when the modulepath is being used. Any classes that belong in a test jar are now placed in a java-test directory that is compiled and packaged before the "normal" test classes are compiled. This work has not been done for log4j-core.
   2. PreciseClock is not supported. When enabled SystemClock causes the zero GC tests to fail on everything due to Instants being created for the timestamp being logged.
   3. tests in log4j-osgi are disabled. They fail and I have no idea why.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877755799


   You would have to wade through the developers mailing list. We've talked about creating confluence pages but never done it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] remkop commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
remkop commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877473666


   My understanding is that we will use the Java 11 compiler to generate Java-8 bytecode, so Log4j2 can be used with Java 8 at runtime.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] perry2of5 commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
perry2of5 commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-812102267


   Building on windows, Zulu Java 11, Maven 3.6.3, I see this warning in log4j-api, log4j-plugins, and others:
   [WARNING] * Required filename-based automodules detected: [org.osgi.core-6.0.0.jar]. Please don't publish this project to a public artifact repository! 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rschmitt commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rschmitt commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877472722


   Just to confirm: does this mean that the next version of log4j2 will drop support for JDK8?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877526559


   Yup, Gary is correct. The 2.x releases will remain on Java 8. The 3.0 releases will require Java 11.
   
   Ralph
   
   > On Jul 9, 2021, at 11:52 AM, Remko Popma ***@***.***> wrote:
   > 
   > 
   > My bad, thanks Gary!
   > 
   > —
   > You are receiving this because you modified the open/close state.
   > Reply to this email directly, view it on GitHub <https://github.com/apache/logging-log4j2/pull/480#issuecomment-877475258>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA5MXY6SN3FC2VRFG2W3KDTW5VSJANCNFSM4Z6Q6M6Q>.
   > 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] remkop commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
remkop commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877475258


   My bad, thanks Gary!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] garydgregory commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877474665


   > My understanding is that we will use the Java 11 compiler to generate Java-8 bytecode, so Log4j2 can be used with Java 8 at runtime.
   
   I do not think so, 2.x will remain Java 8, but 3.0 in master will be Java 11 only.
   
   You should never use Java 11 to compile to Java 8 unless you are compiling against the JRE 8 jars (which I am not sure makes sense) because Java has changed some API signatures in the NIO/Buffer area.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers merged pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers merged pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-812125020


   @perry2of5 Thanks. That is likely related to why I couldn't get the log4j-osgi to run its tests. I'll have to figure out what that warning means and fix it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on a change in pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on a change in pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#discussion_r603718254



##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/time/internal/SystemClock.java
##########
@@ -16,13 +16,19 @@
  */
 package org.apache.logging.log4j.core.time.internal;
 
+import java.time.Instant;
+
 import org.apache.logging.log4j.core.time.Clock;
+import org.apache.logging.log4j.core.time.MutableInstant;
+import org.apache.logging.log4j.core.time.PreciseClock;
 
 /**
  * Implementation of the {@code Clock} interface that returns the system time.
+ * @since 2.11.0
  */
+// Precise clock is not implemented because the instant() method in the init method is not garbage free.

Review comment:
       I agree. If it is possible to detect that we are not in "garbage free mode" then we could go ahead with the init method. I admittedly took a shortcut here as I spent nearly a week getting this done and I didn't want to solve every problem in Log4j.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers edited a comment on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers edited a comment on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-812125020


   @perry2of5 Thanks. That is likely related to why I couldn't get the log4j-osgi module to run its tests. I'll have to figure out what that warning means and fix it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on a change in pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on a change in pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#discussion_r603790813



##########
File path: log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
##########
@@ -16,75 +16,23 @@
  */
 package org.apache.logging.log4j.util;
 
-import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Optional;
 import java.util.Stack;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
- * <em>Consider this class private.</em> Provides various methods to determine the caller class. <h3>Background</h3>
- * <p>
- * This method, available only in the Oracle/Sun/OpenJDK implementations of the Java Virtual Machine, is a much more
- * efficient mechanism for determining the {@link Class} of the caller of a particular method. When it is not available,
- * a {@link SecurityManager} is the second-best option. When this is also not possible, the {@code StackTraceElement[]}
- * returned by {@link Throwable#getStackTrace()} must be used, and its {@code String} class name converted to a
- * {@code Class} using the slow {@link Class#forName} (which can add an extra microsecond or more for each invocation
- * depending on the runtime ClassLoader hierarchy).
- * </p>
- * <p>
- * During Java 8 development, the {@code sun.reflect.Reflection.getCallerClass(int)} was removed from OpenJDK, and this
- * change was back-ported to Java 7 in version 1.7.0_25 which changed the behavior of the call and caused it to be off
- * by one stack frame. This turned out to be beneficial for the survival of this API as the change broke hundreds of
- * libraries and frameworks relying on the API which brought much more attention to the intended API removal.
- * </p>
- * <p>
- * After much community backlash, the JDK team agreed to restore {@code getCallerClass(int)} and keep its existing
- * behavior for the rest of Java 7. However, the method is deprecated in Java 8, and current Java 9 development has not
- * addressed this API. Therefore, the functionality of this class cannot be relied upon for all future versions of Java.
- * It does, however, work just fine in Sun JDK 1.6, OpenJDK 1.6, Oracle/OpenJDK 1.7, and Oracle/OpenJDK 1.8. Other Java
- * environments may fall back to using {@link Throwable#getStackTrace()} which is significantly slower due to
- * examination of every virtual frame of execution.
- * </p>
+ * <em>Consider this class private.</em> Determines the caller's class.
  */
-public final class StackLocator {
+public class StackLocator {

Review comment:
       Interesting. I didn't actually modify the class. Just moved the version from the Java9 branch in. I wonder why this and the @PerformanceSensitive annotation weren't ever added to that.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#discussion_r603671718



##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/time/internal/SystemClock.java
##########
@@ -16,13 +16,19 @@
  */
 package org.apache.logging.log4j.core.time.internal;
 
+import java.time.Instant;
+
 import org.apache.logging.log4j.core.time.Clock;
+import org.apache.logging.log4j.core.time.MutableInstant;
+import org.apache.logging.log4j.core.time.PreciseClock;
 
 /**
  * Implementation of the {@code Clock} interface that returns the system time.
+ * @since 2.11.0
  */
+// Precise clock is not implemented because the instant() method in the init method is not garbage free.

Review comment:
       Hmm, this would result in a regression in some places, we use microsecond precision everywhere at this point at work since events can be scrambled within millisecond buckets by our indexer. I suspect we'd be better of implementing PreciseClock and updating our tests, but I have a very specific use case. Either way we should swing back to add another clock implementation to support whichever implementation SystemClock does not.

##########
File path: log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
##########
@@ -93,158 +41,55 @@ public static StackLocator getInstance() {
     private StackLocator() {
     }
 
-    // TODO: return Object.class instead of null (though it will have a null ClassLoader)
-    // (MS) I believe this would work without any modifications elsewhere, but I could be wrong
-
-    // migrated from ReflectiveCallerClassUtility
-    @PerformanceSensitive
-    public Class<?> getCallerClass(final int depth) {
-        if (depth < 0) {
-            throw new IndexOutOfBoundsException(Integer.toString(depth));
-        }
-        if (GET_CALLER_CLASS == null) {
-            return null;
-        }
-        // note that we need to add 1 to the depth value to compensate for this method, but not for the Method.invoke
-        // since Reflection.getCallerClass ignores the call to Method.invoke()
-        try {
-            return (Class<?>) GET_CALLER_CLASS.invoke(null, depth + 1 + JDK_7u25_OFFSET);
-        } catch (final Exception e) {
-            // theoretically this could happen if the caller class were native code
-            // TODO: return Object.class
-            return null;
-        }
+    public Class<?> getCallerClass(final String fqcn) {
+        return getCallerClass(fqcn, "");
     }
 
-    // migrated from Log4jLoggerFactory
-    @PerformanceSensitive
     public Class<?> getCallerClass(final String fqcn, final String pkg) {
         return getCallerClass(fqcn, pkg, 0);
     }
 
-    @PerformanceSensitive

Review comment:
       Can we retain these annotations?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rgoers commented on a change in pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rgoers commented on a change in pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#discussion_r603718254



##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/time/internal/SystemClock.java
##########
@@ -16,13 +16,19 @@
  */
 package org.apache.logging.log4j.core.time.internal;
 
+import java.time.Instant;
+
 import org.apache.logging.log4j.core.time.Clock;
+import org.apache.logging.log4j.core.time.MutableInstant;
+import org.apache.logging.log4j.core.time.PreciseClock;
 
 /**
  * Implementation of the {@code Clock} interface that returns the system time.
+ * @since 2.11.0
  */
+// Precise clock is not implemented because the instant() method in the init method is not garbage free.

Review comment:
       I agree. If it is possible to detect that we are not in "garbage free mode" then we could go ahead with the init method. I admittedly took a shortcut has I spent nearly a week getting this done and I didn't want to solve every problem in Log4j.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#discussion_r603666062



##########
File path: log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java
##########
@@ -16,75 +16,23 @@
  */
 package org.apache.logging.log4j.util;
 
-import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Optional;
 import java.util.Stack;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
- * <em>Consider this class private.</em> Provides various methods to determine the caller class. <h3>Background</h3>
- * <p>
- * This method, available only in the Oracle/Sun/OpenJDK implementations of the Java Virtual Machine, is a much more
- * efficient mechanism for determining the {@link Class} of the caller of a particular method. When it is not available,
- * a {@link SecurityManager} is the second-best option. When this is also not possible, the {@code StackTraceElement[]}
- * returned by {@link Throwable#getStackTrace()} must be used, and its {@code String} class name converted to a
- * {@code Class} using the slow {@link Class#forName} (which can add an extra microsecond or more for each invocation
- * depending on the runtime ClassLoader hierarchy).
- * </p>
- * <p>
- * During Java 8 development, the {@code sun.reflect.Reflection.getCallerClass(int)} was removed from OpenJDK, and this
- * change was back-ported to Java 7 in version 1.7.0_25 which changed the behavior of the call and caused it to be off
- * by one stack frame. This turned out to be beneficial for the survival of this API as the change broke hundreds of
- * libraries and frameworks relying on the API which brought much more attention to the intended API removal.
- * </p>
- * <p>
- * After much community backlash, the JDK team agreed to restore {@code getCallerClass(int)} and keep its existing
- * behavior for the rest of Java 7. However, the method is deprecated in Java 8, and current Java 9 development has not
- * addressed this API. Therefore, the functionality of this class cannot be relied upon for all future versions of Java.
- * It does, however, work just fine in Sun JDK 1.6, OpenJDK 1.6, Oracle/OpenJDK 1.7, and Oracle/OpenJDK 1.8. Other Java
- * environments may fall back to using {@link Throwable#getStackTrace()} which is significantly slower due to
- * examination of every virtual frame of execution.
- * </p>
+ * <em>Consider this class private.</em> Determines the caller's class.
  */
-public final class StackLocator {
+public class StackLocator {

Review comment:
       Might as well keep around the `final` modifier.
   ```suggestion
   public final class StackLocator {
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [logging-log4j2] rschmitt commented on pull request #480: Master java11

Posted by GitBox <gi...@apache.org>.
rschmitt commented on pull request #480:
URL: https://github.com/apache/logging-log4j2/pull/480#issuecomment-877734105


   Where can I read more about the plans for 3.0?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org