On Fri, 13 Dec 2024 13:41:52 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> src/jdk.jartool/share/classes/sun/tools/jar/Main.java line 435:
>>
>>> 433: file = createTemporaryFile("tmpJar", ".jar");
>>> 434: try (InputStream in = new
>>> FileInputStream(FileDescriptor.in)) {
>>> 435: Files.copy(in, file.toPath());
>>
>> Did you try adding the REPLACE_EXISTING option to Files.copy, I assume that
>> will fix it.
>
> Hello Alan, I had thought about that, but then I looked at the implementation
> of `Files.copy(...)` with `REPLACE_EXISTING`. If that option is specified,
> the `Files.copy(...)` implementation first deletes the existing file:
>
> // attempt to delete an existing file
> if (replaceExisting) {
> deleteIfExists(target);
> }
>
> before it initiates the copying. It didn't feel right to be explicitly
> creating a file (before the call to Files.copy) and then having it deleted
> due to the use of `REPLACE_EXISTING`. So I went ahead with this alternate
> approach.
>
> Would you still prefer that we use `REPLACE_EXISTING` here?
Thank you Lance for the review. Alan, is it OK to proceed with this current
change or do you think we should pursue the `REPLACE_EXISTING` option here?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22734#discussion_r1887865656
