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

[Kevin Rushforth]

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