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