[10] Review request: 8179033: javapackager fails to create Mac Application Bundle

[victor.drozdov at oracle.com]

On 12/2/17 7:11 AM, Kevin Rushforth wrote:

>
>
> Michael Hall wrote:
>>
>>> On Dec 2, 2017, at 8:31 AM, Kevin Rushforth 
>>> <kevin.rushforth at oracle.com <mailto:kevin.rushforth at oracle.com>> wrote:
>>>
>>>
>>>
>>> Michael Hall wrote:
>>>>
>>>>> On Dec 1, 2017, at 7:43 PM, victor.drozdov at oracle.com 
>>>>> <mailto:victor.drozdov at oracle.com> wrote:
>>>>>
>>>>> Kevin,
>>>>>
>>>>> Please review the changes about copying classpath entries on Mac 
>>>>> and Windows.
>>>>>
>>>>> JIRA: https://bugs.openjdk.java.net/browse/JDK-8179033
>>>>> Webrev: 
>>>>> http://cr.openjdk.java.net/~vdrozdov/JDK-8179033/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Evdrozdov/JDK-8179033/webrev.00/>
>>>>
>>>> Sorry, to comment on this when it is not my place.
>>>> But I had looked at this sometime ago and it made no sense.
>>>> For the Mac you have…
>>>>
>>>> -                Files.copy(new File(srcdir, fname).toPath(), new 
>>>> File(javaDirectory.toFile(), fname).toPath());
>>>> +                writeEntry(new FileInputStream(new File(srcdir, 
>>>> fname)),
>>>> +                           new File(javaDirectory.toFile(), 
>>>> fname).toPath());
>>>>
>>>> What is the difference here? You are just changing the method of 
>>>> writing the file. Otherwise locations are identical.
>>>> Assuming the first way isn’t working, why would the second way do 
>>>> better?
>>>>
>>>
>>> writeEntry creates the directory before copying (and also opens the 
>>> file using an InputStream, but its the former that is the main part 
>>> of the fix).
>>>
>> I had just noticed that was different and wondered if it was the fix.
>> Which is fine.
>> Although maybe creating the directory and then do the Files.copy 
>> might be clearer rather than someone later trying to figure out what 
>> makes the writeEntry call necessary.
>>
>
> Perhaps Victor can comment about this. Since many of the other copy 
> operations also use writeEntry, it seems fine to me.
>
> -- Kevin
>
writeEntry() is supposed to be used for the operations where we need to 
create a directory and then do Files.copy(). I see no reason to avoid 
using it here. Linux bundler is also implemented this way.

/ private void writeEntry(InputStream in, Path dstFile) throws 
IOException {//
//        Files.createDirectories(dstFile.getParent());//
//        Files.copy(in, dstFile);//
//    }/

--Victor