On Tue, 13 Apr 2021 18:57:20 GMT, Andy Herrick <herr...@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
