[Engine-devel] java 1.6 compatibility no more?

Mon Jul 23 16:33:16 UTC 2012

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

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