Fix recursive directory-deletion of top-level files/links.

BUG=

Review URL: https://codereview.chromium.org//13771010

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@21060 260f80e4-7a28-3924-810f-c04153c831b5

runtime/bin/directory_android.cc

@@ -221,7 +221,7 @@ static bool ListRecursively(PathBuffer* path,
 
 static bool DeleteFile(char* file_name,
                        PathBuffer* path) {
-  return path->Add(file_name) && remove(path->data) == 0;
+  return path->Add(file_name) && unlink(path->data) == 0;
 }
 
 
@@ -235,15 +235,12 @@ static bool DeleteDir(char* dir_name,
 
 static bool DeleteRecursively(PathBuffer* path) {
   // Do not recurse into links for deletion. Instead delete the link.
+  // If it's a file, delete it.
   struct stat st;
   if (TEMP_FAILURE_RETRY(lstat(path->data, &st)) == -1) {
     return false;
-  } else if (S_ISLNK(st.st_mode)) {
-    if (TEMP_FAILURE_RETRY(stat(path->data, &st)) == -1) {
-      return false;
-    } else if (S_ISDIR(st.st_mode)) {
-      return (unlink(path->data) == 0);
-    }
+  } else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
+    return (unlink(path->data) == 0);
   }
 
   if (!path->Add(File::PathSeparator())) return false;

runtime/bin/directory_linux.cc

@@ -221,7 +221,7 @@ static bool ListRecursively(PathBuffer* path,
 
 static bool DeleteFile(char* file_name,
                        PathBuffer* path) {
-  return path->Add(file_name) && remove(path->data) == 0;
+  return path->Add(file_name) && unlink(path->data) == 0;
 }
 
 
@@ -235,15 +235,12 @@ static bool DeleteDir(char* dir_name,
 
 static bool DeleteRecursively(PathBuffer* path) {
   // Do not recurse into links for deletion. Instead delete the link.
+  // If it's a file, delete it.
   struct stat st;
   if (TEMP_FAILURE_RETRY(lstat(path->data, &st)) == -1) {
     return false;
-  } else if (S_ISLNK(st.st_mode)) {
-    if (TEMP_FAILURE_RETRY(stat(path->data, &st)) == -1) {
-      return false;
-    } else if (S_ISDIR(st.st_mode)) {
-      return (unlink(path->data) == 0);
-    }
+  } else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
+    return (unlink(path->data) == 0);
   }
 
   if (!path->Add(File::PathSeparator())) return false;

runtime/bin/directory_macos.cc

@@ -221,7 +221,7 @@ static bool ListRecursively(PathBuffer* path,
 
 static bool DeleteFile(char* file_name,
                        PathBuffer* path) {
-  return path->Add(file_name) && remove(path->data) == 0;
+  return path->Add(file_name) && unlink(path->data) == 0;
 }
 
 
@@ -235,15 +235,12 @@ static bool DeleteDir(char* dir_name,
 
 static bool DeleteRecursively(PathBuffer* path) {
   // Do not recurse into links for deletion. Instead delete the link.
+  // If it's a file, delete it.
   struct stat st;
   if (TEMP_FAILURE_RETRY(lstat(path->data, &st)) == -1) {
     return false;
-  } else if (S_ISLNK(st.st_mode)) {
-    if (TEMP_FAILURE_RETRY(stat(path->data, &st)) == -1) {
-      return false;
-    } else if (S_ISDIR(st.st_mode)) {
-      return (unlink(path->data) == 0);
-    }
+  } else if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
+    return (unlink(path->data) == 0);
   }
 
   if (!path->Add(File::PathSeparator())) return false;

runtime/bin/directory_win.cc

@@ -263,17 +263,19 @@ static bool DeleteEntry(LPWIN32_FIND_DATAW find_file_data, PathBuffer* path) {
 
 
 static bool DeleteRecursively(PathBuffer* path) {
+  DWORD attributes = GetFileAttributesW(path->data);
+  if ((attributes == INVALID_FILE_ATTRIBUTES)) {
+    return false;
+  }
   // If the directory is a junction, it's pointing to some other place in the
   // filesystem that we do not want to recurse into.
-  DWORD attributes = GetFileAttributesW(path->data);
-  if ((attributes != INVALID_FILE_ATTRIBUTES) &&
-      (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
-    if (IsBrokenLink(path->data)) {
-      return false;
-    } else {
-      // Just delete the junction itself.
-      return RemoveDirectoryW(path->data) != 0;
-    }
+  if ((attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
+    // Just delete the junction itself.
+    return RemoveDirectoryW(path->data) != 0;
+  }
+  // If it's a file, remove it directly.
+  if ((attributes & FILE_ATTRIBUTE_DIRECTORY) == 0) {
+    return DeleteFile(L"", path);
   }
 
   if (!path->Add(L"\\*")) return false;

tests/standalone/io/directory_error_test.dart

@@ -95,13 +95,8 @@ bool checkDeleteRecursivelyNonExistentFileException(e) {
   Expect.isTrue(e is DirectoryIOException);
   Expect.isTrue(e.osError != null);
   Expect.isTrue(e.toString().indexOf("Deletion failed") != -1);
-  if (Platform.operatingSystem == "linux") {
-    Expect.equals(2, e.osError.errorCode);
-  } else if (Platform.operatingSystem == "macos") {
-    Expect.equals(2, e.osError.errorCode);
-  } else if (Platform.operatingSystem == "windows") {
-    Expect.equals(3, e.osError.errorCode);
-  }
+  // File not not found has error code 2 on all supported platforms.
+  Expect.equals(2, e.osError.errorCode);
 
   return true;
 }

tests/standalone/io/file_system_delete_test.dart

@@ -27,11 +27,13 @@ void testDeleteFileSync() {
   file.createSync();
 
   Expect.isTrue(file.existsSync());
-  Expect.throws(() => new Directory(file.path).deleteSync());
-  Expect.isTrue(file.existsSync());
+  new Directory(file.path).deleteSync(recursive: true);
+  Expect.isFalse(file.existsSync());
+
+  file.createSync();
 
   Expect.isTrue(file.existsSync());
-  Expect.throws(() => new Directory(file.path).deleteSync(recursive: true));
+  Expect.throws(() => new Directory(file.path).deleteSync());
   Expect.isTrue(file.existsSync());
 
   Expect.isTrue(file.existsSync());
@@ -56,12 +58,13 @@ void testDeleteFile() {
       .then((_) => file.create())
 
       .then((_) => file.exists().then(Expect.isTrue))
-      .then((_) => throws(() => new Directory(file.path).delete()))
-      .then((_) => file.exists().then(Expect.isTrue))
+      .then((_) => new Directory(file.path).delete(recursive: true))
+      .then((_) => file.exists().then(Expect.isFalse))
+
+      .then((_) => file.create())
 
       .then((_) => file.exists().then(Expect.isTrue))
-      .then((_) => throws(
-          () => new Directory(file.path).delete(recursive: true)))
+      .then((_) => throws(() => new Directory(file.path).delete()))
       .then((_) => file.exists().then(Expect.isTrue))
 
       .then((_) => file.exists().then(Expect.isTrue))
@@ -161,11 +164,13 @@ void testDeleteFileLinkSync() {
   link.createSync(file.path);
 
   Expect.isTrue(link.existsSync());
-  Expect.throws(() => new Directory(link.path).deleteSync());
-  Expect.isTrue(link.existsSync());
+  new Directory(link.path).deleteSync(recursive: true);
+  Expect.isFalse(link.existsSync());
+
+  link.createSync(file.path);
 
   Expect.isTrue(link.existsSync());
-  Expect.throws(() => new Directory(link.path).deleteSync(recursive: true));
+  Expect.throws(() => new Directory(link.path).deleteSync());
   Expect.isTrue(link.existsSync());
 
   link.deleteSync();
@@ -199,12 +204,13 @@ void testDeleteFileLink() {
       .then((_) => link.create(file.path))
 
       .then((_) => link.exists().then(Expect.isTrue))
-      .then((_) => throws(() => new Directory(link.path).delete()))
-      .then((_) => link.exists().then(Expect.isTrue))
+      .then((_) => new Directory(link.path).delete(recursive: true))
+      .then((_) => link.exists().then(Expect.isFalse))
+
+      .then((_) => link.create(file.path))
 
       .then((_) => link.exists().then(Expect.isTrue))
-      .then((_) => throws(
-          () => new Directory(link.path).delete(recursive: true)))
+      .then((_) => throws(() => new Directory(link.path).delete()))
       .then((_) => link.exists().then(Expect.isTrue))
 
       .then((_) => link.deleteSync())
@@ -323,11 +329,15 @@ void testDeleteBrokenLinkSync() {
   directory.deleteSync();
 
   Expect.isTrue(link.existsSync());
-  Expect.throws(() => new Directory(link.path).deleteSync());
-  Expect.isTrue(link.existsSync());
+  new Directory(link.path).deleteSync(recursive: true);
+  Expect.isFalse(link.existsSync());
+
+  directory.createSync();
+  link.createSync(directory.path);
+  directory.deleteSync();
 
   Expect.isTrue(link.existsSync());
-  Expect.throws(() => new Directory(link.path).deleteSync(recursive: true));
+  Expect.throws(() => new Directory(link.path).deleteSync());
   Expect.isTrue(link.existsSync());
 
   Expect.isTrue(link.existsSync());
@@ -358,12 +368,15 @@ void testDeleteBrokenLink() {
       .then((_) => dir.delete())
 
       .then((_) => link.exists().then(Expect.isTrue))
-      .then((_) => throws(() => new Directory(link.path).delete()))
-      .then((_) => link.exists().then(Expect.isTrue))
+      .then((_) => new Directory(link.path).delete(recursive: true))
+      .then((_) => link.exists().then(Expect.isFalse))
+
+      .then((_) => dir.create())
+      .then((_) => link.create(dir.path))
+      .then((_) => dir.delete())
 
       .then((_) => link.exists().then(Expect.isTrue))
-      .then((_) => throws(
-          () => new Directory(link.path).delete(recursive: true)))
+      .then((_) => throws(() => new Directory(link.path).delete()))
       .then((_) => link.exists().then(Expect.isTrue))
 
       .then((_) => link.exists().then(Expect.isTrue))

utils/pub/io.dart

@@ -188,12 +188,7 @@ bool dirExists(String dir) => new Directory(dir).existsSync();
 void deleteEntry(String path) {
   if (linkExists(path)) {
     log.io("Deleting link $path.");
-    if (Platform.operatingSystem == 'windows') {
-      // TODO(nweiz): remove this when issue 9278 is fixed.
-      new Directory(path).deleteSync();
-    } else {
-      new Link(path).deleteSync();
-    }
+    new Link(path).deleteSync();
   } else if (dirExists(path)) {
     log.io("Deleting directory $path.");
     new Directory(path).deleteSync(recursive: true);