[PATCH FOR REVIEW]: Make source/target options explicit for CORBA bootstrap tools

[Jonathan Gibbons]

Andrew John Hughes wrote:
> 2009/8/18 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>   
>> 2009/8/18 Jonathan Gibbons <Jonathan.Gibbons at sun.com>:
>>     
>>> Andrew,
>>>
>>> If this is a patch for jdk7, it does not appear to be a patch to a recent
>>> copy
>>> of 7.
>>>       
>> It's against b69 which is the latest release (from Friday).  The
>> patches are against the IcedTea forest so builds can be tested with
>> IcedTea as well.
>>
>> Specifically, you do not seem to have the recent changeset  to set
>>     
>>> the source/target used to compile JDK to 7. [1]
>>>
>>>       
>> Er... yes I do:
>>
>> # Add the source level
>> LANGUAGE_VERSION = -source 7
>> JAVACFLAGS  += $(LANGUAGE_VERSION)
>>
>> # Add the class version we want
>> TARGET_CLASS_VERSION = 7
>> CLASS_VERSION = -target $(TARGET_CLASS_VERSION)
>> JAVACFLAGS  += $(CLASS_VERSION)
>> JAVACFLAGS  += -encoding ascii
>> JAVACFLAGS  += -classpath $(BOOTDIR)/lib/tools.jar
>> JAVACFLAGS  += $(OTHER_JAVACFLAGS)
>>
>> but these only cover the rt classes and not the bootstrap classes.
>>
>>     
>>> While your patch does not directly conflict with any edits in that patch,
>>> and
>>> while the effect of your patch looks OK, in that patch I was extending the
>>> precedent of TARGET_CLASS_VERSION to have an explicit macro for
>>> (just) the version number, so that it is easy to change the value of (just)
>>> the version number from the command line.
>>>
>>> With that in mind, I would suggest something like the following for your
>>> patch:
>>>
>>> BOOT_SOURCE_LANGUAGE_VERSION = 6
>>> BOOT_TARGET_CLASS_VERSION = 6
>>> BOOT_JAVACFLAGS  += -encoding ascii -source $(BOOT_SOURCE_LANGUAGE_VERSION)
>>> -target $(BOOT_TARGET_CLASS_VERSION)
>>>
>>>       
>> I didn't copy this for the 6 changes because I didn't immediately see
>> the point of using variables just for this single use.  I forgot that
>> it is possible to override these from the command line, so I've update
>> the patch:
>>
>> http://cr.openjdk.java.net/~andrew/ecj/02/webrev.02/
>>
>>     
>>> -- Jon
>>>
>>> [1]
>>> http://mail.openjdk.java.net/pipermail/compiler-dev/2009-July/001286.html
>>>
>>>
>>> On Aug 18, 2009, at 5:24 AM, Andrew John Hughes wrote:
>>>
>>>       
>>>> Currently the javac calls for building the bootstrap tools (not the
>>>> classes for the final JDK, which correctly now use source and target
>>>> 7) don't set an explicit source and target version.
>>>>
>>>> The webrev:
>>>>
>>>> http://cr.openjdk.java.net/~andrew/ecj/02/webrev.01/
>>>>
>>>> sets these to 6 explicitly, as happens in the Ant builds performed by
>>>> langtools/jaxp/jaxws.  This is noticeable especially when using ecj as
>>>> the bootstrap javac as it defaults to a version < 1.5, and the build
>>>> fails.
>>>>
>>>> Ok to push?
>>>>
>>>> Thanks,
>>>> --
>>>> Andrew :-)
>>>>
>>>> Free Java Software Engineer
>>>> Red Hat, Inc. (http://www.redhat.com)
>>>>
>>>> Support Free Java!
>>>> Contribute to GNU Classpath and the OpenJDK
>>>> http://www.gnu.org/software/classpath
>>>> http://openjdk.java.net
>>>>
>>>> PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
>>>> Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8
>>>>         
>>>       
>>
>> --
>> Andrew :-)
>>
>> Free Java Software Engineer
>> Red Hat, Inc. (http://www.redhat.com)
>>
>> Support Free Java!
>> Contribute to GNU Classpath and the OpenJDK
>> http://www.gnu.org/software/classpath
>> http://openjdk.java.net
>>
>> PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
>> Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8
>>
>>     
>
> Is this version now ok?  If so, I'll push it to the build gate using
> the bug ID Kelly allocated for the same fix in JDK.
>   


Andrew,

I approve your webrev http://cr.openjdk.java.net/~andrew/ecj/02/webrev.02/
My earlier confusion was caused by the fact that the corba Makefile is not
consistent with the jdk Makefile with respect to the use of 
SOURCE_LANGUAGE_VERSION.
It would be good to (separately) fix that inconsistency, but that does not
affect the validity of what you propose here.

-- Jon
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/build-dev/attachments/20090819/b2c09ba6/attachment.htm>