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

[Michal Vala]

On Mon, 2018-02-12 at 09:39 -0800, mark.reinhold at oracle.com wrote:
> 2018/2/11 23:10:46 -0800, Michal Vala <mvala at redhat.com>:
> > On 02/10/2018 11:25 AM, Alan Bateman wrote:
> > > ...
> > > 
> > > 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.
> > 
> > Unzip prompts user for individual files and I'm not sure whether
> > it's a good 
> > option here. Maybe prompt user at the beginning that directory is
> > not empty and 
> > there is a risk that files there might be overwritten continue y/n?
> 
> CLI tools that prompt the user are difficult to use in scripts,
> so I advise against that.

Usability in scripts is a valid point, but I still think that silently
overwriting files is a bad practice. I can imagine better behavior
without prompting the user. Like error when non-empty dir and 'force-
overwrite' parameter. Or at least print a warning message that some
files were overwritten.

> 
> jimage is a diagnostic tool meant for rare and specialized use.
> I think its current behavior (extract into an existing directory)
> is fine, and I can imagine use cases where that might actually be
> desired.
> 
> - Mark
-- 
Michal Vala
OpenJDK QE
Red Hat Czech