Retry renames on Windows.

sdk/lib/_internal/pub/lib/src/io.dart

Index: sdk/lib/_internal/pub/lib/src/io.dart
diff --git a/sdk/lib/_internal/pub/lib/src/io.dart b/sdk/lib/_internal/pub/lib/src/io.dart
index f93f34c6d2eccfb801e5519acf96d8044b2edbfc..7483a5a17b76a65ef0e1be1be7869cf597044bb8 100644
--- a/sdk/lib/_internal/pub/lib/src/io.dart
+++ b/sdk/lib/_internal/pub/lib/src/io.dart
@@ -270,49 +270,57 @@ List<String> listDir(String dir, {bool recursive: false,
 /// a symlink only if that symlink is unbroken and points to a directory.
 bool dirExists(String dir) => new Directory(dir).existsSync();
 
-/// Deletes whatever's at [path], whether it's a file, directory, or symlink. If
-/// it's a directory, it will be deleted recursively.
-void deleteEntry(String path) {
-  tryDeleteEntry() {
-    if (linkExists(path)) {
-      log.io("Deleting link $path.");
-      new Link(path).deleteSync();
-    } else if (dirExists(path)) {
-      log.io("Deleting directory $path.");
-      new Directory(path).deleteSync(recursive: true);
-    } else if (fileExists(path)) {
-      log.io("Deleting file $path.");
-      new File(path).deleteSync();
-    }
-  }
-
+/// Try to resiliently perform [operation].
+///
+/// Some file system operations can intermittently fail on Windows because
+/// other processes are locking a file. We've seen this with virus scanners
+/// when we try to delete or move something while it's being scanned. To
+/// mitigate that, on Windows, this will retry the operation a few times if it
+/// fails.
+void _attempt(String description, void operation()) {
   if (Platform.operatingSystem != 'windows') {
-    tryDeleteEntry();
+    operation();
     return;
   }
 
-  // On Windows, we can fail to delete an entry if it's in use by another
-  // process. The only case where we know this to cause a problem is when
-  // testing "pub serve", since it can poll a file at the same time we try to
-  // delete it in the test process (issue 16129).
-  //
-  // TODO(nweiz): Once issue 14428 is fixed for Windows, remove this special
-  // handling.
   for (var i = 0; i < 2; i++) {
     try {
-      tryDeleteEntry();
+      operation();
+      return;
     } on FileSystemException catch (error) {
-      // Errno 32 indicates that the deletion failed because the file was in
-      // use.
-      if (error.osError.errorCode != 32) rethrow;
+      var reason;
+      if (error.osError.errorCode == 5) {
+        reason = "access was denied";
+      } else if (error.osError.errorCode == 32) {
+        reason = "it was in use by another process";
+      } else {
+        rethrow;
+      }
 
-      log.io("Failed to delete entry because it was in use by another process. "
+      log.io("Failed to $description because $reason. "
           "Retrying in 50ms.");
       sleep(new Duration(milliseconds: 50));
     }
   }
 
-  tryDeleteEntry();
+  operation();

[nweiz, 2014/06/16 20:02:14]

It might be nice to catch the error here as well and print a more detailed error
message explaining that a virus scanner may be causing pub to fail.

[Bob Nystrom, 2014/06/17 21:30:29]

Done.

+}
+
+/// Deletes whatever's at [path], whether it's a file, directory, or symlink. If
+/// it's a directory, it will be deleted recursively.
+void deleteEntry(String path) {
+  _attempt("delete entry", () {
+    if (linkExists(path)) {
+      log.io("Deleting link $path.");
+      new Link(path).deleteSync();
+    } else if (dirExists(path)) {
+      log.io("Deleting directory $path.");
+      new Directory(path).deleteSync(recursive: true);
+    } else if (fileExists(path)) {
+      log.io("Deleting file $path.");
+      new File(path).deleteSync();
+    }
+  });
 }
 
 /// "Cleans" [dir]. If that directory already exists, it will be deleted. Then a
@@ -324,14 +332,16 @@ void cleanDir(String dir) {
 
 /// Renames (i.e. moves) the directory [from] to [to].
 void renameDir(String from, String to) {
-  log.io("Renaming directory $from to $to.");
-  try {
-    new Directory(from).renameSync(to);
-  } on IOException catch (error) {
-    // Ensure that [to] isn't left in an inconsistent state. See issue 12436.
-    if (entryExists(to)) deleteEntry(to);
-    rethrow;
-  }
+  _attempt("rename directory", () {
+    log.io("Renaming directory $from to $to.");
+    try {
+      new Directory(from).renameSync(to);
+    } on IOException catch (error) {
+      // Ensure that [to] isn't left in an inconsistent state. See issue 12436.
+      if (entryExists(to)) deleteEntry(to);
+      rethrow;
+    }
+  });
 }
 
 /// Creates a new symlink at path [symlink] that points to [target]. Returns a