mkdir should not be recursive by default (#1530)

It should return an error if a file with the given path exists and
recursive isn't specified.

Because mode is not used on windows and rarely used in unix, it is made
to the last parameter.

In collaboration with Stefan Dombrowski <sdo451@gmail.com>

js/mkdir.ts

@@ -3,33 +3,48 @@ import * as msg from "gen/msg_generated";
 import * as flatbuffers from "./flatbuffers";
 import * as dispatch from "./dispatch";
 
-/** Creates a new directory with the specified path and permission
- * synchronously.
+/** Creates a new directory with the specified path synchronously.
+ * If `recursive` is set to true, nested directories will be created (also known
+ * as "mkdir -p").
+ * `mode` sets permission bits (before umask) on UNIX and does nothing on
+ * Windows.
  *
  *       import { mkdirSync } from "deno";
  *       mkdirSync("new_dir");
+ *       mkdirSync("nested/directories", true);
  */
-export function mkdirSync(path: string, mode = 0o777): void {
-  dispatch.sendSync(...req(path, mode));
+export function mkdirSync(path: string, recursive = false, mode = 0o777): void {
+  dispatch.sendSync(...req(path, recursive, mode));
 }
 
-/** Creates a new directory with the specified path and permission.
+/** Creates a new directory with the specified path.
+ * If `recursive` is set to true, nested directories will be created (also known
+ * as "mkdir -p").
+ * `mode` sets permission bits (before umask) on UNIX and does nothing on
+ * Windows.
  *
  *       import { mkdir } from "deno";
  *       await mkdir("new_dir");
+ *       await mkdir("nested/directories", true);
  */
-export async function mkdir(path: string, mode = 0o777): Promise<void> {
-  await dispatch.sendAsync(...req(path, mode));
+export async function mkdir(
+  path: string,
+  recursive = false,
+  mode = 0o777
+): Promise<void> {
+  await dispatch.sendAsync(...req(path, recursive, mode));
 }
 
 function req(
   path: string,
+  recursive: boolean,
   mode: number
 ): [flatbuffers.Builder, msg.Any, flatbuffers.Offset] {
   const builder = flatbuffers.createBuilder();
   const path_ = builder.createString(path);
   msg.Mkdir.startMkdir(builder);
   msg.Mkdir.addPath(builder, path_);
+  msg.Mkdir.addRecursive(builder, recursive);
   msg.Mkdir.addMode(builder, mode);
   const inner = msg.Mkdir.endMkdir(builder);
   return [builder, msg.Any.Mkdir, inner];

js/mkdir_test.ts

@@ -3,15 +3,15 @@ import { testPerm, assert, assertEqual } from "./test_util.ts";
 import * as deno from "deno";
 
 testPerm({ write: true }, function mkdirSyncSuccess() {
-  const path = deno.makeTempDirSync() + "/dir/subdir";
+  const path = deno.makeTempDirSync() + "/dir";
   deno.mkdirSync(path);
   const pathInfo = deno.statSync(path);
   assert(pathInfo.isDirectory());
 });
 
 testPerm({ write: true }, function mkdirSyncMode() {
-  const path = deno.makeTempDirSync() + "/dir/subdir";
-  deno.mkdirSync(path, 0o755); // no perm for x
+  const path = deno.makeTempDirSync() + "/dir";
+  deno.mkdirSync(path, false, 0o755); // no perm for x
   const pathInfo = deno.statSync(path);
   if (pathInfo.mode !== null) {
     // Skip windows
@@ -31,8 +31,33 @@ testPerm({ write: false }, function mkdirSyncPerm() {
 });
 
 testPerm({ write: true }, async function mkdirSuccess() {
-  const path = deno.makeTempDirSync() + "/dir/subdir";
+  const path = deno.makeTempDirSync() + "/dir";
   await deno.mkdir(path);
   const pathInfo = deno.statSync(path);
   assert(pathInfo.isDirectory());
 });
+
+testPerm({ write: true }, function mkdirErrIfExists() {
+  let err;
+  try {
+    deno.mkdirSync(".");
+  } catch (e) {
+    err = e;
+  }
+  assertEqual(err.kind, deno.ErrorKind.AlreadyExists);
+  assertEqual(err.name, "AlreadyExists");
+});
+
+testPerm({ write: true }, function mkdirSyncRecursive() {
+  const path = deno.makeTempDirSync() + "/nested/directory";
+  deno.mkdirSync(path, true);
+  const pathInfo = deno.statSync(path);
+  assert(pathInfo.isDirectory());
+});
+
+testPerm({ write: true }, async function mkdirRecursive() {
+  const path = deno.makeTempDirSync() + "/nested/directory";
+  await deno.mkdir(path, true);
+  const pathInfo = deno.statSync(path);
+  assert(pathInfo.isDirectory());
+});

js/read_link_test.ts

@@ -3,7 +3,7 @@ import { test, testPerm, assert, assertEqual } from "./test_util.ts";
 import * as deno from "deno";
 
 testPerm({ write: true }, function readlinkSyncSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-readlink-sync";
+  const testDir = deno.makeTempDirSync();
   const target = testDir + "/target";
   const symlink = testDir + "/symln";
   deno.mkdirSync(target);
@@ -30,7 +30,7 @@ test(function readlinkSyncNotFound() {
 });
 
 testPerm({ write: true }, async function readlinkSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-readlink";
+  const testDir = deno.makeTempDirSync();
   const target = testDir + "/target";
   const symlink = testDir + "/symln";
   deno.mkdirSync(target);

js/rename_test.ts

@@ -3,7 +3,7 @@ import { testPerm, assert, assertEqual } from "./test_util.ts";
 import * as deno from "deno";
 
 testPerm({ write: true }, function renameSyncSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-rename-sync";
+  const testDir = deno.makeTempDirSync();
   const oldpath = testDir + "/oldpath";
   const newpath = testDir + "/newpath";
   deno.mkdirSync(oldpath);
@@ -38,7 +38,7 @@ testPerm({ write: false }, function renameSyncPerm() {
 });
 
 testPerm({ write: true }, async function renameSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-rename";
+  const testDir = deno.makeTempDirSync();
   const oldpath = testDir + "/oldpath";
   const newpath = testDir + "/newpath";
   deno.mkdirSync(oldpath);

js/symlink_test.ts

@@ -3,7 +3,7 @@ import { testPerm, assert, assertEqual } from "./test_util.ts";
 import * as deno from "deno";
 
 testPerm({ write: true }, function symlinkSyncSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-symlink-sync";
+  const testDir = deno.makeTempDirSync();
   const oldname = testDir + "/oldname";
   const newname = testDir + "/newname";
   deno.mkdirSync(oldname);
@@ -48,7 +48,7 @@ testPerm({ write: true }, function symlinkSyncNotImplemented() {
 });
 
 testPerm({ write: true }, async function symlinkSuccess() {
-  const testDir = deno.makeTempDirSync() + "/test-symlink";
+  const testDir = deno.makeTempDirSync();
   const oldname = testDir + "/oldname";
   const newname = testDir + "/newname";
   deno.mkdirSync(oldname);

src/deno_dir.rs

@@ -73,10 +73,12 @@ impl DenoDir {
       deps_https,
       reload,
     };
-    deno_fs::mkdir(deno_dir.gen.as_ref(), 0o755)?;
-    deno_fs::mkdir(deno_dir.deps.as_ref(), 0o755)?;
-    deno_fs::mkdir(deno_dir.deps_http.as_ref(), 0o755)?;
-    deno_fs::mkdir(deno_dir.deps_https.as_ref(), 0o755)?;
+
+    // TODO Lazily create these directories.
+    deno_fs::mkdir(deno_dir.gen.as_ref(), 0o755, true)?;
+    deno_fs::mkdir(deno_dir.deps.as_ref(), 0o755, true)?;
+    deno_fs::mkdir(deno_dir.deps_http.as_ref(), 0o755, true)?;
+    deno_fs::mkdir(deno_dir.deps_https.as_ref(), 0o755, true)?;
 
     debug!("root {}", deno_dir.root.display());
     debug!("gen {}", deno_dir.gen.display());

src/fs.rs

@@ -67,15 +67,12 @@ pub fn make_temp_dir(
   }
 }
 
-pub fn mkdir(path: &Path, perm: u32) -> std::io::Result<()> {
+pub fn mkdir(path: &Path, perm: u32, recursive: bool) -> std::io::Result<()> {
   debug!("mkdir -p {}", path.display());
   let mut builder = DirBuilder::new();
-  builder.recursive(true);
+  builder.recursive(recursive);
   set_dir_permission(&mut builder, perm);
-  builder.create(path).or_else(|err| match err.kind() {
-    std::io::ErrorKind::AlreadyExists => Ok(()),
-    _ => Err(err),
-  })
+  builder.create(path)
 }
 
 #[cfg(any(unix))]

src/msg.fbs

@@ -260,14 +260,13 @@ table MakeTempDirRes {
 
 table Mkdir {
   path: string;
-  mode: uint;
-  // mode specified by https://godoc.org/os#FileMode
+  recursive: bool;
+  mode: uint; // Specified by https://godoc.org/os#FileMode
 }
 
 table Chmod {
   path: string;
-  mode: uint;
-  // mode specified by https://godoc.org/os#FileMode
+  mode: uint; // Specified by https://godoc.org/os#FileMode
 }
 
 table Remove {

src/ops.rs

@@ -540,15 +540,17 @@ fn op_mkdir(
 ) -> Box<Op> {
   assert_eq!(data.len(), 0);
   let inner = base.inner_as_mkdir().unwrap();
-  let mode = inner.mode();
   let path = String::from(inner.path().unwrap());
+  let recursive = inner.recursive();
+  let mode = inner.mode();
 
   if let Err(e) = state.check_write(&path) {
     return odd_future(e);
   }
+
   blocking(base.sync(), move || {
     debug!("op_mkdir {}", path);
-    deno_fs::mkdir(Path::new(&path), mode)?;
+    deno_fs::mkdir(Path::new(&path), mode, recursive)?;
     Ok(empty_buf())
   })
 }