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

Andrew John Hughes gnu_andrew at member.fsf.org
Tue Sep 1 22:24:12 UTC 2009 

2009/9/1 Jonathan Gibbons <Jonathan.Gibbons at sun.com>:
> Kelly may not be online today; I'll file it if you like.
>
> -- Jon
>

Thanks Jon.  I don't mind who does it as long as I get an ID for commit :)
I'd do it myself, but there's a delay for obtaining one for external users.


> Andrew John Hughes wrote:
>>
>> 2009/8/27 Andrew John Hughes <gnu_andrew at member.fsf.org>:
>>
>>>
>>> 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
>>>
>>>
>>
>> The HotSpot developers don't see any issue with
>> USE_HOTSPOT_INTERPRETER_MODE:
>>
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2009-September/002032.html
>>
>> and it is already in the JDK makefiles.
>>
>> If someone could allocate me a bug ID for this, I'll 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

More information about the build-dev mailing list