> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote:
> > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs
> 
> Erick Erickson wrote:
>     It turns out not. Thanks to Shawn I now know the shim is used, which 
> means we can (and do) still use log4j calls in the code.
> 
> Tomás Fernández Löbbe wrote:
>     Not entierly sure. We **don't** do log4j calls in the code in general, we 
> have that explicitly forbidden in forbidden APIS today, and code that does 
> something with log4j has to supress that. Developers must instead use slf4j 
> APIs. I don't believe that's changing now with log4j2, or does it?
> 
> Shawn Heisey wrote:
>     ```
>     Not entierly sure. We don't do log4j calls in the code in general, we 
> have that explicitly forbidden in forbidden APIS today, and code that does 
> something with log4j has to supress that. Developers must instead use slf4j 
> APIs. I don't believe that's changing now with log4j2, or does it?
>     ```
>     
>     Solr only uses log4j code in one place -- our log watcher for the admin 
> UI.  This probably has a notation to disable the forbidden API check.  Lucene 
> doesn't use any logging at all as far as I know.
>     
>     The reason that we need the 1.2 compatibility shim is not for Solr or 
> Lucene.  It's for dependencies.  One dependency that I know for sure uses it 
> is zookeeper.  There might be others, but I haven't done an exhaustive check 
> to find out.

That is exactly my point. Solr doesn't use log4j in the code and it's 
forbidden. We need to include the log4j2 packages in the list of forbidden so 
that developers don't try to use it (unless they know what they are doing and 
explicitly remove the forbidden check)


- Tomás


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65888/#review198580
-----------------------------------------------------------


On March 3, 2018, 2:51 a.m., Varun Thacker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65888/
> -----------------------------------------------------------
> 
> (Updated March 3, 2018, 2:51 a.m.)
> 
> 
> Review request for lucene.
> 
> 
> Repository: lucene-solr
> 
> 
> Description
> -------
> 
> Upgrade Solr to use Log4J2
> 
> 
> Diffs
> -----
> 
>   lucene/CHANGES.txt e3799bc9d5 
>   lucene/common-build.xml 4fa59ac936 
>   lucene/ivy-versions.properties 5ab36ddfa2 
>   solr/CHANGES.txt 05f4f560bd 
>   solr/bin/install_solr_service.sh b82957144d 
>   solr/bin/solr 4e178de945 
>   solr/bin/solr.cmd dcff0c6af7 
>   solr/bin/solr.in.cmd bfb33e0e9d 
>   solr/bin/solr.in.sh e7478cdf5c 
>   solr/contrib/clustering/src/test-files/log4j.properties b5216db8b2 
>   solr/contrib/clustering/src/test-files/log4j2.xml PRE-CREATION 
>   solr/contrib/dataimporthandler/src/test-files/log4j.properties d3ea4deafc 
>   solr/contrib/dataimporthandler/src/test-files/log4j2.xml PRE-CREATION 
>   solr/contrib/ltr/src/test-files/log4j.properties d86c6988d5 
>   solr/contrib/ltr/src/test-files/log4j2.xml PRE-CREATION 
>   solr/core/ivy.xml ff4fa48679 
>   
> solr/core/src/java/org/apache/solr/client/solrj/embedded/JettySolrRunner.java 
> 23a8dc1eb3 
>   solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java 
> 122d2cbf8b 
>   solr/core/src/java/org/apache/solr/logging/LogWatcher.java c510590282 
>   solr/core/src/java/org/apache/solr/logging/log4j/EventAppender.java 
> ff2876fb2f 
>   solr/core/src/java/org/apache/solr/logging/log4j/Log4jInfo.java dfd3dde74a 
>   solr/core/src/java/org/apache/solr/logging/log4j/Log4jWatcher.java 
> 04fa5fb1d8 
>   solr/core/src/java/org/apache/solr/logging/log4j/package-info.java 
> f78953385c 
>   solr/core/src/java/org/apache/solr/logging/log4j2/Log4j2Watcher.java 
> PRE-CREATION 
>   solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java 
> 4d944d239b 
>   solr/core/src/java/org/apache/solr/util/SolrCLI.java 266ef3a28a 
>   solr/core/src/java/org/apache/solr/util/SolrLogLayout.java a60ada828b 
>   solr/core/src/java/org/apache/solr/util/StartupLoggingUtils.java c582eff4c0 
>   solr/core/src/test-files/log4j.properties 969439a228 
>   solr/core/src/test-files/log4j2.xml PRE-CREATION 
>   solr/core/src/test/org/apache/solr/handler/RequestLoggingTest.java 
> 4c780ccda4 
>   solr/core/src/test/org/apache/solr/handler/admin/LoggingHandlerTest.java 
> 555c1376a5 
>   solr/core/src/test/org/apache/solr/logging/log4j2/Log4j2WatcherTest.java 
> PRE-CREATION 
>   solr/core/src/test/org/apache/solr/util/TestSolrCLIRunExample.java 
> 89008517f8 
>   solr/example/README.txt 562c256377 
>   solr/example/example-DIH/solr/db/conf/solrconfig.xml 1ffbbe817f 
>   solr/example/example-DIH/solr/mail/conf/solrconfig.xml 770b0fd870 
>   solr/example/example-DIH/solr/solr/conf/solrconfig.xml 3f00141340 
>   solr/example/resources/log4j.properties 02f91c5dae 
>   solr/example/resources/log4j2.xml PRE-CREATION 
>   solr/licenses/audience-annotations-0.7.0.jar.sha1 PRE-CREATION 
>   solr/licenses/audience-annotations-LICENSE-ASL.txt PRE-CREATION 
>   solr/licenses/audience-annotations-NOTICE.txt PRE-CREATION 
>   solr/licenses/disruptor-3.4.0.jar.sha1 PRE-CREATION 
>   solr/licenses/disruptor-LICENSE-ASL.txt PRE-CREATION 
>   solr/licenses/disruptor-NOTICE.txt PRE-CREATION 
>   solr/licenses/log4j-1.2-api-2.10.0.jar.sha1 PRE-CREATION 
>   solr/licenses/log4j-1.2.17.jar.sha1 383110e29f 
>   solr/licenses/log4j-api-2.10.0.jar.sha1 PRE-CREATION 
>   solr/licenses/log4j-api-LICENSE-ASL.txt PRE-CREATION 
>   solr/licenses/log4j-api-NOTICE.txt PRE-CREATION 
>   solr/licenses/log4j-core-2.10.0.jar.sha1 PRE-CREATION 
>   solr/licenses/log4j-core-LICENSE-ASL.txt PRE-CREATION 
>   solr/licenses/log4j-core-NOTICE.txt PRE-CREATION 
>   solr/licenses/log4j-slf4j-LICENSE-ASL.txt PRE-CREATION 
>   solr/licenses/log4j-slf4j-NOTICE.txt PRE-CREATION 
>   solr/licenses/log4j-slf4j-impl-2.10.0.jar.sha1 PRE-CREATION 
>   solr/licenses/slf4j-log4j12-1.7.24.jar.sha1 b8ec050172 
>   solr/server/README.txt 228f4d467b 
>   solr/server/ivy.xml c9b3a73014 
>   solr/server/resources/log4j.properties 9f9c4a0f74 
>   solr/server/resources/log4j2.xml PRE-CREATION 
>   solr/server/scripts/cloud-scripts/log4j.properties 5f2ae18574 
>   solr/server/scripts/cloud-scripts/log4j2.xml PRE-CREATION 
>   solr/server/scripts/cloud-scripts/snapshotscli.sh f885721f66 
>   solr/server/scripts/cloud-scripts/zkcli.bat c5d7b72948 
>   solr/server/scripts/cloud-scripts/zkcli.sh bd971e9ee4 
>   solr/server/solr/configsets/sample_techproducts_configs/conf/solrconfig.xml 
> 1b2563662e 
>   solr/solr-ref-guide/ivy.xml adefe2ced5 
>   solr/solr-ref-guide/src/configuring-logging.adoc 8984744e55 
>   solr/solr-ref-guide/src/solr-control-script-reference.adoc 8ca14ea476 
>   solr/solr-ref-guide/src/taking-solr-to-production.adoc ca0f4ebe76 
>   solr/solrj/ivy.xml 3637bc34bd 
>   solr/solrj/src/test-files/log4j.properties dae4f6f418 
>   solr/solrj/src/test-files/log4j2.xml PRE-CREATION 
>   solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java 706f1ebe8b 
>   solr/test-framework/src/java/org/apache/solr/util/LogLevel.java 7542694441 
>   solr/test-framework/src/test-files/log4j.properties f6fedb6ea2 
>   solr/test-framework/src/test-files/log4j2.xml PRE-CREATION 
>   solr/test-framework/src/test/org/apache/solr/TestLogLevelAnnotations.java 
> 2ede874e50 
> 
> 
> Diff: https://reviews.apache.org/r/65888/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Varun Thacker
> 
>

Reply via email to