Suggested backports of JDK 7 langtools fixes to OpenJDK 6

[Joseph D. Darcy]

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.

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.

Thanks,

-Joe