RFR: JDK-8265078: jpackage tests on Windows leave large temp files

[Alexey Semenyuk]

On Tue, 13 Apr 2021 18:57:20 GMT, Andy Herrick <herrick at openjdk.org> wrote:

> two changes:
> One to jpackage, when recursively removing directory, when IOException occurs, record it and continue (deleting as much as possible) before throwing the exception.
> The other to tests, when running jpackage via ProcessBuilder.execute(), set the "TMP" environment variable to the current value of System Property "java.io.tmpdir".  This causes the sub-process (jpackage) to output tmp files to the tmp file location used by the test. (So the test harness can clean up after test exits).

Changes requested by asemenyuk (Reviewer).

src/jdk.jpackage/share/classes/jdk/jpackage/internal/IOUtils.java line 59:

> 57: 
> 58:     public static void deleteRecursive(Path directory) throws IOException {
> 59:         final IOException [] exception = { (IOException) null };

Do we know `Files.walkFileTree()` synchronizes calls on callback object? If not, I'd use `AtomicReference` to store the first exception.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/Executor.java line 92:

> 90:     }
> 91: 
> 92:     public Executor setTmpDir(String tmp) {

As this would work only on Windows, I'd throw `IllegalStateException` if the method is called on other platform.

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/JPackageCommand.java line 646:

> 644:         } else {
> 645:             exec.setExecutable(JavaTool.JPACKAGE);
> 646:             exec.setTmpDir(System.getProperty("java.io.tmpdir"));

This would work only on Windows. I'd put corresponding `if` around this statement to avoid future confusion.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3473