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
