refactor: move code from fs.rs into ops/fs.rs (#4428)

This a complex boring PR that shifts around code (primarily) in cli/fs.rs and
cli/ops/fs.rs. The gain of this refactoring is to ease the way for #4188 and
#4017, and also to avoid some future development pain.

Mostly there is no change in functionality. Except:
* squashed bugs where op_utime and op_chown weren't using `resolve_from_cwd`
* eliminated the use of the external `remove_dir_all` crate.
* op_chmod now only queries metadata to verify file/dir exists on Windows (it
  will already fail on Unix if it doesn't)
* op_chown now verifies the file/dir's existence on Windows like chmod does.

Cargo.lock

@@ -442,7 +442,6 @@ dependencies = [
  "pty",
  "rand 0.7.3",
  "regex",
- "remove_dir_all",
  "reqwest",
  "ring",
  "rustyline",

cli/Cargo.toml

@@ -44,7 +44,6 @@ log = "0.4.8"
 notify = "5.0.0-pre.2"
 rand = "0.7.3"
 regex = "1.3.5"
-remove_dir_all = "0.5.2"
 reqwest = { version = "0.10.4", default-features = false, features = ["rustls-tls", "stream", "gzip", "brotli"] }
 ring = "0.16.11"
 rustyline = "6.0.0"

cli/fs.rs

@@ -1,21 +1,12 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-use std;
+use std::env::current_dir;
-use std::fs::{DirBuilder, File, OpenOptions};
+use std::fs::OpenOptions;
-use std::io::ErrorKind;
 use std::io::Write;
 use std::path::{Component, Path, PathBuf};
 
 use deno_core::ErrBox;
-use rand;
-use rand::Rng;
 use walkdir::WalkDir;
 
-#[cfg(unix)]
-use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt};
-
-#[cfg(unix)]
-use nix::unistd::{chown as unix_chown, Gid, Uid};
-
 pub fn write_file<T: AsRef<[u8]>>(
   filename: &Path,
   data: T,
@@ -41,99 +32,21 @@ pub fn write_file_2<T: AsRef<[u8]>>(
     .open(filename)?;
 
   if update_mode {
-    set_permissions(&mut file, mode)?;
+    #[cfg(unix)]
+    {
+      use std::os::unix::fs::PermissionsExt;
+      let mode = mode & 0o777;
+      debug!("set file mode to {:o}", mode);
+      let permissions = PermissionsExt::from_mode(mode);
+      file.set_permissions(permissions)?;
+    }
+    #[cfg(not(unix))]
+    let _ = mode;
   }
 
   file.write_all(data.as_ref())
 }
 
-#[cfg(unix)]
-fn set_permissions(file: &mut File, mode: u32) -> std::io::Result<()> {
-  debug!("set file mode to {}", mode);
-  file.set_permissions(PermissionsExt::from_mode(mode & 0o777))
-}
-
-#[cfg(not(unix))]
-fn set_permissions(_file: &mut File, _mode: u32) -> std::io::Result<()> {
-  // NOOP on windows
-  Ok(())
-}
-
-pub fn make_temp(
-  dir: Option<&Path>,
-  prefix: Option<&str>,
-  suffix: Option<&str>,
-  is_dir: bool,
-) -> std::io::Result<PathBuf> {
-  let prefix_ = prefix.unwrap_or("");
-  let suffix_ = suffix.unwrap_or("");
-  let mut buf: PathBuf = match dir {
-    Some(ref p) => p.to_path_buf(),
-    None => std::env::temp_dir(),
-  }
-  .join("_");
-  let mut rng = rand::thread_rng();
-  loop {
-    let unique = rng.gen::<u32>();
-    buf.set_file_name(format!("{}{:08x}{}", prefix_, unique, suffix_));
-    let r = if is_dir {
-      let mut builder = DirBuilder::new();
-      set_dir_permission(&mut builder, 0o700);
-      builder.create(buf.as_path())
-    } else {
-      let mut open_options = OpenOptions::new();
-      open_options.write(true).create_new(true);
-      #[cfg(unix)]
-      open_options.mode(0o600);
-      open_options.open(buf.as_path())?;
-      Ok(())
-    };
-    match r {
-      Err(ref e) if e.kind() == ErrorKind::AlreadyExists => continue,
-      Ok(_) => return Ok(buf),
-      Err(e) => return Err(e),
-    }
-  }
-}
-
-pub fn mkdir(path: &Path, mode: u32, recursive: bool) -> std::io::Result<()> {
-  let mut builder = DirBuilder::new();
-  builder.recursive(recursive);
-  set_dir_permission(&mut builder, mode);
-  builder.create(path)
-}
-
-#[cfg(unix)]
-fn set_dir_permission(builder: &mut DirBuilder, mode: u32) {
-  let mode = mode & 0o777;
-  debug!("set dir mode to {:o}", mode);
-  builder.mode(mode);
-}
-
-#[cfg(not(unix))]
-fn set_dir_permission(_builder: &mut DirBuilder, _mode: u32) {
-  // NOOP on windows
-}
-
-#[cfg(unix)]
-pub fn chown(path: &str, uid: u32, gid: u32) -> Result<(), ErrBox> {
-  let nix_uid = Uid::from_raw(uid);
-  let nix_gid = Gid::from_raw(gid);
-  unix_chown(path, Option::Some(nix_uid), Option::Some(nix_gid))
-    .map_err(ErrBox::from)
-}
-
-#[cfg(not(unix))]
-pub fn chown(_path: &str, _uid: u32, _gid: u32) -> Result<(), ErrBox> {
-  // FAIL on Windows
-  // TODO: implement chown for Windows
-  let e = std::io::Error::new(
-    std::io::ErrorKind::Other,
-    "Not implemented".to_string(),
-  );
-  Err(ErrBox::from(e))
-}
-
 /// Normalize all itermediate components of the path (ie. remove "./" and "../" components).
 /// Similar to `fs::canonicalize()` but doesn't resolve symlinks.
 ///
@@ -171,7 +84,7 @@ pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, ErrBox> {
   let resolved_path = if path.is_absolute() {
     path.to_owned()
   } else {
-    let cwd = std::env::current_dir().unwrap();
+    let cwd = current_dir().unwrap();
     cwd.join(path)
   };
 
@@ -184,19 +97,19 @@ mod tests {
 
   #[test]
   fn resolve_from_cwd_child() {
-    let cwd = std::env::current_dir().unwrap();
+    let cwd = current_dir().unwrap();
     assert_eq!(resolve_from_cwd(Path::new("a")).unwrap(), cwd.join("a"));
   }
 
   #[test]
   fn resolve_from_cwd_dot() {
-    let cwd = std::env::current_dir().unwrap();
+    let cwd = current_dir().unwrap();
     assert_eq!(resolve_from_cwd(Path::new(".")).unwrap(), cwd);
   }
 
   #[test]
   fn resolve_from_cwd_parent() {
-    let cwd = std::env::current_dir().unwrap();
+    let cwd = current_dir().unwrap();
     assert_eq!(resolve_from_cwd(Path::new("a/..")).unwrap(), cwd);
   }
 

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

@@ -1291,25 +1291,25 @@ declare namespace Deno {
 
   // @url js/link.d.ts
 
-  /** Creates `newname` as a hard link to `oldname`.
+  /** Creates `newpath` as a hard link to `oldpath`.
    *
    *       Deno.linkSync("old/name", "new/name");
    *
    * Requires `allow-read` and `allow-write` permissions. */
-  export function linkSync(oldname: string, newname: string): void;
+  export function linkSync(oldpath: string, newpath: string): void;
 
-  /** Creates `newname` as a hard link to `oldname`.
+  /** Creates `newpath` as a hard link to `oldpath`.
    *
    *       await Deno.link("old/name", "new/name");
    *
    * Requires `allow-read` and `allow-write` permissions. */
-  export function link(oldname: string, newname: string): Promise<void>;
+  export function link(oldpath: string, newpath: string): Promise<void>;
 
   // @url js/symlink.d.ts
 
   /** **UNSTABLE**: `type` argument type may be changed to `"dir" | "file"`.
    *
-   * Creates `newname` as a symbolic link to `oldname`. The type argument can be
+   * Creates `newpath` as a symbolic link to `oldpath`. The type argument can be
    * set to `dir` or `file`. Is only available on Windows and ignored on other
    * platforms.
    *
@@ -1317,14 +1317,14 @@ declare namespace Deno {
    *
    * Requires `allow-read` and `allow-write` permissions. */
   export function symlinkSync(
-    oldname: string,
+    oldpath: string,
-    newname: string,
+    newpath: string,
     type?: string
   ): void;
 
   /** **UNSTABLE**: `type` argument may be changed to "dir" | "file"
    *
-   * Creates `newname` as a symbolic link to `oldname`. The type argument can be
+   * Creates `newpath` as a symbolic link to `oldpath`. The type argument can be
    * set to `dir` or `file`. Is only available on Windows and ignored on other
    * platforms.
    *
@@ -1332,8 +1332,8 @@ declare namespace Deno {
    *
    * Requires `allow-read` and `allow-write` permissions. */
   export function symlink(
-    oldname: string,
+    oldpath: string,
-    newname: string,
+    newpath: string,
     type?: string
   ): Promise<void>;
 

cli/js/ops/fs/link.ts

@@ -1,10 +1,10 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import { sendSync, sendAsync } from "../dispatch_json.ts";
 
-export function linkSync(oldname: string, newname: string): void {
+export function linkSync(oldpath: string, newpath: string): void {
-  sendSync("op_link", { oldname, newname });
+  sendSync("op_link", { oldpath, newpath });
 }
 
-export async function link(oldname: string, newname: string): Promise<void> {
+export async function link(oldpath: string, newpath: string): Promise<void> {
-  await sendAsync("op_link", { oldname, newname });
+  await sendAsync("op_link", { oldpath, newpath });
 }

cli/js/ops/fs/symlink.ts

@@ -4,23 +4,23 @@ import * as util from "../../util.ts";
 import { build } from "../../build.ts";
 
 export function symlinkSync(
-  oldname: string,
+  oldpath: string,
-  newname: string,
+  newpath: string,
   type?: string
 ): void {
   if (build.os === "win" && type) {
     return util.notImplemented();
   }
-  sendSync("op_symlink", { oldname, newname });
+  sendSync("op_symlink", { oldpath, newpath });
 }
 
 export async function symlink(
-  oldname: string,
+  oldpath: string,
-  newname: string,
+  newpath: string,
   type?: string
 ): Promise<void> {
   if (build.os === "win" && type) {
     return util.notImplemented();
   }
-  await sendAsync("op_symlink", { oldname, newname });
+  await sendAsync("op_symlink", { oldpath, newpath });
 }

cli/ops/fs.rs

@@ -2,23 +2,20 @@
 // Some deserializer fields are only used on Unix and Windows build fails without it
 use super::dispatch_json::{blocking_json, Deserialize, JsonOp, Value};
 use super::io::{FileMetadata, StreamResource, StreamResourceHolder};
-use crate::fs as deno_fs;
+use crate::fs::resolve_from_cwd;
 use crate::op_error::OpError;
 use crate::ops::dispatch_json::JsonResult;
 use crate::state::State;
 use deno_core::*;
 use futures::future::FutureExt;
-use remove_dir_all::remove_dir_all;
-use std;
 use std::convert::From;
-use std::fs;
+use std::env::{current_dir, set_current_dir, temp_dir};
 use std::io::SeekFrom;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::time::UNIX_EPOCH;
 use tokio;
 
-#[cfg(unix)]
+use rand::{thread_rng, Rng};
-use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt};
 
 pub fn init(i: &mut Isolate, s: &State) {
   i.register_op("op_open", s.stateful_json_op(op_open));
@@ -72,16 +69,19 @@ fn op_open(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: OpenArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let state_ = state.clone();
 
   let mut open_options = if let Some(mode) = args.mode {
     #[allow(unused_mut)]
-    let mut std_options = fs::OpenOptions::new();
+    let mut std_options = std::fs::OpenOptions::new();
     // mode only used if creating the file on Unix
     // if not specified, defaults to 0o666
     #[cfg(unix)]
+    {
+      use std::os::unix::fs::OpenOptionsExt;
       std_options.mode(mode & 0o777);
+    }
     #[cfg(not(unix))]
     let _ = mode; // avoid unused warning
     tokio::fs::OpenOptions::from(std_options)
@@ -287,7 +287,7 @@ fn op_chdir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChdirArgs = serde_json::from_value(args)?;
-  std::env::set_current_dir(&args.directory)?;
+  set_current_dir(&args.directory)?;
   Ok(JsonOp::Sync(json!({})))
 }
 
@@ -306,15 +306,22 @@ fn op_mkdir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: MkdirArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
-  let mode = args.mode.unwrap_or(0o777);
+  let mode = args.mode.unwrap_or(0o777) & 0o777;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_mkdir {} {:o} {}", path.display(), mode, args.recursive);
-    deno_fs::mkdir(&path, mode, args.recursive)?;
+    let mut builder = std::fs::DirBuilder::new();
+    builder.recursive(args.recursive);
+    #[cfg(unix)]
+    {
+      use std::os::unix::fs::DirBuilderExt;
+      builder.mode(mode);
+    }
+    builder.create(path)?;
     Ok(json!({}))
   })
 }
@@ -324,7 +331,6 @@ fn op_mkdir(
 struct ChmodArgs {
   promise_id: Option<u64>,
   path: String,
-  #[allow(unused)]
   mode: u32,
 }
 
@@ -334,20 +340,25 @@ fn op_chmod(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChmodArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
+  let mode = args.mode & 0o777;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_chmod {}", path.display());
+    debug!("op_chmod {} {:o}", path.display(), mode);
-    // Still check file/dir exists on windows
-    let _metadata = fs::metadata(&path)?;
     #[cfg(unix)]
     {
-      let mut permissions = _metadata.permissions();
+      use std::os::unix::fs::PermissionsExt;
-      permissions.set_mode(args.mode);
+      let permissions = PermissionsExt::from_mode(mode);
-      fs::set_permissions(&path, permissions)?;
+      std::fs::set_permissions(&path, permissions)?;
+    }
+    // TODO Implement chmod for Windows (#4357)
+    #[cfg(not(unix))]
+    {
+      // Still check file/dir exists on Windows
+      let _metadata = std::fs::metadata(&path)?;
     }
     Ok(json!({}))
   })
@@ -368,15 +379,28 @@ fn op_chown(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ChownArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_chown {}", path.display());
+    debug!("op_chown {} {} {}", path.display(), args.uid, args.gid);
-    deno_fs::chown(args.path.as_ref(), args.uid, args.gid)?;
+    #[cfg(unix)]
+    {
+      use nix::unistd::{chown, Gid, Uid};
+      let nix_uid = Uid::from_raw(args.uid);
+      let nix_gid = Gid::from_raw(args.gid);
+      chown(&path, Option::Some(nix_uid), Option::Some(nix_gid))?;
       Ok(json!({}))
+    }
+    // TODO Implement chown for Windows
+    #[cfg(not(unix))]
+    {
+      // Still check file/dir exists on Windows
+      let _metadata = std::fs::metadata(&path)?;
+      return Err(OpError::not_implemented());
+    }
   })
 }
 
@@ -394,22 +418,22 @@ fn op_remove(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RemoveArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let recursive = args.recursive;
 
   state.check_write(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_remove {}", path.display());
+    let metadata = std::fs::symlink_metadata(&path)?;
-    let metadata = fs::symlink_metadata(&path)?;
+    debug!("op_remove {} {}", path.display(), recursive);
     let file_type = metadata.file_type();
     if file_type.is_file() || file_type.is_symlink() {
-      fs::remove_file(&path)?;
+      std::fs::remove_file(&path)?;
     } else if recursive {
-      remove_dir_all(&path)?;
+      std::fs::remove_dir_all(&path)?;
     } else {
-      fs::remove_dir(&path)?;
+      std::fs::remove_dir(&path)?;
     }
     Ok(json!({}))
   })
@@ -429,8 +453,8 @@ fn op_copy_file(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: CopyFileArgs = serde_json::from_value(args)?;
-  let from = deno_fs::resolve_from_cwd(Path::new(&args.from))?;
+  let from = resolve_from_cwd(Path::new(&args.from))?;
-  let to = deno_fs::resolve_from_cwd(Path::new(&args.to))?;
+  let to = resolve_from_cwd(Path::new(&args.to))?;
 
   state.check_read(&from)?;
   state.check_write(&to)?;
@@ -446,7 +470,7 @@ fn op_copy_file(
     }
 
     // returns size of from as u64 (we ignore)
-    fs::copy(&from, &to)?;
+    std::fs::copy(&from, &to)?;
     Ok(json!({}))
   })
 }
@@ -463,7 +487,7 @@ macro_rules! to_seconds {
 
 #[inline(always)]
 fn get_stat_json(
-  metadata: fs::Metadata,
+  metadata: std::fs::Metadata,
   maybe_name: Option<String>,
 ) -> JsonResult {
   // Unix stat member (number types only). 0 if not on unix.
@@ -480,6 +504,8 @@ fn get_stat_json(
     }};
   }
 
+  #[cfg(unix)]
+  use std::os::unix::fs::MetadataExt;
   let mut json_val = json!({
     "isFile": metadata.is_file(),
     "isSymlink": metadata.file_type().is_symlink(),
@@ -526,7 +552,7 @@ fn op_stat(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: StatArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let lstat = args.lstat;
 
   state.check_read(&path)?;
@@ -535,9 +561,9 @@ fn op_stat(
   blocking_json(is_sync, move || {
     debug!("op_stat {} {}", path.display(), lstat);
     let metadata = if lstat {
-      fs::symlink_metadata(&path)?
+      std::fs::symlink_metadata(&path)?
     } else {
-      fs::metadata(&path)?
+      std::fs::metadata(&path)?
     };
     get_stat_json(metadata, None)
   })
@@ -556,7 +582,7 @@ fn op_realpath(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RealpathArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
@@ -565,7 +591,7 @@ fn op_realpath(
     debug!("op_realpath {}", path.display());
     // corresponds to the realpath on Unix and
     // CreateFile and GetFinalPathNameByHandle on Windows
-    let realpath = fs::canonicalize(&path)?;
+    let realpath = std::fs::canonicalize(&path)?;
     let mut realpath_str =
       realpath.to_str().unwrap().to_owned().replace("\\", "/");
     if cfg!(windows) {
@@ -588,14 +614,14 @@ fn op_read_dir(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ReadDirArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_read_dir {}", path.display());
-    let entries: Vec<_> = fs::read_dir(path)?
+    let entries: Vec<_> = std::fs::read_dir(path)?
       .filter_map(|entry| {
         let entry = entry.unwrap();
         let metadata = entry.metadata().unwrap();
@@ -627,8 +653,8 @@ fn op_rename(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: RenameArgs = serde_json::from_value(args)?;
-  let oldpath = deno_fs::resolve_from_cwd(Path::new(&args.oldpath))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
-  let newpath = deno_fs::resolve_from_cwd(Path::new(&args.newpath))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
 
   state.check_read(&oldpath)?;
   state.check_write(&oldpath)?;
@@ -637,7 +663,7 @@ fn op_rename(
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_rename {} {}", oldpath.display(), newpath.display());
-    fs::rename(&oldpath, &newpath)?;
+    std::fs::rename(&oldpath, &newpath)?;
     Ok(json!({}))
   })
 }
@@ -646,8 +672,8 @@ fn op_rename(
 #[serde(rename_all = "camelCase")]
 struct LinkArgs {
   promise_id: Option<u64>,
-  oldname: String,
+  oldpath: String,
-  newname: String,
+  newpath: String,
 }
 
 fn op_link(
@@ -656,16 +682,16 @@ fn op_link(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: LinkArgs = serde_json::from_value(args)?;
-  let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
-  let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
 
-  state.check_read(&oldname)?;
+  state.check_read(&oldpath)?;
-  state.check_write(&newname)?;
+  state.check_write(&newpath)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_link {} {}", oldname.display(), newname.display());
+    debug!("op_link {} {}", oldpath.display(), newpath.display());
-    std::fs::hard_link(&oldname, &newname)?;
+    std::fs::hard_link(&oldpath, &newpath)?;
     Ok(json!({}))
   })
 }
@@ -674,8 +700,8 @@ fn op_link(
 #[serde(rename_all = "camelCase")]
 struct SymlinkArgs {
   promise_id: Option<u64>,
-  oldname: String,
+  oldpath: String,
-  newname: String,
+  newpath: String,
 }
 
 fn op_symlink(
@@ -684,20 +710,28 @@ fn op_symlink(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: SymlinkArgs = serde_json::from_value(args)?;
-  let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?;
+  let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?;
-  let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?;
+  let newpath = resolve_from_cwd(Path::new(&args.newpath))?;
+
+  state.check_write(&newpath)?;
 
-  state.check_write(&newname)?;
-  // TODO Use type for Windows.
-  if cfg!(not(unix)) {
-    return Err(OpError::other("Not implemented".to_string()));
-  }
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
-    debug!("op_symlink {} {}", oldname.display(), newname.display());
+    debug!("op_symlink {} {}", oldpath.display(), newpath.display());
     #[cfg(unix)]
-    std::os::unix::fs::symlink(&oldname, &newname)?;
+    {
+      use std::os::unix::fs::symlink;
+      symlink(&oldpath, &newpath)?;
       Ok(json!({}))
+    }
+    // TODO Implement symlink, use type for Windows
+    #[cfg(not(unix))]
+    {
+      // Unlike with chmod/chown, here we don't
+      // require `oldpath` to exist on Windows
+      let _ = oldpath; // avoid unused warning
+      return Err(OpError::other("Not implemented".to_string()));
+    }
   })
 }
 
@@ -714,14 +748,14 @@ fn op_read_link(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: ReadLinkArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
 
   state.check_read(&path)?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_read_link {}", path.display());
-    let path = fs::read_link(&path)?;
+    let path = std::fs::read_link(&path)?;
     let path_str = path.to_str().unwrap();
 
     Ok(json!(path_str))
@@ -742,7 +776,7 @@ fn op_truncate(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: TruncateArgs = serde_json::from_value(args)?;
-  let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
   let len = args.len;
 
   state.check_write(&path)?;
@@ -750,12 +784,57 @@ fn op_truncate(
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_truncate {} {}", path.display(), len);
-    let f = fs::OpenOptions::new().write(true).open(&path)?;
+    let f = std::fs::OpenOptions::new().write(true).open(&path)?;
     f.set_len(len)?;
     Ok(json!({}))
   })
 }
 
+fn make_temp(
+  dir: Option<&Path>,
+  prefix: Option<&str>,
+  suffix: Option<&str>,
+  is_dir: bool,
+) -> std::io::Result<PathBuf> {
+  let prefix_ = prefix.unwrap_or("");
+  let suffix_ = suffix.unwrap_or("");
+  let mut buf: PathBuf = match dir {
+    Some(ref p) => p.to_path_buf(),
+    None => temp_dir(),
+  }
+  .join("_");
+  let mut rng = thread_rng();
+  loop {
+    let unique = rng.gen::<u32>();
+    buf.set_file_name(format!("{}{:08x}{}", prefix_, unique, suffix_));
+    let r = if is_dir {
+      #[allow(unused_mut)]
+      let mut builder = std::fs::DirBuilder::new();
+      #[cfg(unix)]
+      {
+        use std::os::unix::fs::DirBuilderExt;
+        builder.mode(0o700);
+      }
+      builder.create(buf.as_path())
+    } else {
+      let mut open_options = std::fs::OpenOptions::new();
+      open_options.write(true).create_new(true);
+      #[cfg(unix)]
+      {
+        use std::os::unix::fs::OpenOptionsExt;
+        open_options.mode(0o600);
+      }
+      open_options.open(buf.as_path())?;
+      Ok(())
+    };
+    match r {
+      Err(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
+      Ok(_) => return Ok(buf),
+      Err(e) => return Err(e),
+    }
+  }
+}
+
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct MakeTempArgs {
@@ -772,21 +851,18 @@ fn op_make_temp_dir(
 ) -> Result<JsonOp, OpError> {
   let args: MakeTempArgs = serde_json::from_value(args)?;
 
-  let dir = args
+  let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap());
-    .dir
-    .map(|s| deno_fs::resolve_from_cwd(Path::new(&s)).unwrap());
   let prefix = args.prefix.map(String::from);
   let suffix = args.suffix.map(String::from);
 
-  state
+  state.check_write(dir.clone().unwrap_or_else(temp_dir).as_path())?;
-    .check_write(dir.clone().unwrap_or_else(std::env::temp_dir).as_path())?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     // TODO(piscisaureus): use byte vector for paths, not a string.
     // See https://github.com/denoland/deno/issues/627.
     // We can't assume that paths are always valid utf8 strings.
-    let path = deno_fs::make_temp(
+    let path = make_temp(
       // Converting Option<String> to Option<&str>
       dir.as_ref().map(|x| &**x),
       prefix.as_ref().map(|x| &**x),
@@ -806,21 +882,18 @@ fn op_make_temp_file(
 ) -> Result<JsonOp, OpError> {
   let args: MakeTempArgs = serde_json::from_value(args)?;
 
-  let dir = args
+  let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap());
-    .dir
-    .map(|s| deno_fs::resolve_from_cwd(Path::new(&s)).unwrap());
   let prefix = args.prefix.map(String::from);
   let suffix = args.suffix.map(String::from);
 
-  state
+  state.check_write(dir.clone().unwrap_or_else(temp_dir).as_path())?;
-    .check_write(dir.clone().unwrap_or_else(std::env::temp_dir).as_path())?;
 
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     // TODO(piscisaureus): use byte vector for paths, not a string.
     // See https://github.com/denoland/deno/issues/627.
     // We can't assume that paths are always valid utf8 strings.
-    let path = deno_fs::make_temp(
+    let path = make_temp(
       // Converting Option<String> to Option<&str>
       dir.as_ref().map(|x| &**x),
       prefix.as_ref().map(|x| &**x),
@@ -848,7 +921,10 @@ fn op_utime(
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
   let args: UtimeArgs = serde_json::from_value(args)?;
-  state.check_write(Path::new(&args.path))?;
+  let path = resolve_from_cwd(Path::new(&args.path))?;
+
+  state.check_write(&path)?;
+
   let is_sync = args.promise_id.is_none();
   blocking_json(is_sync, move || {
     debug!("op_utime {} {} {}", args.path, args.atime, args.mtime);
@@ -862,7 +938,7 @@ fn op_cwd(
   _args: Value,
   _zero_copy: Option<ZeroCopyBuf>,
 ) -> Result<JsonOp, OpError> {
-  let path = std::env::current_dir()?;
+  let path = current_dir()?;
   let path_str = path.into_os_string().into_string().unwrap();
   Ok(JsonOp::Sync(json!(path_str)))
 }