8249289: Exception thrown when --temp points to non-existant directory

Andy Herrick · Andy Herrick · commit 2f6afe55d879 · 2020-07-17T09:18:51.000-04:00
Reviewed-by: asemenyuk, almatvee
diff --git a/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/DeployParams.java b/src/jdk.incubator.jpackage/share/classes/jdk/incubator/jpackage/internal/DeployParams.java
@@ -79,18 +79,20 @@ List<Path> expandFileset(Path root) throws IOException {
         List<Path> files = new LinkedList<>();
         if (!Files.isSymbolicLink(root)) {
             if (Files.isDirectory(root)) {
-                List<Path> children = Files.list(root).collect(Collectors.toList());
-                if (children != null && children.size() > 0) {
-                    children.forEach(f -> {
-                        try {
-                            files.addAll(expandFileset(f));
-                        } catch (IOException ex) {
-                            throw new RuntimeException(ex);
-                        }
-                    });
-                } else {
-                    // Include empty folders
-                    files.add(root);
+                try (Stream<Path> stream = Files.list(root)) {
+                    List<Path> children = stream.collect(Collectors.toList());
+                    if (children != null && children.size() > 0) {
+                        children.forEach(f -> {
+                            try {
+                                files.addAll(expandFileset(f));
+                            } catch (IOException ex) {
+                                throw new RuntimeException(ex);
+                            }
+                        });
+                    } else {
+                        // Include empty folders
+                        files.add(root);
+                    }
                 }
             } else {
                 files.add(root);
@@ -214,13 +216,13 @@ public void validate() throws PackagerException {
         // Validate temp dir
         String root = (String)bundlerArguments.get(
                 Arguments.CLIOptions.TEMP_ROOT.getId());
-        if (root != null) {
-            try {
-                String [] contents = Files.list(Path.of(root))
-                        .toArray(String[]::new);
-
-                if (contents != null && contents.length > 0) {
-                    throw new PackagerException("ERR_BuildRootInvalid", root);
+        if (root != null && Files.exists(Path.of(root))) {
+            try (Stream<Path> stream = Files.walk(Path.of(root), 1)) {
+                Path [] contents = stream.toArray(Path[]::new);
+                // contents.length > 1 because Files.walk(path) includes path
+                if (contents != null && contents.length > 1) {
+                    throw new PackagerException(
+                            "ERR_BuildRootInvalid", root);
                 }
             } catch (IOException ioe) {
                 throw new PackagerException(ioe);
diff --git a/test/jdk/tools/jpackage/share/jdk/jpackage/tests/BasicTest.java b/test/jdk/tools/jpackage/share/jdk/jpackage/tests/BasicTest.java
@@ -245,11 +245,24 @@ public void testAddModules(String... addModulesArg) {
      * @throws IOException
      */
     @Test
-    public void testTemp() throws IOException {
-        final Path tempRoot = TKit.createTempDirectory("temp-root");
-
+    @Parameter("true")
+    @Parameter("false")
+    public void testTemp(boolean withExistingTempDir) throws IOException {
+        final Path tempRoot = TKit.createTempDirectory("tmp");
+        // This Test has problems on windows where path in the temp dir are too long
+        // for the wix tools.  We can't use a tempDir outside the TKit's WorkDir, so
+        // we minimize both the tempRoot directory name (above) and the tempDir name
+        // (below) to the extension part (which is necessary to differenciate between
+        // the multiple PackageTypes that will be run for one JPackageCommand).
+        // It might be beter if the whole work dir name was shortened from:
+        // jtreg_open_test_jdk_tools_jpackage_share_jdk_jpackage_tests_BasicTest_java.
         Function<JPackageCommand, Path> getTempDir = cmd -> {
-            return tempRoot.resolve(cmd.outputBundle().getFileName());
+            String ext = cmd.outputBundle().getFileName().toString();
+            int i = ext.lastIndexOf(".");
+            if (i > 0 && i < (ext.length() - 1)) {
+                ext = ext.substring(i+1);
+            }
+            return tempRoot.resolve(ext);
         };
 
         Supplier<PackageTest> createTest = () -> {
@@ -259,7 +272,11 @@ public void testTemp() throws IOException {
             .addInitializer(JPackageCommand::setDefaultInputOutput)
             .addInitializer(cmd -> {
                 Path tempDir = getTempDir.apply(cmd);
-                Files.createDirectories(tempDir);
+                if (withExistingTempDir) {
+                    Files.createDirectories(tempDir);
+                } else {
+                    Files.createDirectories(tempDir.getParent());
+                }
                 cmd.addArguments("--temp", tempDir);
             });
         };
