[foreign] RFR 8223413: Improve missing symbols handling in jextract

[Jorn Vernee]

> I also sneak in a fix for intermmitent test failure

Thanks! Was running into this as well.

Cheers,
Jorn

Henry Jen schreef op 2019-05-08 17:21:
> Thanks, pushed. I also sneak in a fix for intermmitent test failure
> for 8223105 on Windows.
> 
> Cheers,
> Henry
> 
> 
> diff -r 34449b3f3b3d
> test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java
> --- a/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java
>  Wed May 08 18:40:07 2019 +0530
> +++ b/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinA.java
>  Wed May 08 08:18:36 2019 -0700
> @@ -33,7 +33,7 @@
>   * @requires os.family == "windows"
>   * @library ..
>   * @run driver JtregJextract -C -DADD -t test.jextract.asmsymbol --
> libAsmSymbol.h
> - * @run testng Test8223105WinA
> + * @run testng/othervm Test8223105WinA
>   */
>  public class Test8223105WinA {
>      static final libAsmSymbol_h libAsmSymbol;
> diff -r 34449b3f3b3d
> test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java
> --- a/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java
>  Wed May 08 18:40:07 2019 +0530
> +++ b/test/jdk/com/sun/tools/jextract/test8223105/Test8223105WinB.java
>  Wed May 08 08:18:36 2019 -0700
> @@ -33,7 +33,7 @@
>   * @requires os.family == "windows"
>   * @library ..
>   * @run driver JtregJextract -t test.jextract.asmsymbol -- 
> libAsmSymbol.h
> - * @run testng Test8223105WinB
> + * @run testng/othervm Test8223105WinB
>   */
>  public class Test8223105WinB {
>      static final libAsmSymbol_h libAsmSymbol;
> 
> 
>> On May 8, 2019, at 2:58 AM, Sundararajan Athijegannathan 
>> <sundararajan.athijegannathan at oracle.com> wrote:
>> 
>> Looks good.
>> 
>> -Sundar
>> 
>> On 07/05/19, 2:45 AM, Henry Jen wrote:
>>> Bug[1] created, and official webrev[2] review request.
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8223413
>>> [2] http://cr.openjdk.java.net/~henryjen/panama/8223413/0/webrev/
>>> 
>>> Cheers,
>>> Henry
>>> 
>>>> On May 3, 2019, at 1:34 PM, Maurizio 
>>>> Cimadamore<maurizio.cimadamore at oracle.com>  wrote:
>>>> 
>>>> If I understand what you are saying, you just want to make explicit 
>>>> the fact that if -l has been set, _some_ library will always be set, 
>>>> either explicitly (via -L) or implicitly (inferred from 
>>>> java.library.path).
>>>> 
>>>> If so, this seems like a good change.
>>>> 
>>>> Maurizio
>>>> 
>>>> On 03/05/2019 18:12, Henry Jen wrote:
>>>>> The current implementation behave correctly as expected, but 
>>>>> reading LibraryLookupFilter feels wrong. It depends on the fact 
>>>>> that linkCheckPaths is initialized to java.library.path when -l is 
>>>>> specified but not -L in Main.java.
>>>>> 
>>>>> At least we can do is to add following change,
>>>>> 
>>>>> --- 
>>>>> a/src/jdk.jextract/share/classes/com/sun/tools/jextract/LibraryLookupFilter.java
>>>>> +++ 
>>>>> b/src/jdk.jextract/share/classes/com/sun/tools/jextract/LibraryLookupFilter.java
>>>>> @@ -86,8 +86,9 @@
>>>>>      }
>>>>> 
>>>>>      private void initSymChecker(List<String>  linkCheckPaths) {
>>>>> -        if (!libraryNames.isEmpty()&&  !linkCheckPaths.isEmpty()) 
>>>>> {
>>>>> +        if (!libraryNames.isEmpty()) {
>>>>>              try {
>>>>> +                assert !linkCheckPaths.isEmpty();
>>>>>                  Library[] libs = 
>>>>> loadLibraries(MethodHandles.lookup(),
>>>>>                          linkCheckPaths.toArray(new String[0]),
>>>>>                          libraryNames.toArray(new String[0]));
>>>>> 
>>>>> Cheers,
>>>>> Henry
>>>>> 
>>>>>> On May 3, 2019, at 10:02 AM, Henry Jen<henry.jen at oracle.com>  
>>>>>> wrote:
>>>>>> 
>>>>>> Correct.
>>>>>> 
>>>>>> Cheers,
>>>>>> Henry
>>>>>> 
>>>>>> 
>>>>>>> On May 3, 2019, at 10:00 AM, Maurizio 
>>>>>>> Cimadamore<maurizio.cimadamore at oracle.com>  wrote:
>>>>>>> 
>>>>>>> Seems like an useful follow up. Just to make sure I understand, 
>>>>>>> if -l is specified, we get same behavior as before. If -l is NOT 
>>>>>>> specified, then the behavior would differ (as we now do the 
>>>>>>> check), and therefore we need the 'ignore' option explicitly, if 
>>>>>>> we want to suppress logging. Right?
>>>>>>> 
>>>>>>> Maurizio
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On 03/05/2019 17:43, Henry Jen wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> Please review a webrev[1] that add the missing —missing-symbols 
>>>>>>>> warn support, and turn on symbol checking against default 
>>>>>>>> library by default.
>>>>>>>> 
>>>>>>>> This is kind of a follow up to JDK-8223247, as that simply 
>>>>>>>> assume we are using the default libraries within JVM if no -l 
>>>>>>>> option is provided. This webrev now will
>>>>>>>> 
>>>>>>>> - Same behavior as before if both -l and -L are provided.
>>>>>>>> - Symbol check is turned on always. If there is no -l provided, 
>>>>>>>> jextract will check symbols against the default library.
>>>>>>>> - Default is to issue warnings without -l, exclude with explicit 
>>>>>>>> -l. This is mostly backward compatible as it doesn’t change 
>>>>>>>> generated code/classes, but show warnings to inform user about 
>>>>>>>> potential missing libraries.
>>>>>>>> 
>>>>>>>> To be 100% compatible with before, use '—missing-symbols ignore’
>>>>>>>> 
>>>>>>>> Thoughts?
>>>>>>>> 
>>>>>>>> Cheers,
>>>>>>>> Henry
>>>>>>>> 
>>>>>>>> [1] 
>>>>>>>> http://cr.openjdk.java.net/~henryjen/panama/missingSymbols/webrev/