[PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy opened a new pull request, #3073: URL: https://github.com/apache/logging-log4j2/pull/3073 Ports #3045 to `main`. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable automatic Android tests [logging-log4j-samples]
ppkarwasz commented on code in PR #198:
URL:
https://github.com/apache/logging-log4j-samples/pull/198#discussion_r1794180099
##
log4j-samples-android/gradlew:
##
@@ -17,67 +17,101 @@
#
##
-##
-## Gradle start up script for UN*X
-##
+#
+# Gradle start up script for POSIX generated by Gradle.
+#
+# Important for running:
+#
+# (1) You need a POSIX-compliant shell to run this script. If your /bin/sh is
+# noncompliant, but you have some other compliant shell such as ksh or
+# bash, then to run this script, type that shell name before the whole
+# command line, like:
+#
+# ksh Gradle
+#
+# Busybox and similar reduced shells will NOT work, because this script
+# requires all of these POSIX shell features:
+# * functions;
+# * expansions «$var», «${var}», «${var:-default}», «${var+SET}»,
+# «${var#prefix}», «${var%suffix}», and «$( cmd )»;
+# * compound commands having a testable exit status, especially «case»;
+# * various built-in commands including «command», «set», and «ulimit».
+#
+# Important for patching:
+#
+# (2) This script targets any POSIX shell, so it avoids extensions provided
+# by Bash, Ksh, etc; in particular arrays are avoided.
+#
+# The "traditional" practice of packing multiple parameters into a
+# space-separated string is a well documented source of bugs and security
+# problems, so this is (mostly) avoided, by progressively accumulating
+# options in "$@", and eventually passing that to Java.
+#
+# Where the inherited environment variables (DEFAULT_JVM_OPTS, JAVA_OPTS,
+# and GRADLE_OPTS) rely on word-splitting, this is performed explicitly;
+# see the in-line comments for details.
+#
+# There are tweaks for specific operating systems such as AIX, CygWin,
+# Darwin, MinGW, and NonStop.
+#
+# (3) This script is generated from the Groovy template
+#
https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
+# within the Gradle project.
+#
+# You can find Gradle at https://github.com/gradle/gradle/.
+#
##
# Attempt to set APP_HOME
+
# Resolve links: $0 may be a link
-PRG="$0"
-# Need this for relative symlinks.
-while [ -h "$PRG" ] ; do
-ls=`ls -ld "$PRG"`
-link=`expr "$ls" : '.*-> \(.*\)$'`
-if expr "$link" : '/.*' > /dev/null; then
-PRG="$link"
-else
-PRG=`dirname "$PRG"`"/$link"
-fi
+app_path=$0
+
+# Need this for daisy-chained symlinks.
+while
+APP_HOME=${app_path%"${app_path##*/}"} # leaves a trailing /; empty if no
leading path
+[ -h "$app_path" ]
+do
+ls=$( ls -ld "$app_path" )
+link=${ls#*' -> '}
+case $link in #(
+ /*) app_path=$link ;; #(
+ *)app_path=$APP_HOME$link ;;
+esac
done
-SAVED="`pwd`"
-cd "`dirname \"$PRG\"`/" >/dev/null
-APP_HOME="`pwd -P`"
-cd "$SAVED" >/dev/null
+
+APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit
APP_NAME="Gradle"
-APP_BASE_NAME=`basename "$0"`
+APP_BASE_NAME=${0##*/}
# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to
pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
# Use the maximum available, or set MAX_FD != -1 to use that value.
-MAX_FD="maximum"
+MAX_FD=maximum
warn () {
echo "$*"
-}
+} >&2
die () {
echo
echo "$*"
echo
exit 1
-}
+} >&2
# OS specific support (must be 'true' or 'false').
cygwin=false
msys=false
darwin=false
nonstop=false
-case "`uname`" in
- CYGWIN* )
-cygwin=true
-;;
- Darwin* )
-darwin=true
-;;
- MINGW* )
-msys=true
-;;
- NONSTOP* )
-nonstop=true
-;;
+case "$( uname )" in#(
Review Comment:
I don't know, maybe Gradle developers like to end each line with `#(`.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix Android-related issues in Log4j Core (logging-log4j2)
ppkarwasz commented on code in PR #3071:
URL: https://github.com/apache/logging-log4j2/pull/3071#discussion_r1794184423
##
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java:
##
@@ -31,17 +34,16 @@
@Plugin(name = "jvmrunargs", category = StrLookup.CATEGORY)
public class JmxRuntimeInputArgumentsLookup extends MapLookup {
-static {
-final List argsList =
ManagementFactory.getRuntimeMXBean().getInputArguments();
-JMX_SINGLETON = new
JmxRuntimeInputArgumentsLookup(MapLookup.toMap(argsList));
-}
+private static final Logger LOGGER = StatusLogger.getLogger();
-public static final JmxRuntimeInputArgumentsLookup JMX_SINGLETON;
+public static final JmxRuntimeInputArgumentsLookup JMX_SINGLETON = new
JmxRuntimeInputArgumentsLookup();
/**
* Constructor when used directly as a plugin.
*/
-public JmxRuntimeInputArgumentsLookup() {}
+public JmxRuntimeInputArgumentsLookup() {
Review Comment:
This is not possible, since lookups are instantiated through a public no-arg
constructor:
https://github.com/apache/logging-log4j2/blob/ab0fdbc51742646144f6ad29715f9bd1d9afa985/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java#L84-L95
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Upgrades JUnit to version `5.11.2` (logging-log4j2)
ppkarwasz commented on code in PR #3069:
URL: https://github.com/apache/logging-log4j2/pull/3069#discussion_r1794129224
##
log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/junit/package-info.java:
##
@@ -20,7 +20,7 @@
* @see org.junit.rules.TestRule
*/
@Export
-@Version("2.23.0")
+@Version("2.23.1")
Review Comment:
Why? It is a `MICRO` change, so I only increased the patch number.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Use a random `${test:logging.path}` (logging-log4j2)
ppkarwasz commented on code in PR #2954:
URL: https://github.com/apache/logging-log4j2/pull/2954#discussion_r1794131668
##
log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java:
##
@@ -135,6 +132,29 @@ private PathHolder createLoggingPath(final
ExtensionContext context, final Clean
return holder;
}
+private Path determinePerClassPath(ExtensionContext context) {
+// Check if the parent context already created a folder
+PathHolder holder =
ExtensionContextAnchor.getAttribute(PathHolder.class, PathHolder.class,
context);
+if (holder == null) {
+try {
+// Create temporary per-class directory
+final String baseDir = System.getProperty("basedir");
+final Path basePath = (baseDir != null ? Paths.get(baseDir,
"target") : Paths.get(".")).resolve("logs");
+final Class clazz = context.getRequiredTestClass();
+final Package pkg = clazz.getPackage();
+final String dir =
+pkg.getName().replaceAll("[.$]", File.separatorChar ==
'\\' ? "" : File.separator);
+// Create a temporary directory that uses the simple class
name as prefix
+Path packagePath = basePath.resolve(dir);
+Files.createDirectories(packagePath);
+return Files.createTempDirectory(packagePath,
clazz.getSimpleName());
Review Comment:
Aren't those reruns happening inside the same JVM?
**Note**: the directories are deleted by default if the test is successful,
so we are only looking at failed runs. I wouldn't want to have too long names.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Document GraalVM usage with Log4j (logging-log4j2)
ppkarwasz merged PR #3054: URL: https://github.com/apache/logging-log4j2/pull/3054 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Provide guidelines and templates for the release process [logging-log4cxx]
ppkarwasz commented on code in PR #413: URL: https://github.com/apache/logging-log4cxx/pull/413#discussion_r1794137931 ## admin/releasing.md: ## @@ -0,0 +1,68 @@ +Releasing a Log4cxx version +=== + +This document lists the steps that must be performed to release Log4cxx +using 1.3.0 as the example. + +Prerequisites +-- + +* The version number (in src/cmake/projectVersionDetails.cmake) has been updated +* The change log (in src/site/markdown/change-report-gh.md) is up-to-date. +* The web-site for the new version has been published to https://logging.staged.apache.org/log4cxx +* An e-mail has been sent to d...@logging.apache.org announcing the intention to release +* Your public key is available in [Apache Logging KEYS file](https://dist.apache.org/repos/dist/release/logging/KEYS) + +Steps +- + +1. Tag HEAD as the release candidate +- `git checkout master` +- `git push origin tag v1.3.0-RC1` +1. Remove the old artifacts from svn +- `svn co https://dist.apache.org/repos/dist/dev/logging -N apache-dist-logging-dev` +- `cd apache-dist-logging-dev` +- `svn up log4cxx` +- `cd log4cxx` +- `svn delete *` +1. Download the latest release artifacts +- The link to the packaged artifacts is available + in the Github action log under the "Run action/upload-artifact" step of + the "Generate release files" Github action. + The log of the most recent commit can be accessed + using the "Details" link in the pop-up window shown + when the green tick is clicked. +- `cd apache-dist-logging-dev/log4cxx` +- `unzip "~/Downloads/Upload release files.zip"` +1. Sign release artifacts (Refer: https://infra.apache.org/release-signing.html) Review Comment: [INFRA-26194](https://issues.apache.org/jira/browse/INFRA-26194) added secrets to our runners, so we can do those things automatically, i.e.: 1. Sign the archives on the Github runner. 2. Commit everything to the `dev` subversion repository. I can try updating the workflows tomorrow. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix Android-related issues in Log4j Core (logging-log4j2)
vy commented on code in PR #3071:
URL: https://github.com/apache/logging-log4j2/pull/3071#discussion_r1793523300
##
log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JmxRuntimeInputArgumentsLookup.java:
##
@@ -31,17 +34,16 @@
@Plugin(name = "jvmrunargs", category = StrLookup.CATEGORY)
public class JmxRuntimeInputArgumentsLookup extends MapLookup {
-static {
-final List argsList =
ManagementFactory.getRuntimeMXBean().getInputArguments();
-JMX_SINGLETON = new
JmxRuntimeInputArgumentsLookup(MapLookup.toMap(argsList));
-}
+private static final Logger LOGGER = StatusLogger.getLogger();
-public static final JmxRuntimeInputArgumentsLookup JMX_SINGLETON;
+public static final JmxRuntimeInputArgumentsLookup JMX_SINGLETON = new
JmxRuntimeInputArgumentsLookup();
/**
* Constructor when used directly as a plugin.
*/
-public JmxRuntimeInputArgumentsLookup() {}
+public JmxRuntimeInputArgumentsLookup() {
Review Comment:
*Nit:* Can't we instead have a `@PluginFactory` returning the singleton
instance?
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] Enable automatic Android tests [logging-log4j-samples]
ppkarwasz opened a new pull request, #198: URL: https://github.com/apache/logging-log4j-samples/pull/198 This PR is a work in progress to enable automatic tests of Log4j Core on Android. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Extract Nexus URL using `nexus-staging-maven-plugin` [logging-parent]
vy merged PR #246: URL: https://github.com/apache/logging-parent/pull/246 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Enable automatic Android tests [logging-log4j-samples]
vy commented on code in PR #198:
URL:
https://github.com/apache/logging-log4j-samples/pull/198#discussion_r1793507975
##
log4j-samples-android/gradlew:
##
@@ -17,67 +17,101 @@
#
##
-##
-## Gradle start up script for UN*X
-##
+#
+# Gradle start up script for POSIX generated by Gradle.
+#
+# Important for running:
+#
+# (1) You need a POSIX-compliant shell to run this script. If your /bin/sh is
+# noncompliant, but you have some other compliant shell such as ksh or
+# bash, then to run this script, type that shell name before the whole
+# command line, like:
+#
+# ksh Gradle
+#
+# Busybox and similar reduced shells will NOT work, because this script
+# requires all of these POSIX shell features:
+# * functions;
+# * expansions «$var», «${var}», «${var:-default}», «${var+SET}»,
+# «${var#prefix}», «${var%suffix}», and «$( cmd )»;
+# * compound commands having a testable exit status, especially «case»;
+# * various built-in commands including «command», «set», and «ulimit».
+#
+# Important for patching:
+#
+# (2) This script targets any POSIX shell, so it avoids extensions provided
+# by Bash, Ksh, etc; in particular arrays are avoided.
+#
+# The "traditional" practice of packing multiple parameters into a
+# space-separated string is a well documented source of bugs and security
+# problems, so this is (mostly) avoided, by progressively accumulating
+# options in "$@", and eventually passing that to Java.
+#
+# Where the inherited environment variables (DEFAULT_JVM_OPTS, JAVA_OPTS,
+# and GRADLE_OPTS) rely on word-splitting, this is performed explicitly;
+# see the in-line comments for details.
+#
+# There are tweaks for specific operating systems such as AIX, CygWin,
+# Darwin, MinGW, and NonStop.
+#
+# (3) This script is generated from the Groovy template
+#
https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
+# within the Gradle project.
+#
+# You can find Gradle at https://github.com/gradle/gradle/.
+#
##
# Attempt to set APP_HOME
+
# Resolve links: $0 may be a link
-PRG="$0"
-# Need this for relative symlinks.
-while [ -h "$PRG" ] ; do
-ls=`ls -ld "$PRG"`
-link=`expr "$ls" : '.*-> \(.*\)$'`
-if expr "$link" : '/.*' > /dev/null; then
-PRG="$link"
-else
-PRG=`dirname "$PRG"`"/$link"
-fi
+app_path=$0
+
+# Need this for daisy-chained symlinks.
+while
+APP_HOME=${app_path%"${app_path##*/}"} # leaves a trailing /; empty if no
leading path
+[ -h "$app_path" ]
+do
+ls=$( ls -ld "$app_path" )
+link=${ls#*' -> '}
+case $link in #(
+ /*) app_path=$link ;; #(
+ *)app_path=$APP_HOME$link ;;
+esac
done
-SAVED="`pwd`"
-cd "`dirname \"$PRG\"`/" >/dev/null
-APP_HOME="`pwd -P`"
-cd "$SAVED" >/dev/null
+
+APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit
APP_NAME="Gradle"
-APP_BASE_NAME=`basename "$0"`
+APP_BASE_NAME=${0##*/}
# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to
pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
# Use the maximum available, or set MAX_FD != -1 to use that value.
-MAX_FD="maximum"
+MAX_FD=maximum
warn () {
echo "$*"
-}
+} >&2
die () {
echo
echo "$*"
echo
exit 1
-}
+} >&2
# OS specific support (must be 'true' or 'false').
cygwin=false
msys=false
darwin=false
nonstop=false
-case "`uname`" in
- CYGWIN* )
-cygwin=true
-;;
- Darwin* )
-darwin=true
-;;
- MINGW* )
-msys=true
-;;
- NONSTOP* )
-nonstop=true
-;;
+case "$( uname )" in#(
Review Comment:
What are these `#(` suffixes for?
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
ppkarwasz commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793558443
##
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRendererFactory.java:
##
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.pattern;
+
+import org.apache.logging.log4j.core.impl.ThrowableFormatOptions;
+
+/**
+ * A {@link ThrowableStackTraceRenderer} factory contract.
+ */
+class ThrowableStackTraceRendererFactory {
+
+static final ThrowableStackTraceRendererFactory INSTANCE = new
ThrowableStackTraceRendererFactory();
+
+ThrowableStackTraceRendererFactory() {}
Review Comment:
Since the class is private, it doesn't matter. :wink:
I guess it is just a question of style.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
ppkarwasz commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r179381
##
log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java:
##
@@ -64,20 +63,15 @@ public void testParametersAreNotLeaked() throws Exception {
final String line1 = reader.readLine();
final String line2 = reader.readLine();
final String line3 = reader.readLine();
-// line4 is empty line because of the line separator after throwable
pattern
final String line4 = reader.readLine();
final String line5 = reader.readLine();
-final String line6 = reader.readLine();
-final String line7 = reader.readLine();
reader.close();
-file.delete();
+// file.delete();
assertThat(line1, containsString("Message with parameter paramValue"));
assertThat(line2, containsString("paramValue"));
assertThat(line3, containsString("paramValue"));
-assertThat(line4, is(""));
-assertThat(line5, containsString("paramValue"));
-assertThat(line6, is(""));
-assertNull(line7, "Expected only six lines");
+assertThat(line4, containsString("paramValue"));
+assertNull(line5, "Expected only six lines");
Review Comment:
Why do we still need some `startsWith` assertions?
https://github.com/apache/logging-log4j2/blob/2022db7d3910bcd07b006452efc12e99482ca14c/log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java#L62-L66
These prevent us from using `containsExactly(CharSequence...)`.
If it is complicated, don't bother. This looks good enough to me.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793563712
##
log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java:
##
@@ -64,20 +63,15 @@ public void testParametersAreNotLeaked() throws Exception {
final String line1 = reader.readLine();
final String line2 = reader.readLine();
final String line3 = reader.readLine();
-// line4 is empty line because of the line separator after throwable
pattern
final String line4 = reader.readLine();
final String line5 = reader.readLine();
-final String line6 = reader.readLine();
-final String line7 = reader.readLine();
reader.close();
-file.delete();
+// file.delete();
assertThat(line1, containsString("Message with parameter paramValue"));
assertThat(line2, containsString("paramValue"));
assertThat(line3, containsString("paramValue"));
-assertThat(line4, is(""));
-assertThat(line5, containsString("paramValue"));
-assertThat(line6, is(""));
-assertNull(line7, "Expected only six lines");
+assertThat(line4, containsString("paramValue"));
+assertNull(line5, "Expected only six lines");
Review Comment:
Yeah, I didn't want to bother serializing the exception with `Writer`s,
`PrintStream`s, etc. (Exception serialization checks have already been
extensively revamped anyway in #2691.)
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy merged PR #3045: URL: https://github.com/apache/logging-log4j2/pull/3045 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Provide guidelines and templates for the release process [logging-log4cxx]
rm5248 commented on PR #413: URL: https://github.com/apache/logging-log4cxx/pull/413#issuecomment-2402127948 Are these the review instructions you're talking about? https://lists.apache.org/thread/4vr3k0jfdn8rtl2ck1p0gw9y2gj6tckv -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Provide guidelines and templates for the release process [logging-log4cxx]
rm5248 commented on code in PR #413: URL: https://github.com/apache/logging-log4cxx/pull/413#discussion_r1793397109 ## admin/releasing.md: ## @@ -0,0 +1,68 @@ +Releasing a Log4cxx version +=== + +This document lists the steps that must be performed to release Log4cxx +using 1.3.0 as the example. + +Prerequisites +-- + +* The version number (in src/cmake/projectVersionDetails.cmake) has been updated +* The change log (in src/site/markdown/change-report-gh.md) is up-to-date. +* The web-site for the new version has been published to https://logging.staged.apache.org/log4cxx +* An e-mail has been sent to d...@logging.apache.org announcing the intention to release +* Your public key is available in [Apache Logging KEYS file](https://dist.apache.org/repos/dist/release/logging/KEYS) + +Steps +- + +1. Tag HEAD as the release candidate +- `git checkout master` +- `git push origin tag v1.3.0-RC1` +1. Remove the old artifacts from svn +- `svn co https://dist.apache.org/repos/dist/dev/logging -N apache-dist-logging-dev` +- `cd apache-dist-logging-dev` +- `svn up log4cxx` +- `cd log4cxx` +- `svn delete *` +1. Download the latest release artifacts +- The link to the packaged artifacts is available + in the Github action log under the "Run action/upload-artifact" step of + the "Generate release files" Github action. + The log of the most recent commit can be accessed + using the "Details" link in the pop-up window shown + when the green tick is clicked. +- `cd apache-dist-logging-dev/log4cxx` +- `unzip "~/Downloads/Upload release files.zip"` +1. Sign release artifacts (Refer: https://infra.apache.org/release-signing.html) Review Comment: These two steps here(`Download the latest release artifacts` and `Sign release artifacts`) should be automatically handled for you if you do `-DAPACHE_MAINTAINER=ON`; I know you changed something about that recently and I haven't gone back and checked to make sure that it still works. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump actions/cache from 4.1.0 to 4.1.1 [logging-parent]
github-actions[bot] merged PR #263: URL: https://github.com/apache/logging-parent/pull/263 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Bump site-logback.version from 1.4.14 to 1.5.9 (logging-log4j2)
github-actions[bot] merged PR #3072: URL: https://github.com/apache/logging-log4j2/pull/3072 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793306069
##
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableStackTraceRendererFactory.java:
##
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.pattern;
+
+import org.apache.logging.log4j.core.impl.ThrowableFormatOptions;
+
+/**
+ * A {@link ThrowableStackTraceRenderer} factory contract.
+ */
+class ThrowableStackTraceRendererFactory {
+
+static final ThrowableStackTraceRendererFactory INSTANCE = new
ThrowableStackTraceRendererFactory();
+
+ThrowableStackTraceRendererFactory() {}
Review Comment:
Package-private is more restrictive than `protected` – we shouldn't loosen
the accessibility without a use case.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793306440
##
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowablePropertyRendererFactory.java:
##
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.pattern;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+import org.apache.logging.log4j.core.impl.ThrowableFormatOptions;
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
+
+/**
+ * A factory of {@link ThrowableRenderer} implementations for extracting
certain properties from a {@link Throwable}.
+ */
+@NullMarked
+class ThrowablePropertyRendererFactory {
+
+private static final ThrowableRenderer MESSAGE_RENDERER = (buffer,
throwable, lineSeparator) -> {
+final String message = throwable.getMessage();
+buffer.append(message);
+};
+
+private static final ThrowableRenderer LOCALIZED_MESSAGE_RENDERER =
(buffer, throwable, lineSeparator) -> {
+final String localizedMessage = throwable.getLocalizedMessage();
+buffer.append(localizedMessage);
+};
+
+private static final Function
THROWING_METHOD_EXTRACTOR = throwable -> {
+@Nullable final StackTraceElement[] stackTraceElements =
throwable.getStackTrace();
+return (stackTraceElements != null && stackTraceElements.length > 0) ?
stackTraceElements[0] : null;
+};
+
+static final ThrowablePropertyRendererFactory INSTANCE =
+new ThrowablePropertyRendererFactory(THROWING_METHOD_EXTRACTOR);
+
+private final Map rendererByPropertyName;
+
+ThrowablePropertyRendererFactory(final Function throwingMethodExtractor) {
Review Comment:
Package-private is more restrictive than `protected` – we shouldn't loosen
the accessibility without a use case.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Use a random `${test:logging.path}` (logging-log4j2)
vy commented on code in PR #2954:
URL: https://github.com/apache/logging-log4j2/pull/2954#discussion_r1793353005
##
log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/TempLoggingDirectory.java:
##
@@ -135,6 +132,29 @@ private PathHolder createLoggingPath(final
ExtensionContext context, final Clean
return holder;
}
+private Path determinePerClassPath(ExtensionContext context) {
+// Check if the parent context already created a folder
+PathHolder holder =
ExtensionContextAnchor.getAttribute(PathHolder.class, PathHolder.class,
context);
+if (holder == null) {
+try {
+// Create temporary per-class directory
+final String baseDir = System.getProperty("basedir");
+final Path basePath = (baseDir != null ? Paths.get(baseDir,
"target") : Paths.get(".")).resolve("logs");
+final Class clazz = context.getRequiredTestClass();
+final Package pkg = clazz.getPackage();
+final String dir =
+pkg.getName().replaceAll("[.$]", File.separatorChar ==
'\\' ? "" : File.separator);
+// Create a temporary directory that uses the simple class
name as prefix
+Path packagePath = basePath.resolve(dir);
+Files.createDirectories(packagePath);
+return Files.createTempDirectory(packagePath,
clazz.getSimpleName());
Review Comment:
`Files.createTempDirectory()` already takes care of the distinctiveness – I
just want to be able to spot the last effective folder. I don't see how adding
just a counter helps, since counters get reset at reruns. Am I missing
something?
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Document GraalVM usage with Log4j (logging-log4j2)
vy commented on code in PR #3054: URL: https://github.com/apache/logging-log4j2/pull/3054#discussion_r1793361891 ## src/site/antora/modules/ROOT/pages/graalvm.adoc: ## @@ -0,0 +1,128 @@ + + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + + += Building GraalVM native images + +Since version +xref:release-notes.adoc#release-notes-2-25-0[`2.25.0`] +both +xref:manual/api.adoc[Log4j API] +and +xref:manual/implementation.adoc[its reference implementation] +provide out-of-the-box support for creating native executables using +https://www.graalvm.org/[GraalVM]. + +This document complements the +xref:manual/installation.adoc[Installation Guide] +and provides additional details on the steps necessary to create native images that use the Log4j API. +The information is split depending on which Log4j API implementation you are using. + +.Struggling with the logging API, implementation, and bridge concepts? Click for an introduction. +[%collapsible] + +include::partial$concepts.adoc[tag=!software-type] + + +[TIP] + +Are you looking for an example of GraalVM application that uses the Log4j API? +Check out +the https://github.com/apache/logging-log4j-samples/tree/main/log4j-samples-graalvm[`log4j-samples-graalvm`] +project. + + +[#impl-simple] +== Using Simple Logger + +If you use +xref:manual/simple-logger.adoc[Simple Logger] shipped with xref:manual/api.html[Log4j API] Review Comment: ```suggestion xref:manual/simple-logger.adoc[Simple Logger] shipped with xref:manual/api.adoc[Log4j API] ``` -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793313669
##
log4j-layout-template-json/src/main/java/org/apache/logging/log4j/layout/template/json/JsonTemplateLayoutDefaults.java:
##
@@ -93,7 +95,7 @@ public static String getStackTraceElementTemplateUri() {
}
public static String getEventDelimiter() {
-return
PROPERTIES.getStringProperty("log4j.layout.jsonTemplate.eventDelimiter",
System.lineSeparator());
+return
PROPERTIES.getStringProperty("log4j.layout.jsonTemplate.eventDelimiter",
LINE_SEPARATOR);
Review Comment:
I was trying to eat our own dog food – it tastes horrible. In a follow-up
PR, I will replace all references to `Constants.LINE_SEPARATOR` and deprecate
the constant.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793335685
##
log4j-core-test/src/test/java/org/apache/logging/log4j/core/EventParameterMemoryLeakTest.java:
##
@@ -64,20 +63,15 @@ public void testParametersAreNotLeaked() throws Exception {
final String line1 = reader.readLine();
final String line2 = reader.readLine();
final String line3 = reader.readLine();
-// line4 is empty line because of the line separator after throwable
pattern
final String line4 = reader.readLine();
final String line5 = reader.readLine();
-final String line6 = reader.readLine();
-final String line7 = reader.readLine();
reader.close();
-file.delete();
+// file.delete();
assertThat(line1, containsString("Message with parameter paramValue"));
assertThat(line2, containsString("paramValue"));
assertThat(line3, containsString("paramValue"));
-assertThat(line4, is(""));
-assertThat(line5, containsString("paramValue"));
-assertThat(line6, is(""));
-assertNull(line7, "Expected only six lines");
+assertThat(line4, containsString("paramValue"));
+assertNull(line5, "Expected only six lines");
Review Comment:
Fixed in 2022db7d3910bcd07b006452efc12e99482ca14c. Could you check, please?
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793337368
##
log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/NestedLoggingFromThrowableMessageTest.java:
##
@@ -80,14 +80,15 @@ public void testNestedLoggingInLastArgument() throws
Exception {
final Set lines2 = readUniqueLines(file2);
assertEquals("Expected the same data from both appenders", lines1,
lines2);
-assertEquals(3, lines1.size());
+assertEquals(2, lines1.size());
assertTrue(lines1.contains("INFO NestedLoggingFromThrowableMessageTest
Logging in getMessage "));
assertTrue(lines1.contains("ERROR
NestedLoggingFromThrowableMessageTest Test message"));
Review Comment:
Not really, since lines 2 and 3 get checked against their start, not the
complete content. Nevertheless, the situation is improved in
2022db7d3910bcd07b006452efc12e99482ca14c.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Prefix stack traces with a newline in Pattern Layout (logging-log4j2)
vy commented on code in PR #3045:
URL: https://github.com/apache/logging-log4j2/pull/3045#discussion_r1793337694
##
log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/PatternLayoutDefaultExceptionHandlerTest.java:
##
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.layout;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.core.Layout;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.NullConfiguration;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.assertj.core.api.AbstractStringAssert;
+import org.junit.jupiter.api.Test;
+
+class PatternLayoutDefaultExceptionHandlerTest {
+
+private static final Configuration CONFIG = new NullConfiguration();
+
+private static final Exception EXCEPTION = new RuntimeException("foo");
+
+@Test
+void default_exception_handler_should_be_provided() {
+final String threadName = Thread.currentThread().getName();
+final String exceptionClassName =
EXCEPTION.getClass().getCanonicalName();
+final String exceptionMessage = EXCEPTION.getMessage();
+final String firstLine = String.format("%s%n%s: %s", threadName,
exceptionClassName, exceptionMessage);
+assertThatPatternEncodes("%t", true).startsWith(firstLine);
Review Comment:
Fixed in d7d46be7d022e4f1821dcd39e52b84844d7d2e92.
##
src/changelog/.2.x.x/.release-notes.adoc.ftl:
##
@@ -20,14 +20,21 @@
<#if release.date?has_content>Release date:: ${release.date}
-This releases contains ...
+This minor release contains bug fixes, behavioral improvements, and a
fully-fledged support for the GraalVM native image generation.
+[#release-notes-2-25-0-graalvm]
=== GraalVM reachability metadata
-Log4j Core and all its extension modules have been enhanced with embedded
+Log4j Core and all its extension modules have been enriched with embedded
https://www.graalvm.org/latest/reference-manual/native-image/metadata/[GraalVM
reachability metadata].
-This will allow the generation of GraalVM native images out-of-the-box,
without any additional step necessary.
+This allows the generation of GraalVM native images out-of-the-box without any
additional steps.
+See our xref:graalvm.adoc[GraalVM guide] for details.
-See our xref:graalvm.adoc[GraalVM guide] for more details.
+[#release-notes-2-25-0-PL-ex]
+=== Exception handling in Pattern Layout
+
+Exception handling in Pattern Layout went through a major rewrite.
+This effectively helped with fixing some bugs by matching the feature parity
of all exception converters.
+Additionally, rendered stack traces are ensured to be prefixed with a newline,
which used to be a whitespace in earlier versions.
Review Comment:
Fixed in 28a8325413ae8fd923d3174a080271cfb92625ad.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Upgrades JUnit to version `5.11.2` (logging-log4j2)
vy commented on code in PR #3069:
URL: https://github.com/apache/logging-log4j2/pull/3069#discussion_r1793343038
##
log4j-core-test/src/main/java/org/apache/logging/log4j/core/test/junit/package-info.java:
##
@@ -20,7 +20,7 @@
* @see org.junit.rules.TestRule
*/
@Export
-@Version("2.23.0")
+@Version("2.23.1")
Review Comment:
```suggestion
@Version("2.25.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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Provide guidelines and templates for the release process [logging-log4cxx]
ppkarwasz commented on PR #413: URL: https://github.com/apache/logging-log4cxx/pull/413#issuecomment-2404141765 Hi Stephen, > > Commit everything to the dev subversion repository. > > What will we use to trigger this? I do not believe we should to do it on every commit. In Log4j we have two different workflows: - On each commit to the default branch a snapshot is published to the Nexus Repository, but no source code. The binaries are not signed. I believe that the Log4cxx equivalent would be to just attach the source distribution to the GHA run. - The `deploy-release` workflow is triggered by each push to branches named `release/`. This workflow signs the artifacts (both binary and source) and uploads them to the subversion 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. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Provide guidelines and templates for the release process [logging-log4cxx]
swebb2066 commented on PR #413: URL: https://github.com/apache/logging-log4cxx/pull/413#issuecomment-2403705759 > Commit everything to the dev subversion repository. What will we use to trigger this? I do not believe we should to do it on every commit. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
