Re: [Engine-devel] java 1.6 compatibility no more?

From: "Juan Hernandez" <jhernand(a)redhat.com&gt; To: "Allon Mureinik" <amureini(a)redhat.com&gt; Cc: engine-devel(a)ovirt.org 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" <jhernand(a)redhat.com&gt; >> To: "Allon Mureinik" <amureini(a)redhat.com&gt; >> Cc: "Itamar Heim" <iheim(a)redhat.com&gt;, engine-devel(a)ovirt.org >> 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" <iheim(a)redhat.com&gt; >>>> To: "Allon Mureinik" <amureini(a)redhat.com&gt; >>>> Cc: "Livnat Peer" <lpeer(a)redhat.com&gt;, "Juan Hernandez" >>>> <jhernand(a)redhat.com&gt;, engine-devel(a)ovirt.org, "Michael >>>> Kublin" <mkublin(a)redhat.com&gt; >>>> 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" <iheim(a)redhat.com&gt; >>>>>> To: "Allon Mureinik" <amureini(a)redhat.com&gt; >>>>>> Cc: "Livnat Peer" <lpeer(a)redhat.com&gt;, "Juan Hernandez" >>>>>> <jhernand(a)redhat.com&gt;, engine-devel(a)ovirt.org, "Michael >>>>>> Kublin" <mkublin(a)redhat.com&gt; >>>>>> 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" <lpeer(a)redhat.com&gt; >>>>>>>> To: "Itamar Heim" <iheim(a)redhat.com&gt;, "Michael Kublin" >>>>>>>> <mkublin(a)redhat.com&gt; >>>>>>>> Cc: "Juan Hernandez" <jhernand(a)redhat.com&gt;, >>>>>>>> engine-devel(a)ovirt.org >>>>>>>> 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.

>> 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.

>> 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.