[foreign] RFR 8222919: jextract should compile generated java sources rather than use ASM to generate class files

[Sundararajan Athijegannathan]

Thanks for your review.

Updated:  http://cr.openjdk.java.net/~sundar/8222919/webrev.03/

-Sundar

On 25/04/19, 8:37 PM, Henry Jen wrote:
> Looks good, it’s nice to have source and actually use javac to generate classes.
>
> - In Writer.java, we change the JEXTRACT_MANIFEST, while it’s good for zip entry, I am not sure how tolerate that is for nio op in line 79.
>
> - In InMemoryJavaCompiler.java:59, the error message says “Test bug”?
>
> Cheers,
> Henry
>
>
>> On Apr 25, 2019, at 3:44 AM, Sundararajan Athijegannathan<sundararajan.athijegannathan at oracle.com>  wrote:
>>
>>
>> Updated: http://cr.openjdk.java.net/~sundar/8222919/webrev.02/
>>
>> Removed unused (asm related) stuff from JType.java
>>
>> PS. All platform tests passed with Internal mach5 job.
>>
>> -Sundar
>>
>> On 24/04/19, 8:44 PM, Sundararajan Athijegannathan wrote:
>>> Thanks.
>>>
>>> Yes, we can start investigating extension/plugin api.
>>>
>>> -Sundar
>>>
>>> On 24/04/19, 8:23 PM, Jorn Vernee wrote:
>>>> Looks good!
>>>>
>>>> Now that the transition to source code generation is complete, and the ASM factories are removed, we can start looking at the plugin/extension story some more I think?
>>>>
>>>> Cheers,
>>>> Jorn
>>>>
>>>> Sundararajan Athijegannathan schreef op 2019-04-24 16:16:
>>>>> Hi,
>>>>>
>>>>> Fixed. Updated webrev: https://cr.openjdk.java.net/~sundar/8222919/webrev.01/
>>>>>
>>>>> Issues:
>>>>>
>>>>> 1) Wrong source file name for static forwarder classes -harmless (as
>>>>> the name is used only in SourceFile attribute) and hence no test
>>>>> failure because of that. But it has been fixed to use correct source
>>>>> file name (with package name rather than just simple name).
>>>>>
>>>>> 2) '\\' in names of Zip/Jarfile:
>>>>>
>>>>> Previously Map<String, byte[]>  used "/" separated names for class
>>>>> names (com/acme/Foo for com.acme.Foo). This masked the bug in
>>>>> JarWriter (which replaced '.' with File.separatorChar - but "." didn't
>>>>> occur in the name at all)! With InMemoryJavaCompiler, names are normal
>>>>> fully qualified classnames (com.acme.Foo) - which JarWriter
>>>>> transformed with "\\" (File.separatorChar) on Windows! Resulting in
>>>>> zip/jar file containing '\\' chars! This is a bug per Zip standard.
>>>>>
>>>>> https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT
>>>>>
>>>>> "4.4.17.1 The name of the file, with optional relative path. The path
>>>>> stored MUST NOT contain a drive or device letter, or a leading slash.
>>>>> All slashes MUST be forward slashes '/' as opposed to backwards
>>>>> slashes '\' for compatibility with Amiga and UNIX file systems etc. If
>>>>> input came from standard input, there is no file name field. "
>>>>>
>>>>> :)
>>>>>
>>>>> PS. Jorn and I discussed this via private emails as well - found the
>>>>> same issue independently! Submitting internal mach5 job concurrently.
>>>>>
>>>>> Thanks,
>>>>> -Sundar
>>>>>
>>>>> On 24/04/19, 6:48 PM, Sundararajan Athijegannathan wrote:
>>>>>> Hi Jorn,
>>>>>>
>>>>>> Yes, I see 3 test failures on Windows (only) with internal mach5 build. I'll check this out. Thanks.
>>>>>>
>>>>>> -Sundar
>>>>>>
>>>>>> On 24/04/19, 4:35 PM, Jorn Vernee wrote:
>>>>>>> Hi Sundar,
>>>>>>>
>>>>>>> In InMemoryJavaCompiler.FileManager::getJavaFileForOutput; Should this use computeIfAbsent instead of put? You're probably more aware of the backing implementation. Is there any chance a file with the same name is requested twice, and then the previously created ClassFile object being overwritten?
>>>>>>>
>>>>>>> Also, there are some tests failing. This seems to be due to Unix vs. Windows path separators, for instance in the Runner test:
>>>>>>>
>>>>>>>   test Runner.testJarManifest(): failure
>>>>>>>   java.lang.AssertionError: Sets differ: expected [com.acme.pad_h, com.acme.pad_h$anon$pad_h$1195, com.acme.pad_h$PaddyStruct] but got [com\\acme\\pad_h$anon$pad_h$1195, com\\acme\\pad_h, com\\acme\\pad_h$PaddyStruct]
>>>>>>>     at org.testng.Assert.fail(Assert.java:94)
>>>>>>>
>>>>>>> I'm looking into this right now, but maybe you know where the problem might be?
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Jorn
>>>>>>>
>>>>>>> Sundararajan Athijegannathan schreef op 2019-04-24 11:34:
>>>>>>>> Please review.
>>>>>>>>
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8222919
>>>>>>>> Webrev: https://cr.openjdk.java.net/~sundar/8222919/webrev.00/
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> -Sundar