Re: Regression in OpenJDK8 Makefiles

2011-07-27 Thread Dr Andrew John Hughes
On 17:12 Wed 27 Jul , Kelly O'Hair wrote:
> 
> On Jul 27, 2011, at 4:28 PM, Dr Andrew John Hughes wrote:
> 
> > On 11:58 Wed 27 Jul , Kelly O'Hair wrote:
> >> 
> >> On Jul 27, 2011, at 11:04 AM, Dr Andrew John Hughes wrote:
> >> 
> >>> Hi,
> >>> 
> >>> Can someone please tell me why:
> >>> 
> >>> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf4edfcd7119
> >>> 
> >>> reverted my earlier fix:
> >>> 
> >>> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/80368890a2a0
> >>> 
> >>> without any discussion?
> >> 
> >> My apologies, the webrev should have been made public.
> >> 
> >>> 
> >>> The correct fix would have been to bump the boot source language/target 
> >>> class
> >>> versions to 7, not erase the lines altogether.
> >> 
> >> Unfortunately, that did not work with jdk7 as a boot
> > 
> > Why was this?  I don't understand why anything would fail with javac 
> > -source 7 -target 7 over
> > just javac.
> 
> Basically, we never really know what the BOOT javac will accept.

I was referring more to the actual build failure when you tried this.

If we're requiring 7 support as a minimum for bootstrapping, surely we know it 
will support this.

> I assume that if we use the default for javac, that the BOOT java can run 
> them.

This is the assumption that has not always held in my experience.

Being explicit means that a newer javac can be used with an older VM.

snip...

> 
> If the BOOT jdk was jdk8, I would want these sources built for jdk8 and run 
> with a jdk8,
> it should just work.
> 

This seems to contradict what you're saying about the files ending up in the 
image.  Why
does it matter if they use the minimum supported version?  Presumably an 
OpenJDK8 VM will
be able to run 1.7 class files.  But an OpenJDK7 VM wouldn't be able to run 1.8 
class files
if these are the default produced by the bootstrap javac.

> -kto
> 
> 
> > 
> >> -kto
> >> 
> >>> -- 
> >>> Andrew :)
> >>> 
> >>> Free Java Software Engineer
> >>> Red Hat, Inc. (http://www.redhat.com)
> >>> 
> >>> Support Free Java!
> >>> Contribute to GNU Classpath and IcedTea
> >>> http://www.gnu.org/software/classpath
> >>> http://icedtea.classpath.org
> >>> PGP Key: F5862A37 (https://keys.indymedia.org/)
> >>> Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37
> >> 
> > 
> > -- 
> > Andrew :)
> > 
> > Free Java Software Engineer
> > Red Hat, Inc. (http://www.redhat.com)
> > 
> > Support Free Java!
> > Contribute to GNU Classpath and IcedTea
> > http://www.gnu.org/software/classpath
> > http://icedtea.classpath.org
> > PGP Key: F5862A37 (https://keys.indymedia.org/)
> > Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37


Re: Regression in OpenJDK8 Makefiles

2011-07-27 Thread Kelly O'Hair

On Jul 27, 2011, at 4:28 PM, Dr Andrew John Hughes wrote:

> On 11:58 Wed 27 Jul , Kelly O'Hair wrote:
>> 
>> On Jul 27, 2011, at 11:04 AM, Dr Andrew John Hughes wrote:
>> 
>>> Hi,
>>> 
>>> Can someone please tell me why:
>>> 
>>> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf4edfcd7119
>>> 
>>> reverted my earlier fix:
>>> 
>>> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/80368890a2a0
>>> 
>>> without any discussion?
>> 
>> My apologies, the webrev should have been made public.
>> 
>>> 
>>> The correct fix would have been to bump the boot source language/target 
>>> class
>>> versions to 7, not erase the lines altogether.
>> 
>> Unfortunately, that did not work with jdk7 as a boot
> 
> Why was this?  I don't understand why anything would fail with javac -source 
> 7 -target 7 over
> just javac.

Basically, we never really know what the BOOT javac will accept.
I assume that if we use the default for javac, that the BOOT java can run them.

> 
> , and jdk6 won't work as a boot jdk
>> soon anyway.
> 
> Well, yes I assumed this patch was related to moving to OpenJDK7 as the 
> minimum bootstrap JDK.
> 
>> Since we will be requiring a jdk7 as the boot jdk, I did not feel it was 
>> needed
>> to even specify this. Anything compiled with the boot javac technically 
>> doesn't care what
>> the source/target is, or should, and the class files created just need to 
>> work with the boot jdk,
>> and should not be shipped as part of the jdk8 being built.
>> 
>> When I looked at the Makefiles, there was no comment as to why we even had 
>> to set the -source
>> or -target options when using the boot javac at all.
>> My conclusion was that it was unnecessary, and deleting these lines made 
>> 'jdk7 as boot' work.
>> 
>> If that was wrong I apologize, why does this matter?
>> 
> 
> It means that we are depending on whatever defaults the bootstrap
> javac uses rather than being explicit.  In most cases, that might not
> cause a problem but I know I've run into problems with this and I
> think it better to be safe than sorry, as you don't know what the
> bootstrap javac is or what its default is.  If we expect the
> bootstrap javac to produce java 7 class files, it should be set
> explicitly:
> 
> http://blogs.oracle.com/darcy/entry/build_advice_set_source_target
> 
> I think I've hit it mostly with using ecj as the bootstrap javac, but
> it couldcause an issue during development if the bootstrap javac
> started producing 1.8 code by default but the VM used wasn't capable of
> handling it.  Unlikely maybe, but I think it's better to have this set
> explicitly and just avoid such issues altogether.

IF these class files were becoming part of the final jdk image, then I would 
agree,
but they aren't. I see no reason to be explicit here, or to even specify it.

I am assuming that the BOOT javac comes from a BOOT jdk image, and that it
has a java command in that BOOT jdk, that can run the default classes created 
by that
BOOT javac.

So what is the BOOT javac used for?

As far as I know, it is used to compile the bootstrap langtools javac.jar and 
it is used to
build the jdk/make/tools jar files, both of which we run during the build but 
we do not
deliver into the final jdk image.
Both are run with the BOOT jdk java command.

The sources for langtools and these jdk/make/tools might be jdk5 or jdk6 or jdk7
sources, but that's an issue for those sources to deal with, in terms of 
checking that
the BOOT jdk is 'good enough' to compile them, and the BOOT java is good enough 
to run them.
If the building of these particular sources NEEDS to set the source/target, 
again, that's
something for that particular part of the build to specify.

If the BOOT jdk was jdk8, I would want these sources built for jdk8 and run 
with a jdk8,
it should just work.

-kto


> 
>> -kto
>> 
>>> -- 
>>> Andrew :)
>>> 
>>> Free Java Software Engineer
>>> Red Hat, Inc. (http://www.redhat.com)
>>> 
>>> Support Free Java!
>>> Contribute to GNU Classpath and IcedTea
>>> http://www.gnu.org/software/classpath
>>> http://icedtea.classpath.org
>>> PGP Key: F5862A37 (https://keys.indymedia.org/)
>>> Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37
>> 
> 
> -- 
> Andrew :)
> 
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
> 
> Support Free Java!
> Contribute to GNU Classpath and IcedTea
> http://www.gnu.org/software/classpath
> http://icedtea.classpath.org
> PGP Key: F5862A37 (https://keys.indymedia.org/)
> Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37



Re: Regression in OpenJDK8 Makefiles

2011-07-27 Thread Dr Andrew John Hughes
On 11:58 Wed 27 Jul , Kelly O'Hair wrote:
> 
> On Jul 27, 2011, at 11:04 AM, Dr Andrew John Hughes wrote:
> 
> > Hi,
> > 
> > Can someone please tell me why:
> > 
> > http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf4edfcd7119
> > 
> > reverted my earlier fix:
> > 
> > http://hg.openjdk.java.net/jdk8/tl/jdk/rev/80368890a2a0
> > 
> > without any discussion?
> 
> My apologies, the webrev should have been made public.
> 
> > 
> > The correct fix would have been to bump the boot source language/target 
> > class
> > versions to 7, not erase the lines altogether.
> 
> Unfortunately, that did not work with jdk7 as a boot

Why was this?  I don't understand why anything would fail with javac -source 7 
-target 7 over
just javac.

, and jdk6 won't work as a boot jdk
> soon anyway.

Well, yes I assumed this patch was related to moving to OpenJDK7 as the minimum 
bootstrap JDK.

> Since we will be requiring a jdk7 as the boot jdk, I did not feel it was 
> needed
> to even specify this. Anything compiled with the boot javac technically 
> doesn't care what
> the source/target is, or should, and the class files created just need to 
> work with the boot jdk,
> and should not be shipped as part of the jdk8 being built.
> 
> When I looked at the Makefiles, there was no comment as to why we even had to 
> set the -source
> or -target options when using the boot javac at all.
> My conclusion was that it was unnecessary, and deleting these lines made 
> 'jdk7 as boot' work.
> 
> If that was wrong I apologize, why does this matter?
> 

It means that we are depending on whatever defaults the bootstrap
javac uses rather than being explicit.  In most cases, that might not
cause a problem but I know I've run into problems with this and I
think it better to be safe than sorry, as you don't know what the
bootstrap javac is or what its default is.  If we expect the
bootstrap javac to produce java 7 class files, it should be set
explicitly:

http://blogs.oracle.com/darcy/entry/build_advice_set_source_target

I think I've hit it mostly with using ecj as the bootstrap javac, but
it couldcause an issue during development if the bootstrap javac
started producing 1.8 code by default but the VM used wasn't capable of
handling it.  Unlikely maybe, but I think it's better to have this set
explicitly and just avoid such issues altogether.

> -kto
> 
> > -- 
> > Andrew :)
> > 
> > Free Java Software Engineer
> > Red Hat, Inc. (http://www.redhat.com)
> > 
> > Support Free Java!
> > Contribute to GNU Classpath and IcedTea
> > http://www.gnu.org/software/classpath
> > http://icedtea.classpath.org
> > PGP Key: F5862A37 (https://keys.indymedia.org/)
> > Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37
> 

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37


Re: Regression in OpenJDK8 Makefiles

2011-07-27 Thread Kelly O'Hair

On Jul 27, 2011, at 11:04 AM, Dr Andrew John Hughes wrote:

> Hi,
> 
> Can someone please tell me why:
> 
> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cf4edfcd7119
> 
> reverted my earlier fix:
> 
> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/80368890a2a0
> 
> without any discussion?

My apologies, the webrev should have been made public.

> 
> The correct fix would have been to bump the boot source language/target class
> versions to 7, not erase the lines altogether.

Unfortunately, that did not work with jdk7 as a boot, and jdk6 won't work as a 
boot jdk
soon anyway. Since we will be requiring a jdk7 as the boot jdk, I did not feel 
it was needed
to even specify this. Anything compiled with the boot javac technically doesn't 
care what
the source/target is, or should, and the class files created just need to work 
with the boot jdk,
and should not be shipped as part of the jdk8 being built.

When I looked at the Makefiles, there was no comment as to why we even had to 
set the -source
or -target options when using the boot javac at all.
My conclusion was that it was unnecessary, and deleting these lines made 'jdk7 
as boot' work.

If that was wrong I apologize, why does this matter?

-kto

> -- 
> Andrew :)
> 
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
> 
> Support Free Java!
> Contribute to GNU Classpath and IcedTea
> http://www.gnu.org/software/classpath
> http://icedtea.classpath.org
> PGP Key: F5862A37 (https://keys.indymedia.org/)
> Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37