Reorganize and another test for directory-permissions handling

cli/fs.rs

@@ -95,7 +95,6 @@ pub fn make_temp(
 }
 
 pub fn mkdir(path: &Path, mode: u32, recursive: bool) -> std::io::Result<()> {
-  debug!("mkdir -p {}", path.display());
   let mut builder = DirBuilder::new();
   builder.recursive(recursive);
   set_dir_permission(&mut builder, mode);
@@ -104,8 +103,9 @@ pub fn mkdir(path: &Path, mode: u32, recursive: bool) -> std::io::Result<()> {
 
 #[cfg(unix)]
 fn set_dir_permission(builder: &mut DirBuilder, mode: u32) {
-  debug!("set dir mode to {}", mode);
-  builder.mode(mode & 0o777);
+  let mode = mode & 0o777;
+  debug!("set dir mode to {:o}", mode);
+  builder.mode(mode);
 }
 
 #[cfg(not(unix))]

cli/js/lib.deno.ns.d.ts

@@ -740,7 +740,7 @@ declare namespace Deno {
     recursive?: boolean;
     /** Permissions to use when creating the directory (defaults to `0o777`,
      * before the process's umask).
-     * Does nothing/raises on Windows. */
+     * Ignored on Windows. */
     mode?: number;
   }
 

cli/js/ops/fs/mkdir.ts

@@ -1,14 +1,16 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import { sendSync, sendAsync } from "../dispatch_json.ts";
 
+type MkdirArgs = { path: string; recursive: boolean; mode?: number };
+
 // TODO(ry) The complexity in argument parsing is to support deprecated forms of
 // mkdir and mkdirSync.
 function mkdirArgs(
   path: string,
   optionsOrRecursive?: MkdirOptions | boolean,
   mode?: number
-): { path: string; recursive: boolean; mode: number } {
-  const args = { path, recursive: false, mode: 0o777 };
+): MkdirArgs {
+  const args: MkdirArgs = { path, recursive: false };
   if (typeof optionsOrRecursive == "boolean") {
     args.recursive = optionsOrRecursive;
     if (mode) {
@@ -34,7 +36,7 @@ export interface MkdirOptions {
   recursive?: boolean;
   /** Permissions to use when creating the directory (defaults to `0o777`,
    * before the process's umask).
-   * Does nothing/raises on Windows. */
+   * Ignored on Windows. */
   mode?: number;
 }
 

cli/js/tests/mkdir_test.ts

@@ -15,11 +15,10 @@ unitTest(
   { perms: { read: true, write: true } },
   function mkdirSyncMode(): void {
     const path = Deno.makeTempDirSync() + "/dir";
-    Deno.mkdirSync(path, { mode: 0o755 }); // no perm for x
+    Deno.mkdirSync(path, { mode: 0o737 });
     const pathInfo = Deno.statSync(path);
-    if (pathInfo.mode !== null) {
-      // Skip windows
-      assertEquals(pathInfo.mode & 0o777, 0o755);
+    if (Deno.build.os !== "win") {
+      assertEquals(pathInfo.mode! & 0o777, 0o737 & ~Deno.umask());
     }
   }
 );
@@ -45,6 +44,18 @@ unitTest(
   }
 );
 
+unitTest(
+  { perms: { read: true, write: true } },
+  async function mkdirMode(): Promise<void> {
+    const path = Deno.makeTempDirSync() + "/dir";
+    await Deno.mkdir(path, { mode: 0o737 });
+    const pathInfo = Deno.statSync(path);
+    if (Deno.build.os !== "win") {
+      assertEquals(pathInfo.mode! & 0o777, 0o737 & ~Deno.umask());
+    }
+  }
+);
+
 unitTest({ perms: { write: true } }, function mkdirErrIfExists(): void {
   let err;
   try {

cli/ops/fs.rs

@@ -280,7 +280,7 @@ struct MkdirArgs {
   promise_id: Option<u64>,
   path: String,
   recursive: bool,
-  mode: u32,
+  mode: Option<u32>,
 }
 
 fn op_mkdir(
@@ -290,13 +290,14 @@ fn op_mkdir(
 ) -> Result<JsonOp, OpError> {
   let args: MkdirArgs = serde_json::from_value(args)?;
   let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let mode = args.mode.unwrap_or(0o777);
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_mkdir {}", path.display());
-    deno_fs::mkdir(&path, args.mode, args.recursive)?;
+    debug!("op_mkdir {} {:o} {}", path.display(), mode, args.recursive);
+    deno_fs::mkdir(&path, mode, args.recursive)?;
     Ok(json!({}))
   })
 }