----- Original Message ----- > From: "Juan Hernandez" <[email protected]> > To: "Allon Mureinik" <[email protected]> > Cc: [email protected] > Sent: Monday, July 23, 2012 2:31:24 PM > Subject: Re: [Engine-devel] java 1.6 compatibility no more? > > On 07/23/2012 01:02 PM, Allon Mureinik wrote: > > > > > > ----- Original Message ----- > >> From: "Juan Hernandez" <[email protected]> > >> To: "Allon Mureinik" <[email protected]> > >> Cc: "Itamar Heim" <[email protected]>, [email protected] > >> Sent: Monday, July 23, 2012 1:22:37 PM > >> Subject: Re: [Engine-devel] java 1.6 compatibility no more? > >> > >> On 07/23/2012 11:46 AM, Allon Mureinik wrote: > >>> > >>> > >>> ----- Original Message ----- > >>>> From: "Itamar Heim" <[email protected]> > >>>> To: "Allon Mureinik" <[email protected]> > >>>> Cc: "Livnat Peer" <[email protected]>, "Juan Hernandez" > >>>> <[email protected]>, [email protected], "Michael > >>>> Kublin" <[email protected]> > >>>> Sent: Monday, July 23, 2012 8:43:02 AM > >>>> Subject: Re: [Engine-devel] java 1.6 compatibility no more? > >>>> > >>>> On 07/23/2012 08:29 AM, Allon Mureinik wrote: > >>>>> > >>>>> > >>>>> ----- Original Message ----- > >>>>>> From: "Itamar Heim" <[email protected]> > >>>>>> To: "Allon Mureinik" <[email protected]> > >>>>>> Cc: "Livnat Peer" <[email protected]>, "Juan Hernandez" > >>>>>> <[email protected]>, [email protected], "Michael > >>>>>> Kublin" <[email protected]> > >>>>>> Sent: Sunday, July 22, 2012 7:41:00 PM > >>>>>> Subject: Re: [Engine-devel] java 1.6 compatibility no more? > >>>>>> > >>>>>> On 07/22/2012 07:38 PM, Allon Mureinik wrote: > >>>>>>> > >>>>>>> > >>>>>>> ----- Original Message ----- > >>>>>>>> From: "Livnat Peer" <[email protected]> > >>>>>>>> To: "Itamar Heim" <[email protected]>, "Michael Kublin" > >>>>>>>> <[email protected]> > >>>>>>>> Cc: "Juan Hernandez" <[email protected]>, > >>>>>>>> [email protected] > >>>>>>>> Sent: Sunday, July 22, 2012 9:50:47 AM > >>>>>>>> Subject: Re: [Engine-devel] java 1.6 compatibility no more? > >>>>>>>> > >>>>>>>> On 21/07/12 15:15, Itamar Heim wrote: > >>>>>>>>> On 07/19/2012 03:34 PM, Ayal Baron wrote: > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> ----- Original Message ----- > >>>>>>>>>>> > >>>>>>>>>>> On Jul 19, 2012, at 14:14 , Livnat Peer wrote: > >>>>>>>>>>> > >>>>>>>>>>>> On 19/07/12 14:41, Juan Hernandez wrote: > >>>>>>>>>>>>> On 07/19/2012 01:39 PM, Yair Zaslavsky wrote: > >>>>>>>>>>>>>> On 07/19/2012 02:31 PM, Vojtech Szocs wrote: > >>>>>>>>>>>>>>>> Don't we need that (the source part) to avoid Java 7 > >>>>>>>>>>>>>>>> syntax > >>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>> GWT code? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> That's a very good point. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> In general, GWT compiler supports Java 5 syntax (note > >>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>> there > >>>>>>>>>>>>>>> are no language changes between Java 5 and 6). For > >>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>> reason, > >>>>>>>>>>>>>>> our frontend code should be compliant with Java 5. If > >>>>>>>>>>>>>>> someone > >>>>>>>>>>>>>>> uses new Java 7 language features in frontend code, > >>>>>>>>>>>>>>> GWT > >>>>>>>>>>>>>>> compiler will throw an error and the build will fail. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> So the 'Java 5 only' limitation applies to frontend > >>>>>>>>>>>>>>> code > >>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>> any > >>>>>>>>>>>>>>> other code (e.g. shared modules) that is directly > >>>>>>>>>>>>>>> referenced > >>>>>>>>>>>>>>> by > >>>>>>>>>>>>>>> frontend code. This shouldn't affect the backend, > >>>>>>>>>>>>>>> however. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> We could do something like this: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> - let oVirt root POM declare source and target > >>>>>>>>>>>>>>> compliance > >>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>> Java 7 > >>>>>>>>>>>>>>> - let frontend modules POM > >>>>>>>>>>>>>>> (frontend/webadmin/modules/pom.xml) > >>>>>>>>>>>>>>> declare source compliance to Java 5 (or 6) > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> (note that target compliance can be left to Java 7 > >>>>>>>>>>>>>>> since > >>>>>>>>>>>>>>> frontend compilation results in JavaScript code) > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> What do you think? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Vojtech > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> +1 - I really like this idea! > >>>>>>>>>>>>> > >>>>>>>>>>>>> +1 from me as well. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> There are two calls to make when it comes to JDK7 > >>>>>>>>>>>> (regardless > >>>>>>>>>>>> of > >>>>>>>>>>>> GWT - > >>>>>>>>>>>> excuse me for taking this discussion some steps > >>>>>>>>>>>> backwards) > >>>>>>>>>>>> > >>>>>>>>>>>> 1. Are we running with JRE 7? > >>>>>>>>>>>> The answer is yes we agreed on that a few months ago. > >>>>>>>>>>>> > >>>>>>>>>>>> 2. Are we using code syntax which is incompatible with > >>>>>>>>>>>> JDK6? > >>>>>>>>>>>> > >>>>>>>>>>>> I think the answer to the above should be no (at least > >>>>>>>>>>>> for > >>>>>>>>>>>> now > >>>>>>>>>>>> - > >>>>>>>>>>>> maybe > >>>>>>>>>>>> until the next ovirt release?). > >>>>>>>>>>> +1 > >>>>>>>>>>> exactly. Why starting with jdk6 incompatible constructs > >>>>>>>>>>> unless > >>>>>>>>>>> there > >>>>>>>>>>> is a good (or at least any) reason for them… > >>>>>>>>>> > >>>>>>>>>> +1 > >>>>>>>>> > >>>>>>>>> +1 - there is merit keeping backward compatibility to allow > >>>>>>>>> comparing > >>>>>>>>> behavior while java 7 is still young. > >>>>>>>> > >>>>>>>> Since no one objected, we'll go with JDK6 syntax > >>>>>>>> compatibility > >>>>>>>> for > >>>>>>>> Now. > >>>>>>> I'm a very small fan of enforcing policy by reviewers. > >>>>>>> Not that the community reviews aren't great - but people miss > >>>>>>> things. > >>>>>>> > >>>>>>> Here's my take on Maven's enforcer plugin to actually verify > >>>>>>> we > >>>>>>> aren't compiling with JDK 7: > >>>>>>> http://gerrit.ovirt.org/#/c/6523 > >>>>>> > >>>>>> we don't want to enforce compilation or run with JDK 6, only > >>>>>> to > >>>>>> preserve > >>>>>> backward compatibility. > >>>>>> I'm for jenkins to have a job to compile and run unitests with > >>>>>> openjdk 6 > >>>>>> to be on the safe side. > >>>>> > >>>>> I don't understand this suggestion. > >>>>> What you're saying is that you can compile with whatever JDK > >>>>> you > >>>>> want, but: > >>>>> - it won't compile with JDKs prior to 6, since we're using 6's > >>>>> features. > >>>>> - you aren't allowed to use JDK 7 features, and if you do, > >>>>> you'll > >>>>> get an email from jenkins that you broke something and must fix > >>>>> it. > >>>>> > >>>>> To me, this sounds a lot like enforcing JDK 6 compatibility. > >>>>> > >>>> > >>>> its preserving jdk 6 compatibility for a few more months, not > >>>> enforcing > >>>> to use jdk 6 compiler. > >>> Fair enough. > >>> > >>>> > >>>>> /today/ if have way too many (i.e., >0) jenkins breaks, a lot > >>>>> of > >>>>> which could be avoided by not running with -DskipTetst or > >>>>> making > >>>>> sure to run with -Penable-dao-tests. > >>>>> I fear this suggestion will just add to this "noise", and could > >>>>> easily be avoided. > >>>> > >>>> jenkins breaks should be visible at patch level prior to commit, > >>>> something we are trying to resolve by adding more hardware to > >>>> allow > >>>> running the various tests at patch level rather than post commit > >>>> only. > >>> I agree that this is an excellent goal, but I maintain that this > >>> is > >>> an uncomfortable way to work. > >>> I would still like a way to check, on my own machine, as part of > >>> my > >>> compilation process, that I'm not doing anything I shouldn't. > >>> Here's my second take on the issue, using Animal Sniffer > >>> (http://mojo.codehaus.org/animal-sniffer/): > >>> http://gerrit.ovirt.org/#/c/6540 > >>> > >>> Again, comments welcome. > >> > >> Before going ahead I would check that using it doesn't increase > >> the > >> already long compilation time to an unacceptable level. > > mvn clean install on my machine took just over 5 minutes - not too > > bad considering that up till a month ago or so it took 15-20 > > minutes to run the test suite. > > Can you compare the build with and without animal-sniffer? Just to > have > an idea of what is the difference. Anyhow five minutes seems > acceptable > to me.
The overhead is roughly 50 seconds: using [merged] commit hash 5845c732560dc325132661e1d1260de0a096c6b7 and the animal-sniffer patch rebased on top of it: mvn clean install -DskipTests: 2:55.76 minutes mvn clean install: 4:49.28 minutes using [merged] commit hash 5845c732560dc325132661e1d1260de0a096c6b7: mvn clean install -DskipTests: 2:05:40 minutes mvn clean install: 3:57.68 minutes [Note: this is done on my personal machine, with everything closed except the terminal running mvn, but this is still hardly a strict benchmark] > > >> Also need to make sure that the new dependency is available in the > >> build > >> environments we use. > > Built it against brew's repo, seemed to work fine. > > If you have any more suggestions on how to check it, please advise. > > That is very good. Actually building there would be even better. > > >> I am specially concerned about the Fedora build > >> system, where we have the plugin but not the signatures for the > >> JDKs. > > The signatures are provided as part of the plugin - they are not > > taken directly from the JDK. > > Or am I missing something in your point? > > I don't know very well this plugin, but it is my understanding that > the > signatures are additional dependencies that need to be downloaded > from > the maven repository. In your patch you are using the following: > > <signature> > <groupId>org.jvnet.animal-sniffer</groupId> > <artifactId>java1.6</artifactId> > <version>1.0</version> > </signature> > > This org.jvnet.animal-sniffer:java1.6:1.0 artifact is what I can't > find > in the Fedora build system. Not a big problem, this can be patched > out > while building the package, just a minor inconvenience. Now I get your drift. Odd indeed, and I think this actually may spell a NACK for this patch - we don't want to add more inconveniences to the build process. Perhaps we should add this check as an optional step in maven, like you originally suggested? That way developers have a decent tool for performing this check, without having to necessarily interfere with the build system. > > >> This means that we will need to ignore the plugin or build the > >> signatures ourselves. > > Not so - see above. > > > >> > >> Also take into account that every new maven plugin we add to the > >> POMs > >> introduces new potential problems with the maven eclipse support. > > True. > > IMHO, it's a small price to pay, but I guess that's why we discuss > > things upstream - to get different opinions ;-) > > Well, for me personally the price is actually zero, as I don't use > the > Eclipse maven support ;-) . But I know that many people hates when > they > try to import the projects and they get errors because of plugins > that > Eclipse doesn't understand. Let's see what they think. > > >> I think we can leave the decision to each developer, maybe > >> providing > >> an > >> script that calls "mvn animal-sniffer:check ..." with the right > >> parameters, maybe with git pre-commit hook, to make it more > >> automatic. > > I really don't like this approach, but again - difference of > > opinions :-) > > Let's gather somemore feedback before deciding either way? > > Yes, of course. In my opinion adding this check is a good idea, and > you > already cleared most of the objections. > > >> This combined with the Jenkins checks can be a good compromise. > > -- > Dirección Comercial: C/Jose Bardasano Baos, 9, Edif. Gorbea 3, planta > 3ºD, 28016 Madrid, Spain > Inscrita en el Reg. Mercantil de Madrid – C.I.F. B82657941 - Red Hat > S.L. > > > _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
