Hi Woonsan,
I've spent about a day trying to get SLF4J logging to work but have run into
some setbacks.
-=-=-
TL;DR
-=-=-
In order for a portlet WAR context to use an implementation/adapter for SLF4J other than
slf4j-jdk14-1.7.5.jar (java.util.logging) in Pluto 3.0, the SLF4J API must be specified as
<scope>compile</scope> rather than <scope>provided</scope>.
For example, if using Log4J, a portlet WAR needs to include the following:
WEB-INF/lib/log4j-1.2.17.jar
WEB-INF/lib/slf4j-api-1.7.25.jar
WEB-INF/lib/slf4j-log4j12-1.7.25.jar
Would you be willing to +1 the release if the archetype pom.xml files specified
the following dependencies?
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
<version>1.7.25</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
<version>1.7.25</version>
<scope>runtime</scope>
</dependency>
-=-=-=-=-=-=-=-=-=-=-
Detailed Explanation:
-=-=-=-=-=-=-=-=-=-=-
Pluto 3.0 has the SLF4J API in the Tomcat global classloader:
tomcat/lib/slf4j-api-1.7.5.jar/
The reason why, is because jars like the following have a compile-time
dependency on the SLF4J API:
tomcat/lib/pluto-container-3.0.1-SNAPSHOT.jar
Now, in order for Pluto to start without errors, there *MUST* be an impl (or
adapter impl) of the SLF4J API in the global classloader. That is probably why
the JDK14 (java.util.logging) adapter appears in the global classloader as well:
tomcat/lib/slf4j-jdk14-1.7.5.jar
-----------
Problem #1:
-----------
If a portlet WAR relies on tomcat/lib/slf4j-api-1.7.5.jar (the SLF4J API in the
global classloader), it will be *forced* to use JDK14 logging when using the
SLF4J Logger and LoggerFactory.
The reason why is because the SLF4J API always goes with the first impl that it
finds in the classpath. Since the JDK14 adapter is present in the global
classloader, the Tomcat ClassLoader always chooses the JDK14 adapter, even if
the Log4J adapter is found in the portlet WAR context.
-----------
Problem #2:
-----------
It is not possible to swap-out the JDK14 adapter for the Log4J adapter in the
global classpath and have Log4J read the WEB-INF/classes/log4j.properties file.
The reason why is because log4j-1.2.17.jar must *always* appear in the
WEB-INF/lib folder of the portlet WAR context in order for the
WEB-INF/classes/log4j.properties file to get picked up -- Log4J will only look
for a log4j.properties file with the LogManager class is loaded into the
ClassLoader:
https://github.com/apache/log4j/blob/log4j-1.2.17/src/main/java/org/apache/log4j/LogManager.java#L109
In other words, when log4j-1.2.17.jar is present in the global classloader, it
can't read a portlet's log4j.properties file since it only gets loaded into the
global classloader once.
------------------
Proposed Solution:
------------------
Since the SLF4J API always goes with the first impl/adapter that it finds, the SLF4J API must be
specified as <scope>compile</scope> rather than <scope>provided</scope>.
Repeating the example from the TL;DR section, if a portlet WAR wants to use
Log4J, it would need to include the following:
WEB-INF/lib/log4j-1.2.17.jar
WEB-INF/lib/slf4j-api-1.7.25.jar
WEB-INF/lib/slf4j-log4j12-1.7.25.jar
Thanks,
Neil
On 4/25/17 12:04 PM, Woonsan Ko wrote:
Thanks, Neil! Please see my comments inline.
On Tue, Apr 25, 2017 at 11:53 AM, Neil Griffin
<neil.grif...@portletfaces.org> wrote:
Hi Woonsan,
OK, I will rollback and re-release with the header present in the following
files:
- pom.xml
- src/main/resources/META-INF/maven/archetype-metadata.xml
- src/main/resources/META-INF/maven/archetype.xml
Additionally, I will move from the Log4J API to the SLF4J API.
Would it be OK if I specify "org.slf4j:slf4j-log4j12" as the logging
implementation?
Absolutely. That's how we do in most portals project right now, AFAIK.
You can keep that and log4j12 as runtime scope dependencies.
Also, if I complete the work today, then will it require a new 72 hour
voting process?
I don't think so. If you re-stage it with the fixes, I'll review it
and cast my vote with +1. You don't need another 72 hours.
Kind regards,
Woonsan
Thank you,
Neil
On 4/25/17 11:36 AM, Woonsan Ko wrote:
On Tue, Apr 25, 2017 at 11:09 AM, Neil Griffin
<neil.grif...@portletfaces.org> wrote:
Hi Woonsan,
I don't think that I have the administrative privileges to fix the
staging
repository visibility problem you encountered.
But thank you for your careful observations. Regarding the licensing, the
Apache 2.0 License is specified in the pom.xml descriptor of each
archetype:
<license>
<name>Apache License, Version 2.0</name>
<url>http://www.apache.org/licenses/LICENSE-2.0</url>
</license>
Also, the archetype JAR artifacts contain the text of the Apache 2.0
License
in the META-INF/LICENSE file.
The reason why license "headers" are not present in files like
HelloWorldPortlet.java is because archetype files are essentially
templates
that will be used by the "mvn archetype:generate" command to create a new
project. The developer would then be free to apply whatever license they
want to their newly generated portlet project.
It's totally fine not to have license headers in archetype-generated
files like HelloWorldPortlet.java.
My concerns were at these files, which are the archetype source
itself, not generated ones, for instance:
- pom.xml
- src/main/resources/META-INF/maven/archetype-metadata.xml
- src/main/resources/META-INF/maven/archetype.xml
I think those three files must have license headers.
Regarding log4j, I would be happy to migrate to the SLF4J API in a future
dot release.
Cool!
Please let me know whether or not I have addressed your concerns to your
satisfaction.
Without proper license headers in those three files, I don't think
that's qualified for a proper Apache release.
Sometimes we miss license headers in some source files unintentionally
in a bigger project, which might be excused, but in this case, it's
obvious that we totally forgot adding the headers in the whole
project, IMHO.
Regards,
Woonsan
Best Regards,
Neil
On 4/24/17 11:46 PM, Woonsan Ko wrote:
On Thu, Apr 20, 2017 at 1:44 PM, Neil Griffin
<neil.grif...@portletfaces.org> wrote:
Dear Apache Portals Pluto Team and community,
I have staged a release candidate for the new Apache Portals Pluto
Maven
Archetypes 3.0.0 release,
which includes the following two artifacts:
<groupId>org.apache.portals.pluto.archetype</groupId>
<artifactId>bean-portlet-archetype</artifactId>
<packaging>maven-archetype</packaging>
<groupId>org.apache.portals.pluto.archetype</groupId>
<artifactId>generic-portlet-archetype</artifactId>
<packaging>maven-archetype</packaging>
Please review the release candidate which is available from the
following
maven staging repository:
https://repository.apache.org/content/repositories/orgapacheportals-1016
This link doesn't work for me. I managed to find the staging repo at
https://repository.apache.org/#stagingRepositories.
It shows "404 - Repository "orgapacheportals-1016 (staging: open)"
[id=orgapacheportals-1016] exists but is not exposed" when clicked on,
regardless whether or not I signed in https://repository.apache.org/.
Does anyone know the reason?
This vote is open for the next 72 hours.
Please cast your vote:
I'm not sure if it is desirable to release this. When I downloaded the
bean-portlet-archetype-3.0.0-source-release.zip from the Nexus, I
could hardly find source files with Apache License header [1]. Most
files are missing the license header.
Wouldn't it be more desirable to correct this issue first?
And, one minor thing to remark is that the archetype is using log4j
v1, neither slf4j nor log4j v2. AFAIK, pluto project itself and its
submodules such as container have used slf4j as logging API and log4j
as a default binding. Not a major, but just something to consider
later perhaps...
Kind regards,
Woonsan
[1] http://www.apache.org/legal/src-headers.html#headers
[ ] +1 for Release
[ ] 0 for Don't care
[ ] -1 Don't release (do provide a reason then)
Thanks,
Neil
