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

Thu Aug 27 15:19:45 UTC 2009

On Aug 27, 2009, at 3:53 AM, Andrew John Hughes wrote:

> 2009/8/20 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>> 2009/8/20 Jonathan Gibbons <Jonathan.Gibbons at sun.com>:
>>> 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/
>>
>> Thanks.  Pushed this last night:
>> http://hg.openjdk.java.net/jdk7/build/corba/rev/8001ba2bf10d
>>
>>> 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.
>>>
>>
>> Yes, I see what you mean now.  Here's another webrev:
>>
>> http://cr.openjdk.java.net/~andrew/consistency/01/webrev.01/
>>
>> which should make the two fairly consistent.  It brings in a lot of
>> changes to the version in JDK that seem to have been missed from
>> CORBA, namely:
>>
>>  * Turning off option outputs on fastdebug builds
>>  * Supporting USE_HOTSPOT_INTERPRETER_MODE
>>  * Supporting JAVAC_MAX_WARNINGS and JAVAC_WARNINGS_FATAL
>>  * The SOURCE_LANGUAGE_VERSION sync above
>>  * Include JAR_JFLAGS in BOOT_JAR_JFLAGS
>>
>> The following differences remain, which didn't seem appropriate to  
>> include:
>>
>> -JAVACFLAGS  += -classpath $(BOOTDIR)/lib/tools.jar
>> +JAVACFLAGS  += "-Xbootclasspath:$(CLASSBINDIR)"
>>  JAVACFLAGS  += $(OTHER_JAVACFLAGS)
>>
>>  # Needed for javah
>> -JAVAHFLAGS += -classpath $(CLASSBINDIR)
>> +JAVAHFLAGS += -bootclasspath $(CLASSBINDIR)
>>
>> I wasn't sure of the pros/cons of these changes but can easy add  
>> them if needed.
>>
>>> -- Jon
>>>
>>
>>
>>
>> --
>> 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
>>
>
> Does this change look ok? Can I push it?
>
> 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,

The changes regarding flags for javac look OK.  I can't speak to the  
Hotspot options.

-- Jon