RFR JDK-8170114 jimage extract to not an empty directory overwrites content of the directory

[Alan Bateman]

On 10/02/2018 00:04, mandy chung wrote:
> On 2/9/18 7:23 AM, Michal Vala wrote:
>>
>> Patch validates output directory before any jimage extracting happen. 
>> I've moved validation to extra private method as it is few lines of 
>> code. I've also added proper error message for case when output path 
>> is not a directory (JImageTask.java#449).
>>
>
> Thanks for looking at JDK-8170114 and JDK-8170120.  I took a look at  
> http://cr.openjdk.java.net/~shade/8170114/webrev.01/
Yes, thanks for looking at these issues.

For `jimage info <not-a-jimage-file>` issue then jimage correctly fails 
(with a non-zero status) but the IOException isn't converted into an 
error message as it does with other errors. Yes, it would be good to fix 
that.

I think the `jimage extract --dir <existing-file> <jimage-file>` 
scenario needs discussion. If <existing-file> is a non-directory file 
then jimage has to fail, I don't expect disagreement on that. For the 
case where it is an existing directory then the options seem to be:

1. Extract into the existing directory (existing JDK 9 and JDK 10 behavior)
2. Fail if it's not empty (your patch)
3. Fail if it exists (Mandy's mail, the motivation being to keep it 
consistent with jlink)

I view `jimage extract --dir <dir>` as being similar to `unzip -d <dir>` 
so I don't think existing behavior (#1) is incorrect, the only issue is 
that it silently overrides files whereas unzip will prompt before 
overriding (unless you specify -o). The `jar` tool, and legacy `tar` 
tool side with `jimage` and are happy to silently replace existing files.

What would you think about focusing on the override case instead of 
disallowing extracting into an existing non-empty directory? I realize 
this is more work as it means deciding on whether to prompt, warn or 
fail. It also means thinking about the equivalent of unzip -o to 
allowing existing files be replaced.

-Alan