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

Thu Aug 27 15:50:38 UTC 2009

2009/8/27 Jonathan Gibbons <Jonathan.Gibbons at sun.com>:
>
> 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
>
>

Ok, thanks Jonathan.  I'll forward a copy to hotspot-dev for approval
on supporting USE_HOTSPOT_INTERPRETER_MODE.
-- 
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