On Tue, 13 Apr 2021 19:48:24 GMT, Alexey Semenyuk <asemen...@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). > > 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. That seems like overkill. walkFileTree must call visitFile, preVisitDirectory, and postVisitDirectory synchronously, because their return value tells walkFileTree where to go next. > 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. That makes sense, should this be here in JPackageCommand.CreateExecutor() or in Executor.setTempDir() (maybe renamed setWinTempDir ?) ------------- PR: https://git.openjdk.java.net/jdk/pull/3473