Update the Mac OS watcher in pkg/watcher.
The behavior of Directory.watch has changed, and this updates the
watcher to match the new behavior.
R=rnystrom@google.com
BUG=
Review URL: https://codereview.chromium.org//100823005
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart/pkg/watcher@30915 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/lib/src/directory_watcher/mac_os.dart b/lib/src/directory_watcher/mac_os.dart
index c3e03b8..3e68cd9 100644
--- a/lib/src/directory_watcher/mac_os.dart
+++ b/lib/src/directory_watcher/mac_os.dart
@@ -23,8 +23,8 @@
/// succession, it won't report them in the order they occurred. See issue
/// 14373.
///
-/// This also works around issues 14793, 14806, and 14849 in the implementation
-/// of [Directory.watch].
+/// This also works around issues 15458 and 14849 in the implementation of
+/// [Directory.watch].
class MacOSDirectoryWatcher extends ResubscribableDirectoryWatcher {
MacOSDirectoryWatcher(String directory)
: super(directory, () => new _MacOSDirectoryWatcher(directory));
@@ -106,11 +106,18 @@
for (var event in events) {
if (event is FileSystemCreateEvent) {
if (!event.isDirectory) {
+ // Don't emit ADD events for files or directories that we already
+ // know about. Such an event comes from FSEvents reporting an add
+ // that happened prior to the watch beginning.
+ if (_files.contains(path)) continue;
+
_emitEvent(ChangeType.ADD, path);
_files.add(path);
continue;
}
+ if (_files.containsDir(path)) continue;
+
_listen(Chain.track(new Directory(path).list(recursive: true)),
(entity) {
if (entity is Directory) return;
@@ -164,34 +171,12 @@
set.add(event);
}
- for (var event in batch.where((event) => event is! FileSystemMoveEvent)) {
+ for (var event in batch) {
+ // The Mac OS watcher doesn't emit move events. See issue 14806.
+ assert(event is! FileSystemMoveEvent);
addEvent(event.path, event);
}
- // Issue 14806 means that move events can be misleading if they're in the
- // same batch as another modification of a related file. If they are, we
- // make the event set empty to ensure we check the state of the filesystem.
- // Otherwise, treat them as a DELETE followed by an ADD.
- for (var event in batch.where((event) => event is FileSystemMoveEvent)) {
- if (eventsForPaths.containsKey(event.path) ||
- eventsForPaths.containsKey(event.destination)) {
-
- if (!isInModifiedDirectory(event.path)) {
- eventsForPaths[event.path] = new Set();
- }
- if (!isInModifiedDirectory(event.destination)) {
- eventsForPaths[event.destination] = new Set();
- }
-
- continue;
- }
-
- addEvent(event.path, new ConstructableFileSystemDeleteEvent(
- event.path, event.isDirectory));
- addEvent(event.destination, new ConstructableFileSystemCreateEvent(
- event.path, event.isDirectory));
- }
-
return eventsForPaths;
}
@@ -212,6 +197,7 @@
var type = batch.first.type;
var isDir = batch.first.isDirectory;
+ var hadModifyEvent = false;
for (var event in batch.skip(1)) {
// If one event reports that the file is a directory and another event
@@ -222,7 +208,10 @@
// safely assume the file was modified after a CREATE or before the
// REMOVE; otherwise there will also be a REMOVE or CREATE event
// (respectively) that will be contradictory.
- if (event is FileSystemModifyEvent) continue;
+ if (event is FileSystemModifyEvent) {
+ hadModifyEvent = true;
+ continue;
+ }
assert(event is FileSystemCreateEvent || event is FileSystemDeleteEvent);
// If we previously thought this was a MODIFY, we now consider it to be a
@@ -237,13 +226,23 @@
if (type != event.type) return null;
}
+ // If we got a CREATE event for a file we already knew about, that comes
+ // from FSEvents reporting an add that happened prior to the watch
+ // beginning. If we also received a MODIFY event, we want to report that,
+ // but not the CREATE.
+ if (type == FileSystemEvent.CREATE && hadModifyEvent &&
+ _files.contains(batch.first.path)) {
+ type = FileSystemEvent.MODIFY;
+ }
+
switch (type) {
case FileSystemEvent.CREATE:
- // Issue 14793 means that CREATE events can actually mean DELETE, so we
- // should always check the filesystem for them.
- return null;
+ return new ConstructableFileSystemCreateEvent(batch.first.path, isDir);
case FileSystemEvent.DELETE:
- return new ConstructableFileSystemDeleteEvent(batch.first.path, isDir);
+ // Issue 15458 means that DELETE events for directories can actually
+ // mean CREATE, so we always check the filesystem for them.
+ if (isDir) return null;
+ return new ConstructableFileSystemCreateEvent(batch.first.path, false);
case FileSystemEvent.MODIFY:
return new ConstructableFileSystemModifyEvent(
batch.first.path, isDir, false);
@@ -329,11 +328,6 @@
void _emitEvent(ChangeType type, String path) {
if (!isReady) return;
- // Don't emit ADD events for files that we already know about. Such an event
- // probably comes from FSEvents reporting an add that happened prior to the
- // watch beginning.
- if (type == ChangeType.ADD && _files.contains(path)) return;
-
_eventsController.add(new WatchEvent(type, path));
}
diff --git a/test/directory_watcher/mac_os_test.dart b/test/directory_watcher/mac_os_test.dart
index 7d31221..b71e145 100644
--- a/test/directory_watcher/mac_os_test.dart
+++ b/test/directory_watcher/mac_os_test.dart
@@ -2,8 +2,6 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
-import 'dart:async';
-
import 'package:scheduled_test/scheduled_test.dart';
import 'package:watcher/src/directory_watcher/mac_os.dart';
import 'package:watcher/watcher.dart';
@@ -52,28 +50,6 @@
withPermutations((i, j, k) =>
writeFile("dir/sub/sub-$i/sub-$j/file-$k.txt"));
- // We sleep here because a narrow edge case caused by two interacting bugs
- // can produce events that aren't expected if we start the watcher too
- // soon after creating the files above. Here's what happens:
- //
- // * We create "dir/sub" and its contents above.
- //
- // * We initialize the watcher watching "dir".
- //
- // * Due to issue 14373, the watcher can receive native events describing
- // the creation of "dir/sub" and its contents despite the fact that they
- // occurred before the watcher was started.
- //
- // * Usually the above events will occur while the watcher is doing its
- // initial scan of "dir/sub" and be ignored, but occasionally they will
- // occur afterwards.
- //
- // * When processing the bogus CREATE events, the watcher has to assume that
- // they could mean something other than CREATE (issue 14793). Thus it
- // assumes that the files or directories in question could have changed
- // and emits CHANGE events or additional REMOVE/CREATE pairs for them.
- schedule(() => new Future.delayed(new Duration(seconds: 2)));
-
startWatcher(dir: "dir");
renameDir("dir/sub", "sub");
diff --git a/test/no_subscription/mac_os_test.dart b/test/no_subscription/mac_os_test.dart
index 3862134..e0275c4 100644
--- a/test/no_subscription/mac_os_test.dart
+++ b/test/no_subscription/mac_os_test.dart
@@ -9,32 +9,6 @@
import 'shared.dart';
import '../utils.dart';
-// This is currently failing due to issue 14793. The reason is fairly complex:
-//
-// 1. As part of the test, an "unwatched.txt" file is created while there are no
-// active watchers on the containing directory.
-//
-// 2. A watcher is then added.
-//
-// 3. The watcher lists the contents of the directory and notices that
-// "unwatched.txt" already exists.
-//
-// 4. Since FSEvents reports past changes (issue 14373), the IO event stream
-// emits a CREATED event for "unwatched.txt".
-//
-// 5. Due to issue 14793, the watcher cannot trust that this is really a CREATED
-// event and checks the status of "unwatched.txt" on the filesystem against
-// its internal state.
-//
-// 6. "unwatched.txt" exists on the filesystem and the watcher knows about it
-// internally as well. It assumes this means that the file was modified.
-//
-// 7. The watcher emits an unexpected MODIFIED event for "unwatched.txt",
-// causing the test to fail.
-//
-// Once issue 14793 is fixed, this will no longer be the case and the test will
-// work again.
-
main() {
initConfig();
diff --git a/test/no_subscription/shared.dart b/test/no_subscription/shared.dart
index de625d5..99172a2 100644
--- a/test/no_subscription/shared.dart
+++ b/test/no_subscription/shared.dart
@@ -41,16 +41,6 @@
schedule(() {
completer = new Completer();
subscription = watcher.events.listen(wrapAsync((event) {
- // TODO(nweiz): Remove this when either issue 14373 or 14793 is fixed.
- // Issue 14373 means that the new [Directory.watch] will emit an event
- // for "unwatched.txt" being created, and issue 14793 means we have to
- // check the filesystem, which leads us to assume that the file has been
- // modified.
- if (Platform.isMacOS && event.path.endsWith("unwatched.txt")) {
- expect(event, isWatchEvent(ChangeType.MODIFY, "unwatched.txt"));
- return;
- }
-
// We should get an event for the third file, not the one added while
// we weren't subscribed.
expect(event, isWatchEvent(ChangeType.ADD, "added.txt"));
