chmod should throw on Windows (#4446)

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

@@ -950,7 +950,7 @@ declare namespace Deno {
    *
    * For a full description, see [chmod](#chmod)
    *
-   * NOTE: This API currently has no effect on Windows
+   * NOTE: This API currently throws on Windows
    *
    * Requires `allow-write` permission. */
   export function chmodSync(path: string, mode: number): void;
@@ -978,7 +978,7 @@ declare namespace Deno {
    * | 1      | execute only |
    * | 0      | no permission |
    *
-   * NOTE: This API currently has no effect on Windows
+   * NOTE: This API currently throws on Windows
    *
    * Requires `allow-write` permission. */
   export function chmod(path: string, mode: number): Promise<void>;

cli/js/tests/chmod_test.ts

@@ -1,10 +1,8 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import { unitTest, assert, assertEquals } from "./test_util.ts";
 
-const isNotWindows = Deno.build.os !== "win";
-
 unitTest(
-  { perms: { read: true, write: true } },
+  { ignore: Deno.build.os === "win", perms: { read: true, write: true } },
   function chmodSyncSuccess(): void {
     const enc = new TextEncoder();
     const data = enc.encode("Hello");
@@ -12,16 +10,12 @@ unitTest(
     const filename = tempDir + "/test.txt";
     Deno.writeFileSync(filename, data, { mode: 0o666 });
 
-    // On windows no effect, but should not crash
     Deno.chmodSync(filename, 0o777);
 
-    // Check success when not on windows
-    if (isNotWindows) {
     const fileInfo = Deno.statSync(filename);
     assert(fileInfo.mode);
     assertEquals(fileInfo.mode & 0o777, 0o777);
   }
-  }
 );
 
 // Check symlink when not on windows
@@ -79,7 +73,7 @@ unitTest({ perms: { write: false } }, function chmodSyncPerm(): void {
 });
 
 unitTest(
-  { perms: { read: true, write: true } },
+  { ignore: Deno.build.os === "win", perms: { read: true, write: true } },
   async function chmodSuccess(): Promise<void> {
     const enc = new TextEncoder();
     const data = enc.encode("Hello");
@@ -87,16 +81,12 @@ unitTest(
     const filename = tempDir + "/test.txt";
     Deno.writeFileSync(filename, data, { mode: 0o666 });
 
-    // On windows no effect, but should not crash
     await Deno.chmod(filename, 0o777);
 
-    // Check success when not on windows
-    if (isNotWindows) {
     const fileInfo = Deno.statSync(filename);
     assert(fileInfo.mode);
     assertEquals(fileInfo.mode & 0o777, 0o777);
   }
-  }
 );
 
 // Check symlink when not on windows

cli/js/write_file.ts

@@ -3,6 +3,7 @@ import { stat, statSync } from "./ops/fs/stat.ts";
 import { open, openSync } from "./files.ts";
 import { chmod, chmodSync } from "./ops/fs/chmod.ts";
 import { writeAll, writeAllSync } from "./buffer.ts";
+import { build } from "./build.ts";
 
 export interface WriteFileOptions {
   append?: boolean;
@@ -26,7 +27,11 @@ export function writeFileSync(
   const openMode = !!options.append ? "a" : "w";
   const file = openSync(path, openMode);
 
-  if (options.mode !== undefined && options.mode !== null) {
+  if (
+    options.mode !== undefined &&
+    options.mode !== null &&
+    build.os !== "win"
+  ) {
     chmodSync(path, options.mode);
   }
 
@@ -50,7 +55,11 @@ export async function writeFile(
   const openMode = !!options.append ? "a" : "w";
   const file = await open(path, openMode);
 
-  if (options.mode !== undefined && options.mode !== null) {
+  if (
+    options.mode !== undefined &&
+    options.mode !== null &&
+    build.os !== "win"
+  ) {
     await chmod(path, options.mode);
   }
 

cli/ops/fs.rs

@@ -353,14 +353,15 @@ fn op_chmod(
       use std::os::unix::fs::PermissionsExt;
       let permissions = PermissionsExt::from_mode(mode);
       std::fs::set_permissions(&path, permissions)?;
+      Ok(json!({}))
     }
     // TODO Implement chmod for Windows (#4357)
     #[cfg(not(unix))]
     {
       // Still check file/dir exists on Windows
       let _metadata = std::fs::metadata(&path)?;
+      return Err(OpError::not_implemented());
     }
-    Ok(json!({}))
   })
 }