Hi Neil, On Wed, Apr 26, 2017 at 4:23 PM, Neil Griffin <neil.grif...@portletfaces.org> wrote: > 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?
Yes. I think logging library issue was a minor issue anyway in release process anyway. > > <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> +1 for this release. > > -=-=-=-=-=-=-=-=-=-=- > 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 +1. I think your proposal is the best solution from what we can do in the current circumstances without big changes. If someone wants to improve the situation in a different / better way in the longer term, please feel free to start a new thread for that. Thank you so much for your investigation with the proposal! Cheers, Woonsan > > 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> > On Wed, Apr 26, 2017 at 4:23 PM, Neil Griffin <neil.grif...@portletfaces.org> wrote: > 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >> >
