I have done my review, overall is OK, some minor comments -- Enrico
2016-12-19 17:42 GMT+01:00 Sijie Guo <[email protected]>: > I already sent a PR for addressing the RAT issues. > https://github.com/apache/bookkeeper/pull/95 > > If you can review that, I can get that change in to address the RAT issue. > > - Sijie > > On Mon, Dec 19, 2016 at 8:23 AM, Enrico Olivelli <[email protected]> > wrote: > >> OK >> I can file a PR tomorrow >> there are some RAT issues too, I will check but i have not a clear >> idea of RAT and licensing >> >> -- Enrico >> >> 2016-12-19 16:39 GMT+01:00 Sijie Guo <[email protected]>: >> > I would suggest skipping the check first. We can discuss a fix later with >> > addressing performance concerns. >> > >> > Sijie >> > >> > >> > On Dec 19, 2016 2:03 AM, "Enrico Olivelli" <[email protected]> wrote: >> > >> > I have reopened this issue, as most of the errors come from it >> > https://issues.apache.org/jira/browse/BOOKKEEPER-964 >> > >> > >> > @Matteo >> > do you have time to review and fix ? >> > I would file a PR, but the fix is not so trivial. >> > ideas: >> > 1) replace "volatile int" with AtomicInteger >> > 2) Add fixbugs rules to skip the checks >> > >> > I prefer the first solution, but I don't know the impact on performance >> > >> > >> > 2016-12-18 22:39 GMT+01:00 Enrico Olivelli <[email protected]>: >> >> I am sorry but there is a bunch of findbugs errors. >> >> I can file a PR tomorrow. See the JIRA for comments. >> >> >> >> Enrico >> >> >> >> >> >> Il sab 17 dic 2016, 10:14 Enrico Olivelli <[email protected]> ha >> > scritto: >> >>> >> >>> Thank you very much Sijie! >> >>> >> >>> Il sab 17 dic 2016, 02:47 Sijie Guo <[email protected]> ha scritto: >> >>>> >> >>>> Enrico, >> >>>> >> >>>> Sorry for delaying on handling PRs. I just merged bunch of PRs that >> >>>> already >> >>>> reviewed. The CI builds should be back to normal. >> >>>> >> >>>> I will try to go another round of reviews on the other PRs. >> >>>> >> >>>> - Sijie >> >>>> >> >>>> >> >>>> >> >>>> On Fri, Dec 16, 2016 at 8:18 AM, Enrico Olivelli <[email protected] >> > >> >>>> wrote: >> >>>> >> >>>> > I think that the problem did not show on jenkins due to the failing >> >>>> > tests, the shade plugin works after the execution of the tests in >> case >> >>>> > of full success. >> >>>> > >> >>>> > Maybe can someone review the PRs for the fixes to the failing tests, >> >>>> > so that they can be merged and we have a functional QA env ? >> >>>> > >> >>>> > They are: >> >>>> > https://github.com/apache/bookkeeper/pull/91 - BOOKKEEPER-984: Fix >> >>>> > BookieClientTest.testWriteGaps >> >>>> > https://github.com/apache/bookkeeper/pull/82 - BOOKKEEPER-946 >> Provide >> >>>> > an option to delay auto recovery of lost bookies >> >>>> > >> >>>> > >> >>>> > Thank you very much >> >>>> > >> >>>> > 2016-12-16 17:12 GMT+01:00 Enrico Olivelli <[email protected]>: >> >>>> > > I found a quick solution, this is the issue >> >>>> > > https://issues.apache.org/jira/browse/BOOKKEEPER-987 >> >>>> > > >> >>>> > > and the Pull Request >> >>>> > > https://github.com/apache/bookkeeper/pull/94 >> >>>> > > >> >>>> > > The fix is easy, it is an upgrade of the shade plugin >> >>>> > > >> >>>> > > -- Enrico >> >>>> > > >> >>>> > > >> >>>> > > 2016-12-16 16:54 GMT+01:00 Enrico Olivelli <[email protected]>: >> >>>> > >> Hi, >> >>>> > >> it seems to me that making a fresh clean clone from github checks >> >>>> > >> out >> >>>> > >> a bad version of BookKeeper which could not be compiled. >> >>>> > >> >> >>>> > >> I am at commit f710e5a44569aa080b80eff855fff2c3810957fd >> >>>> > >> >> >>>> > >> The first "compilable" commit is 4cf097871d28b70a53f8c6bffaf1c2 >> >>>> > 022e9953b2 >> >>>> > >> >> >>>> > >> The broken commit is ecbb053e6e873859507e247cae727f4bc8b9f7fa >> >>>> > >> >> >>>> > >> My java -version >> >>>> > >> java version "1.8.0_92" >> >>>> > >> Java(TM) SE Runtime Environment (build 1.8.0_92-b14) >> >>>> > >> >> >>>> > >> SO: >> >>>> > >> Linux xxx.xxx.xxx 4.8.8-200.fc24.x86_64 #1 SMP Tue Nov 15 >> 19:41:51 >> >>>> > >> UTC >> >>>> > >> 2016 x86_64 x86_64 x86_64 GNU/Linux >> >>>> > >> >> >>>> > >> >> >>>> > >> this is the error on my (linux) laptop, it seems a problem on the >> >>>> > >> shade >> >>>> > plugin >> >>>> > >> >> >>>> > >> [INFO] Excluding commons-codec:commons-codec:jar:1.6 from the >> > shaded >> >>>> > jar. >> >>>> > >> [INFO] Excluding commons-io:commons-io:jar:2.1 from the shaded >> jar. >> >>>> > >> [INFO] Excluding net.java.dev.jna:jna:jar:3.2.7 from the shaded >> > jar. >> >>>> > >> [INFO] Excluding log4j:log4j:jar:1.2.15 from the shaded jar. >> >>>> > >> [INFO] Minimizing jar org.apache.bookkeeper: >> >>>> > bookkeeper-server:jar:4.5.0-SNAPSHOT >> >>>> > >> [INFO] ------------------------------ >> ------------------------------ >> >>>> > ------------ >> >>>> > >> [INFO] Reactor Summary: >> >>>> > >> [INFO] >> >>>> > >> [INFO] bookkeeper ......................................... >> SUCCESS >> >>>> > >> [ >> >>>> > 0.428 s] >> >>>> > >> [INFO] compability dependencies ........................... >> SUCCESS >> >>>> > >> [ >> >>>> > 0.025 s] >> >>>> > >> [INFO] bookkeeper-server-compat400 ........................ >> SUCCESS >> >>>> > >> [ >> >>>> > 2.324 s] >> >>>> > >> [INFO] bookkeeper-server-compat410 ........................ >> SUCCESS >> >>>> > >> [ >> >>>> > 1.206 s] >> >>>> > >> [INFO] bookkeeper-server-compat420 ........................ >> SUCCESS >> >>>> > >> [ >> >>>> > 1.309 s] >> >>>> > >> [INFO] Stats API for bookkeeper ........................... >> SUCCESS >> >>>> > >> [ >> >>>> > 0.560 s] >> >>>> > >> [INFO] bookkeeper-server .................................. >> FAILURE >> >>>> > >> [ >> >>>> > 4.201 s] >> >>>> > >> [INFO] bookkeeper-benchmark ............................... >> SKIPPED >> >>>> > >> [INFO] Stats provider for twitter-stats package ........... >> SKIPPED >> >>>> > >> [INFO] Stats provider for twitter-ostrich package ......... >> SKIPPED >> >>>> > >> [INFO] Stats provider for codahale metrics ................ >> SKIPPED >> >>>> > >> [INFO] bookkeeper-stats-providers ......................... >> SKIPPED >> >>>> > >> [INFO] ------------------------------ >> ------------------------------ >> >>>> > ------------ >> >>>> > >> [INFO] BUILD FAILURE >> >>>> > >> [INFO] ------------------------------ >> ------------------------------ >> >>>> > ------------ >> >>>> > >> [INFO] Total time: 10.222 s >> >>>> > >> [INFO] Finished at: 2016-12-16T16:45:34+01:00 >> >>>> > >> [INFO] Final Memory: 46M/959M >> >>>> > >> [INFO] ------------------------------ >> ------------------------------ >> >>>> > ------------ >> >>>> > >> [ERROR] Failed to execute goal >> >>>> > >> org.apache.maven.plugins:maven-shade-plugin:2.1:shade (default) >> on >> >>>> > >> project bookkeeper-server: Error creating shaded jar: 46848 -> >> > [Help >> >>>> > >> 1] >> >>>> > >> org.apache.maven.lifecycle.LifecycleExecutionException: Failed >> to >> >>>> > >> execute goal org.apache.maven.plugins: >> maven-shade-plugin:2.1:shade >> >>>> > >> (default) on project bookkeeper-server: Error creating shaded >> jar: >> >>>> > >> 46848 >> >>>> > >> at org.apache.maven.lifecycle.internal.MojoExecutor.execute( >> >>>> > MojoExecutor.java:212) >> >>>> > >> at org.apache.maven.lifecycle.internal.MojoExecutor.execute( >> >>>> > MojoExecutor.java:153) >> >>>> > >> at org.apache.maven.lifecycle.internal.MojoExecutor.execute( >> >>>> > MojoExecutor.java:145) >> >>>> > >> at org.apache.maven.lifecycle.internal. >> LifecycleModuleBuilder. >> >>>> > buildProject(LifecycleModuleBuilder.java:116) >> >>>> > >> at org.apache.maven.lifecycle.internal. >> LifecycleModuleBuilder. >> >>>> > buildProject(LifecycleModuleBuilder.java:80) >> >>>> > >> at org.apache.maven.lifecycle.internal.builder. >> singlethreaded. >> >>>> > SingleThreadedBuilder.build(SingleThreadedBuilder.java:51) >> >>>> > >> at org.apache.maven.lifecycle.internal.LifecycleStarter. >> >>>> > execute(LifecycleStarter.java:128) >> >>>> > >> at >> >>>> > >> org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:307) >> >>>> > >> at >> >>>> > >> org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:193) >> >>>> > >> at org.apache.maven.DefaultMaven. >> execute(DefaultMaven.java:106) >> >>>> > >> at org.apache.maven.cli.MavenCli.execute(MavenCli.java:863) >> >>>> > >> at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:288) >> >>>> > >> at org.apache.maven.cli.MavenCli.main(MavenCli.java:199) >> >>>> > >> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native >> Method) >> >>>> > >> at sun.reflect.NativeMethodAccessorImpl.invoke( >> >>>> > NativeMethodAccessorImpl.java:62) >> >>>> > >> at sun.reflect.DelegatingMethodAccessorImpl.invoke( >> >>>> > DelegatingMethodAccessorImpl.java:43) >> >>>> > >> at java.lang.reflect.Method.invoke(Method.java:498) >> >>>> > >> at org.codehaus.plexus.classworlds.launcher.Launcher. >> >>>> > launchEnhanced(Launcher.java:289) >> >>>> > >> at org.codehaus.plexus.classworlds.launcher.Launcher. >> >>>> > launch(Launcher.java:229) >> >>>> > >> at org.codehaus.plexus.classworlds.launcher.Launcher. >> >>>> > mainWithExitCode(Launcher.java:415) >> >>>> > >> at org.codehaus.plexus.classworlds.launcher.Launcher. >> >>>> > main(Launcher.java:356) >> >>>> > >> Caused by: org.apache.maven.plugin.MojoExecutionException: Error >> >>>> > >> creating shaded jar: 46848 >> >>>> > >> at org.apache.maven.plugins.shade.mojo.ShadeMojo.execute( >> >>>> > ShadeMojo.java:528) >> >>>> > >> at >> >>>> > >> org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo( >> >>>> > DefaultBuildPluginManager.java:134) >> >>>> > >> at org.apache.maven.lifecycle.internal.MojoExecutor.execute( >> >>>> > MojoExecutor.java:207) >> >>>> > >> ... 20 more >> >>>> > >> Caused by: java.lang.ArrayIndexOutOfBoundsException: 46848 >> >>>> > >> at org.objectweb.asm.ClassReader.readClass(Unknown Source) >> >>>> > >> at org.objectweb.asm.ClassReader.accept(Unknown Source) >> >>>> > >> at org.objectweb.asm.ClassReader.accept(Unknown Source) >> >>>> > >> at org.vafer.jdependency.Clazzpath.addClazzpathUnit( >> >>>> > Clazzpath.java:94) >> >>>> > >> at org.apache.maven.plugins.shade.filter.MinijarFilter.< >> >>>> > init>(MinijarFilter.java:77) >> >>>> > >> at org.apache.maven.plugins.shade.mojo.ShadeMojo. >> >>>> > getFilters(ShadeMojo.java:767) >> >>>> > >> at org.apache.maven.plugins.shade.mojo.ShadeMojo.execute( >> >>>> > ShadeMojo.java:445) >> >>>> > >> ... 22 more >> >>>> > >> [ERROR] >> >>>> > >> [ERROR] >> >>>> > >> [ERROR] For more information about the errors and possible >> >>>> > >> solutions, >> >>>> > >> please read the following articles: >> >>>> > >> [ERROR] [Help 1] >> >>>> > >> >> >>>> > >> http://cwiki.apache.org/confluence/display/MAVEN/ >> > MojoExecutionException >> >>>> > >> [ERROR] >> >>>> > >> >> >>>> > >> >> >>>> > >> It seems to strange to me as the last commit >> >>>> > >> (f710e5a44569aa080b80eff855fff2c3810957fd) comes from me and at >> the >> >>>> > >> time of the PR it was working fine. >> >>>> > >> On Jenkins the build seems fine >> >>>> > >> >> >>>> > >> If I checkout other PRs which are based on previus commits, like >> > the >> >>>> > >> ZooKeeper 3.5.x version PR the build works fine >> >>>> > >> >> >>>> > >> does anyone else have the same problem ? >> >>>> > >> >> >>>> > >> Enrico >> >>>> > >> >>> >> >>> -- >> >>> >> >>> >> >>> -- Enrico Olivelli >> >> >> >> -- >> >> >> >> >> >> -- Enrico Olivelli >>
