FSInfo#getJarClassPath throws an exception not declared in its throws clause

[Jaikiran Pai]

Thank you Jon.

-Jaikiran

On 14/10/19 7:54 PM, Jonathan Gibbons wrote:
>
> Jaikiran,
>
> I'll take a look and sponsor for you.
>
> -- Jon
>
> On 10/13/19 6:55 PM, Jaikiran Pai wrote:
>>
>> Hello Jon,
>>
>> Thank you for the review. I have taken your inputs and updated the
>> patch to include this change. I have uploaded that patch as a webrev,
>> in the RFR thread.
>>
>> -Jaikiran
>>
>> On 14/10/19 1:34 AM, Jonathan Gibbons wrote:
>>>
>>> Or, ...
>>>
>>>  111             for (StringTokenizer st = new StringTokenizer(path);
>>>  112                  st.hasMoreTokens(); ) {
>>>  113                 String elt = st.nextToken();
>>> 115 try {
>>> 116 Path f = FileSystems.getDefault().getPath(elt);
>>>  121                     if (!f.isAbsolute() && parent != null)
>>>  122                         f = parent.resolve(f).toAbsolutePath();
>>>  126                     list.add(f);
>>>  123 } catch (InvalidPathException | IOError e) {
>>> 124 throw new IOException(e);
>>> 125 }
>>>  127             }
>>>
>>> -- Jon
>>>
>>>
>>> On 10/13/19 1:00 PM, Jonathan Gibbons wrote:
>>>>
>>>> Jaikiran,
>>>>
>>>> A slightly simpler patch with the same effective functionality
>>>> would be to use a single try-catch block with a multi-catch
>>>>
>>>>  111             for (StringTokenizer st = new StringTokenizer(path);
>>>>  112                  st.hasMoreTokens(); ) {
>>>>  113                 String elt = st.nextToken();
>>>> 114 Path f = null;
>>>> 115 try {
>>>> 116 f = FileSystems.getDefault().getPath(elt);
>>>>  121                     if (!f.isAbsolute() && parent != null)
>>>>  122                         f = parent.resolve(f).toAbsolutePath();
>>>> 123 } catch (InvalidPathException | IOError e) {
>>>> 124 throw new IOException(e);
>>>> 125 }
>>>>  126                 list.add(f);
>>>>  127             }
>>>>  128 
>>>> -- Jon
>>>>
>>>> On 10/12/19 4:33 AM, Jaikiran Pai wrote:
>>>>> Thank you Jon. I've created
>>>>> https://bugs.openjdk.java.net/browse/JDK-8232170 and submitted a RFR
>>>>> thread with the patch.
>>>>>
>>>>> -Jaikiran
>>>>>
>>>>> On 11/10/19 8:20 PM, Jonathan Gibbons wrote:
>>>>>> Jaikiran,
>>>>>>
>>>>>> Not catching InvalidPathException is a bug and an unforeseen
>>>>>> consequence of converting the file manager from using java.io.File
>>>>>> to java.nio.file.Path.
>>>>>>
>>>>>> -- Jon
>>>>>>
>>>>>> On 10/11/19 7:26 AM, Jaikiran Pai wrote:
>>>>>>> In recent versions of JDK (I think after JDK 8), the
>>>>>>> com.sun.tools.javac.file.FSInfo#getJarClassPath(...) is throwing a
>>>>>>> java.nio.file.InvalidPathException in certain cases when the classpath
>>>>>>> entry it is parsing isn't a valid one. This is because of its usage of
>>>>>>> the java.nio.file.Path APIs, specifically
>>>>>>> https://github.com/openjdk/jdk/blob/4ad3d82c76936a8927ed8a505689a3683144ad98/src/jdk.compiler/share/classes/com/sun/tools/javac/file/FSInfo.java#L112.
>>>>>>>
>>>>>>>
>>>>>>> The throws clause of this FSInfo#getJarClassPath API lists IOException,
>>>>>>> so this method is now throwing an exception which isn't listed in its
>>>>>>> throws clause. As a result, callers, like the
>>>>>>> com.sun.tools.javac.file.Locations.SearchPath#addJarClassPath(...)
>>>>>>> https://github.com/openjdk/jdk/blob/4ad3d82c76936a8927ed8a505689a3683144ad98/src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java#L425
>>>>>>>
>>>>>>> are no longer able to catch this exception and log it and move on.
>>>>>>> Instead it gets propagated all the way back to the top level callers,
>>>>>>> thus breaking applications which are trying to compile java files
>>>>>>> programmatically.
>>>>>>>
>>>>>>> Would this be considered a bug? If so, I can create a JBS issue and
>>>>>>> provide a patch (and will try a jtreg test case too) for review.
>>>>>>>
>>>>>>> Although these classes are internal and not public API, the calling code
>>>>>>> is actually using public APIs (which internally end up calling these
>>>>>>> APIs). Like here
>>>>>>> https://github.com/quarkusio/quarkus/blob/master/core/devmode/src/main/java/io/quarkus/dev/JavaCompilationProvider.java#L48.
>>>>>>>
>>>>>>>
>>>>>>> For a lengthy discussion/context - please read the comments in this
>>>>>>> issue https://github.com/quarkusio/quarkus/issues/3592
>>>>>>>
>>>>>>> -Jaikiran
>>>>>>>
>>>>>>>
>>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20191017/67d21c2e/attachment.html>