Suggested backports of JDK 7 langtools fixes to OpenJDK 6

[Andrew John Hughes]

On 3 March 2010 23:34, Joseph D. Darcy <Joe.Darcy at sun.com> wrote:
> Andrew John Hughes wrote:
>>
>> On 3 March 2010 19:36, Joseph D. Darcy <Joe.Darcy at sun.com> wrote:
>>
>>>
>>> Andrew John Hughes wrote:
>>>
>>>>
>>>> On 3 March 2010 17:24, Joseph D. Darcy <Joe.Darcy at sun.com> wrote:
>>>>
>>>>
>>>>>
>>>>> Jonathan Gibbons wrote:
>>>>>
>>>>>
>>>>>>
>>>>>> Martin Buchholz wrote:
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> On Wed, Mar 3, 2010 at 08:29, Andrew John Hughes
>>>>>>> <gnu_andrew at member.fsf.org> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> On 3 March 2010 00:26, Joseph D. Darcy <Joe.Darcy at sun.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>             -es -esa \
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> I think
>>>>>>> -vmoption:-es -vmoption:-esa
>>>>>>> was meant.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Whoops,
>>>>>>
>>>>>> I think
>>>>>>  -ea -esa
>>>>>> was meant.
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Yes, sorry for the typo!  The options I meant are "-ea" and "-esa" to
>>>>> enable
>>>>> assertions and enable system assertions, respectively.
>>>>>
>>>>> -Joe
>>>>>
>>>>>
>>>>>
>>>>
>>>> With the right options, two tests fail:
>>>>
>>>> FAILED: tools/javac/processing/environment/TestSourceVersion.java:
>>>> javac: invalid source release: 1.7
>>>> FAILED: tools/javac/unicode/Unmappable.java: Unmappable.java:37:
>>>> warning: unmappable character for encoding ascii String s =
>>>> "z\ufffdhler";
>>>>
>>>>
>>>
>>> How are you running these tests to elicit the failures?
>>>
>>>
>>
>> As I listed above when reporting the failures:
>>
>> mkdir -p test/langtools/JTwork test/langtools/JTreport
>> /mnt/builder/icedtea6-hg/bootstrap/jdk1.6.0/bin/java -jar
>> test/jtreg.jar -v1 -a -ignore:quiet \
>>               -w:test/langtools/JTwork -r:test/langtools/JTreport \
>>               -s -jdk:`pwd`/openjdk/build/linux-amd64/j2sdk-image \
>>               -ea -esa \
>>               `pwd`/openjdk/langtools/test \
>>           | tee test/check-langtools.log
>>
>
> Got it.
>
>>
>>>
>>> When I run the tests on OpenJDK 6 langtools tip with -ea -esa, I only see
>>> the one expected failure for the EarlyAssert merge issue.
>>>
>>>
>>
>> Ok, I'm not running on tip but with the webrev applied -- as you
>> originally asked.
>>
>
> Right; I was just listed that as a comparison since there are failures
> reported that are outside of the set of patched test files.
>
>>
>>>
>>> The current OpenJDK 6 version of TestSourceVersion.java doesn't refer to
>>> 1.7; checking your webrev... ah -- the patch should *not* add these
>>> options
>>> to OpenJDK 6.
>>>
>>
>> Ok, so we just drop the changes to that file, right?
>>
>
> Yes, the changes to that file should be dropped.
>
>>
>>>>
>>>> The first seems a correct failure; javac on OpenJDK6 should not support
>>>> 1.7.
>>>> I see different failures on 7:
>>>>
>>>> FAILED: tools/apt/Basics/print.sh: java.lang.AssertionError at
>>>>
>>>>
>>>> com.sun.mirror.util.SourceOrderDeclScanner$SourceOrderComparator.compareEqualPosition(SourceOrderDeclScanner.java:104)
>>>> FAILED: tools/apt/Scanners/scanner.sh: java.lang.AssertionError at
>>>>
>>>>
>>>> com.sun.mirror.util.SourceOrderDeclScanner$SourceOrderComparator.compareEqualPosition(SourceOrderDeclScanner.java:104)
>>>>
>>>>
>>>
>>> Strange; those are the problems corrected by 6926703: apt tests should
>>> run
>>> with assertions enabled.
>>>
>>>
>>
>> Not so such strange when this is b84.  6926703 is only currently in tl
>> and hasn't been promoted yet.  I guess that's also why we see
>> EarlyAssert fail in 7 and not on 6; that change is also only in tl.
>> I ran the tests on the build I had handy just to compare.  I'd need to
>> use IcedTea to build tl in order to test that.
>>
>
> Okay.  Yes, some langtools tests in the promoted JDK 7 forest will will fail
> when  with assertions enabled against the currently promoted bits, but a
> build from the patched version of langtools now in JDK 7 TL will pass.
>
> I've applied your patch to my working OpenJDK 6 langtools forest and the
> changes for another file should be backed out too,
> test/tools/javac/unicode/Unmappable.java.  The default source level in
> OpenJDK 6 (and Sun's propriety JDK 6) is 5 and the encoding issue only
> forces a compile failure under source 6 and higher.  In OpenJDK 7, the
> default source level is 7 so the explicit source option is no longer needed
> for this file.
>
> As an aside, some of these source level intricacies are explained in.
>
> "Source, target, class file version decoder ring"
> http://blogs.sun.com/darcy/entry/source_target_class_file_version
>
> I've put a patch of the amended changes from your webrev (no changes to
> TestSourceVersion.java or test/tools/javac/unicode/Unmappable.java) at
>
> http://cr.openjdk.java.net/~darcy/6843761.langtools.patch
>
> With the current version of jtreg, when this patch is applied, all the
> OpenJDK 6 langtools tests pass when run with assertions enabled and I
> approve this amended version of 6843761 going back into OpenJDK 6.  I can do
> the push or you can, whichever you prefer.
>

The two files have been reverted and the rest of the changes pushed.
The new version passed all tests with -ea -esa.

> Generally, langtools is an early adopter of new jtreg features and the
> "@compile/proc" support is only available on newer versions of jtreg, so
> tests using those features are expected to fail with older versions. I run
> the March 31, 2009 version of jtreg.
>

Yeah, we need to update the IcedTea version at some point.

> Thanks,
>
> -Joe
>



-- 
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