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

[Michael Hall]

> On Dec 2, 2017, at 9:11 AM, Kevin Rushforth <kevin.rushforth at oracle.com> 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 <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.

If it is normally used to ensure the directory is there this would probably be something people more familiar with the code would know.

Thanks.