----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61014/#review181201 -----------------------------------------------------------
Ship it! LGTM spring dependency went to test only which is great: ``` [INFO] - org.apache.activemq:activemq-core:jar:5.7.0:test [INFO] +- org.apache.geronimo.specs:geronimo-jms_1.1_spec:jar:1.1.1:test [INFO] +- org.apache.activemq:kahadb:jar:5.7.0:test [INFO] +- org.apache.activemq.protobuf:activemq-protobuf:jar:1.1:test [INFO] +- org.fusesource.mqtt-client:mqtt-client:jar:1.3:test [INFO] | +- org.fusesource.hawtdispatch:hawtdispatch-transport:jar:1.11:test [INFO] | | - org.fusesource.hawtdispatch:hawtdispatch:jar:1.11:test [INFO] | - org.fusesource.hawtbuf:hawtbuf:jar:1.9:test [INFO] +- org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec:jar:1.0.1:test [INFO] +- org.springframework:spring-context:jar:3.0.7.RELEASE:test [INFO] | +- org.springframework:spring-aop:jar:3.0.7.RELEASE:test [INFO] | | - aopalliance:aopalliance:jar:1.0:test [INFO] | +- org.springframework:spring-beans:jar:3.0.7.RELEASE:test [INFO] | +- org.springframework:spring-core:jar:3.0.7.RELEASE:test [INFO] | +- org.springframework:spring-expression:jar:3.0.7.RELEASE:test [INFO] | - org.springframework:spring-asm:jar:3.0.7.RELEASE:test [INFO] +- commons-net:commons-net:jar:3.1:test [INFO] - org.jasypt:jasypt:jar:1.9.0:test ``` jms-api was added correctly. build and junit passes. ``` mvn clean install -DskipTests mvn test -pl flume-ng-sources/flume-jms-source ``` sanity check on dependency diff was ok: ``` mvn -B -U dependency:list | grep -v Download | grep jar | sort | uniq > 1.8.0.dep.list curl https://reviews.apache.org/r/61014/diff/raw/ | patch -p0 mvn -B -U dependency:list | grep -v Download | grep jar | sort | uniq > 3131.dep.list diff 1.8.0.dep.list 3131.dep.list ``` result (aopalliance and commons-net was only removed from the `provided` scope. They have been already in the `test` scope via other module thus not listed as a difference) ``` 5d4 < [INFO] aopalliance:aopalliance:jar:1.0:provided 114d112 < [INFO] commons-net:commons-net:jar:3.1:provided 131a130 > [INFO] javax.jms:jms-api:jar:1.1-rev-1:compile 180,182c179,181 < [INFO] org.apache.activemq.protobuf:activemq-protobuf:jar:1.1:provided < [INFO] org.apache.activemq:activemq-core:jar:5.7.0:provided < [INFO] org.apache.activemq:kahadb:jar:5.7.0:provided --- > [INFO] org.apache.activemq.protobuf:activemq-protobuf:jar:1.1:test > [INFO] org.apache.activemq:activemq-core:jar:5.7.0:test > [INFO] org.apache.activemq:kahadb:jar:5.7.0:test 287c286 < [INFO] org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec:jar:1.0.1:provided --- > [INFO] > org.apache.geronimo.specs:geronimo-j2ee-management_1.1_spec:jar:1.0.1:test 289c288 < [INFO] org.apache.geronimo.specs:geronimo-jms_1.1_spec:jar:1.1.1:provided --- > [INFO] org.apache.geronimo.specs:geronimo-jms_1.1_spec:jar:1.1.1:test 459,461c458,460 < [INFO] org.fusesource.hawtbuf:hawtbuf:jar:1.9:provided < [INFO] org.fusesource.hawtdispatch:hawtdispatch-transport:jar:1.11:provided < [INFO] org.fusesource.hawtdispatch:hawtdispatch:jar:1.11:provided --- > [INFO] org.fusesource.hawtbuf:hawtbuf:jar:1.9:test > [INFO] org.fusesource.hawtdispatch:hawtdispatch-transport:jar:1.11:test > [INFO] org.fusesource.hawtdispatch:hawtdispatch:jar:1.11:test 465c464 < [INFO] org.fusesource.mqtt-client:mqtt-client:jar:1.3:provided --- > [INFO] org.fusesource.mqtt-client:mqtt-client:jar:1.3:test 473c472 < [INFO] org.jasypt:jasypt:jar:1.9.0:provided --- > [INFO] org.jasypt:jasypt:jar:1.9.0:test 524,529c523,528 < [INFO] org.springframework:spring-aop:jar:3.0.7.RELEASE:provided < [INFO] org.springframework:spring-asm:jar:3.0.7.RELEASE:provided < [INFO] org.springframework:spring-beans:jar:3.0.7.RELEASE:provided < [INFO] org.springframework:spring-context:jar:3.0.7.RELEASE:provided < [INFO] org.springframework:spring-core:jar:3.0.7.RELEASE:provided < [INFO] org.springframework:spring-expression:jar:3.0.7.RELEASE:provided --- > [INFO] org.springframework:spring-aop:jar:3.0.7.RELEASE:test > [INFO] org.springframework:spring-asm:jar:3.0.7.RELEASE:test > [INFO] org.springframework:spring-beans:jar:3.0.7.RELEASE:test > [INFO] org.springframework:spring-context:jar:3.0.7.RELEASE:test > [INFO] org.springframework:spring-core:jar:3.0.7.RELEASE:test > [INFO] org.springframework:spring-expression:jar:3.0.7.RELEASE:test ``` pom.xml Lines 1074-1080 (original), 1074-1084 (patched) <https://reviews.apache.org/r/61014/#comment256693> Seems like javax.jms:jms:1.1 was kind of replaced by javax.jms:jms-api:1.1-rev-1 in maven central. Have you considered removing the unused javax.jms:jms:1.1 dependency definition? It is not a biggie I think it might be even better to submit this in a (follow-up) separate commit. - Attila Simon On July 23, 2017, 11:13 a.m., Ferenc Szabo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61014/ > ----------------------------------------------------------- > > (Updated July 23, 2017, 11:13 a.m.) > > > Review request for Flume. > > > Repository: flume-git > > > Description > ------- > > Moved activemq to test scope so the vulnerable spring dependencies move out > from production and added javax.jms to handle the jms dependencies > > > Diffs > ----- > > flume-ng-sources/flume-jms-source/pom.xml cbae702 > pom.xml 5730db0 > > > Diff: https://reviews.apache.org/r/61014/diff/1/ > > > Testing > ------- > > mvn clean install > Build and unit tests were successful. > > > Thanks, > > Ferenc Szabo > >
