From 73ac74cce9d4dcafc4a3075c5cc191b7c2d91f47 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 16 Jun 2017 10:29:32 -0400 Subject: [PATCH 01/94] Use nsfw for file watching --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 1ed1c87fb..826ef5926 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "mocha": "2.5.1", "mock-spawn": "^0.2.6", "normalize-package-data": "^2.0.0", + "nsfw": "^1.0.15", "nslog": "^3", "oniguruma": "6.2.1", "pathwatcher": "7.1.0", From a1ccd49b8e0a336a1b39e672b1f2b5646558eb27 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 19 Jun 2017 15:08:34 -0400 Subject: [PATCH 02/94] Use a tree-backed registry to deduplicate and consolidate native watchers --- spec/native-watcher-registry-spec.js | 113 +++++++++++++ src/native-watcher-registry.js | 240 +++++++++++++++++++++++++++ 2 files changed, 353 insertions(+) create mode 100644 spec/native-watcher-registry-spec.js create mode 100644 src/native-watcher-registry.js diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js new file mode 100644 index 000000000..fd8d898ec --- /dev/null +++ b/spec/native-watcher-registry-spec.js @@ -0,0 +1,113 @@ +/** @babel */ + +import path from 'path' + +import NativeWatcherRegistry from '../src/native-watcher-registry' + +class MockWatcher { + constructor () { + this.native = null + } + + attachToNative (native) { + this.native = native + this.native.attached.push(this) + } +} + +class MockNative { + constructor (name) { + this.name = name + this.attached = [] + this.disposed = false + this.stopped = false + } + + reattachTo (newNative) { + for (const watcher of this.attached) { + watcher.attachToNative(newNative) + } + + this.attached = [] + } + + dispose() { + this.disposed = true + } + + stop() { + this.stopped = true + } +} + +describe('NativeWatcherRegistry', function () { + let registry, watcher + + beforeEach(function () { + registry = new NativeWatcherRegistry() + watcher = new MockWatcher() + }) + + it('attaches a Watcher to a newly created NativeWatcher for a new directory', function() { + const NATIVE = new MockNative('created') + registry.attach('/some/path', watcher, () => NATIVE) + + expect(watcher.native).toBe(NATIVE) + }) + + it('reuses an existing NativeWatcher on the same directory', function () { + const EXISTING = new MockNative('existing') + registry.attach('/existing/path', new MockWatcher(), () => EXISTING) + + registry.attach('/existing/path', watcher, () => new MockNative('no')) + + expect(watcher.native).toBe(EXISTING) + }) + + it('attaches to an existing NativeWatcher on a parent directory', function () { + const EXISTING = new MockNative('existing') + registry.attach('/existing/path', new MockWatcher(), () => EXISTING) + + registry.attach('/existing/path/sub/directory/', watcher, () => new MockNative('no')) + + expect(watcher.native).toBe(EXISTING) + }) + + it('adopts Watchers from NativeWatchers on child directories', function () { + const EXISTING0 = new MockNative('existing0') + const watcher0 = new MockWatcher() + registry.attach('/existing/path/child/directory/zero', watcher0, () => EXISTING0) + + const EXISTING1 = new MockNative('existing1') + const watcher1 = new MockWatcher() + registry.attach('/existing/path/child/directory/one', watcher1, () => EXISTING1) + + const EXISTING2 = new MockNative('existing2') + const watcher2 = new MockWatcher() + registry.attach('/another/path', watcher2, () => EXISTING2) + + expect(watcher0.native).toBe(EXISTING0) + expect(watcher1.native).toBe(EXISTING1) + expect(watcher2.native).toBe(EXISTING2) + + // Consolidate all three watchers beneath the same native watcher on the parent directory + const CREATED = new MockNative('created') + registry.attach('/existing/path/', watcher, () => CREATED) + + expect(watcher.native).toBe(CREATED) + + expect(watcher0.native).toBe(CREATED) + expect(EXISTING0.stopped).toBe(true) + expect(EXISTING0.disposed).toBe(true) + + expect(watcher1.native).toBe(CREATED) + expect(EXISTING1.stopped).toBe(true) + expect(EXISTING1.disposed).toBe(true) + + expect(watcher2.native).toBe(EXISTING2) + expect(EXISTING2.stopped).toBe(false) + expect(EXISTING2.disposed).toBe(false) + }) + + it('removes NativeWatchers when all Watchers have been disposed') +}) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js new file mode 100644 index 000000000..e78c87c9c --- /dev/null +++ b/src/native-watcher-registry.js @@ -0,0 +1,240 @@ +/** @babel */ + +import path from 'path' + +// Private: Non-leaf node in a tree used by the {NativeWatcherRegistry} to cover the allocated {Watcher} instances with +// the most efficient set of {NativeWatcher} instances possible. Each {RegistryNode} maps to a directory in the +// filesystem tree. +class RegistryNode { + + // Private: Construct a new, empty node representing a node with no watchers. + constructor () { + this.children = {} + } + + // Private: Recursively discover any existing watchers corresponding to a path. + // + // * `pathSegments` filesystem path of a new {Watcher} already split into an Array of directory names. + // + // Returns: A {ParentResult} if the exact requested directory or a parent directory is being watched, a + // {ChildrenResult} if one or more child paths are being watched, or a {MissingResult} if no relevant watchers + // exist. + lookup(pathSegments) { + if (pathSegments.length === 0) { + return new ChildrenResult(this.leaves()) + } + + const child = this.children[pathSegments[0]] + if (child === undefined) { + return new MissingResult(this) + } + + return child.lookup(pathSegments.slice(1)) + } + + // Private: Insert a new {RegistryWatcherNode} into the tree, creating new intermediate {RegistryNode} instances as + // needed. Any existing children of the watched directory are removed. + // + // * `pathSegments` filesystem path of the new {Watcher}, already split into an Array of directory names. + // * `leaf` initialized {RegistryWatcherNode} to insert + // + // Returns: The root of a new tree with the {RegistryWatcherNode} inserted at the correct location. Callers should + // replace their node references with the returned value. + insert(pathSegments, leaf) { + if (pathSegments.length === 0) { + return leaf + } + + const pathKey = pathSegments[0] + let child = this.children[pathKey] + if (child === undefined) { + child = new RegistryNode() + } + this.children[pathKey] = child.insert(pathSegments.slice(1), leaf) + return this + } + + // Private: Discover all {RegistryWatcherNode} instances beneath this tree node. + // + // Returns: A possibly empty {Array} of {RegistryWatcherNode} instances that are the descendants of this node. + leaves() { + const results = [] + for (const p of Object.keys(this.children)) { + results.push(...this.children[p].leaves()) + } + return results + } +} + +// Private: Leaf node within a {NativeWatcherRegistry} tree. Represents a directory that is covered by a +// {NativeWatcher}. +class RegistryWatcherNode { + + // Private: Allocate a new node to track a {NativeWatcher}. + // + // * `nativeWatcher` An existing {NativeWatcher} instance. + constructor (nativeWatcher) { + this.nativeWatcher = nativeWatcher + } + + // Private: Accessor for the {NativeWatcher}. + getNativeWatcher () { + return this.nativeWatcher + } + + // Private: Identify how this watcher relates to a request to watch a directory tree. + // + // * `pathSegments` filesystem path of a new {Watcher} already split into an Array of directory names. + // + // Returns: A {ParentResult} referencing this node. + lookup(pathSegments) { + return new ParentResult(this, pathSegments) + } + + // Private: Discover this {RegistryWatcherNode} instance. + // + // Returns: An {Array} containing this node. + leaves() { + return [this] + } +} + +// Private: A {RegisteryNode} traversal result that's returned when neither a directory, its children, nor its parents +// are present in the tree. +class MissingResult { + // Private: Instantiate a new {MissingResult}. + // + // * `lastParent` the final succesfully traversed {RegistryNode}. + constructor (lastParent) { + this.lastParent = lastParent + } + + // Private: Dispatch within a map of callback actions. + // + // * `actions` {Object} containing a `missing` key that maps to a callback to be invoked when no results were returned + // by {RegistryNode.lookup}. The callback will be called with the last parent node that was encountered during the + // traversal. + // + // Returns: the result of the `actions` callback. + when (actions) { + return actions.missing(this.lastParent) + } +} + +// Private: A {RegistryNode.lookup} traversal result that's returned when a parent or an exact match of the requested +// directory is being watched by an existing {RegistryWatcherNode}. +class ParentResult { + + // Private: Instantiate a new {ParentResult}. + // + // * `parent` the {RegistryWatcherNode} that was discovered. + // * `remainingPathSegments` an {Array} of the directories that lie between the leaf node's watched directory and + // the requested directory. This will be empty for exact matches. + constructor (parent, remainingPathSegments) { + this.parent = parent + this.remainingPathSegments = remainingPathSegments + } + + // Private: Dispatch within a map of callback actions. + // + // * `actions` {Object} containing a `parent` key that maps to a callback to be invoked when a parent of a requested + // requested directory is returned by a {RegistryNode.lookup} call. The callback will be called with the + // {RegistryWatcherNode} instance and an {Array} of the {String} path segments that separate the parent node + // and the requested directory. + // + // Returns: the result of the `actions` callback. + when (actions) { + return actions.parent(this.parent, this.remainingPathSegments) + } +} + +// Private: A {RegistryNode.lookup} traversal result that's returned when one or more children of the requested +// directory are already being watched. +class ChildrenResult { + + // Private: Instantiate a new {ChildrenResult}. + // + // * `children` {Array} of the {RegistryWatcherNode} instances that were discovered. + constructor (children) { + this.children = children + } + + // Private: Dispatch within a map of callback actions. + // + // * `actions` {Object} containing a `children` key that maps to a callback to be invoked when a parent of a requested + // requested directory is returned by a {RegistryNode.lookup} call. The callback will be called with the + // {RegistryWatcherNode} instance. + // + // Returns: the result of the `actions` callback. + when (actions) { + return actions.children(this.children) + } +} + +// Private: Track the directories being monitored by native filesystem watchers. Minimize the number of native watchers +// allocated to receive events for a desired set of directories by: +// +// 1. Subscribing to the same underlying {NativeWatcher} when watching the same directory multiple times. +// 2. Subscribing to an existing {NativeWatcher} on a parent of a desired directory. +// 3. Replacing multiple {NativeWatcher} instances on child directories with a single new {NativeWatcher} on the +// parent. +export default class NativeWatcherRegistry { + + // Private: Instantiate an empty registry. + constructor () { + this.tree = new RegistryNode() + } + + // Private: Attach a watcher to a directory, assigning it a {NativeWatcher}. If a suitable {NativeWatcher} already + // exists, it will be attached to the new {Watcher} with an appropriate subpath configuration. Otherwise, the + // `createWatcher` callback will be invoked to create a new {NativeWatcher}, which will be registered in the tree + // and attached to the watcher. + // + // If any pre-existing child watchers are removed as a result of this operation, {NativeWatcher.onWillReattach} will + // be broadcast on each with the new parent watcher as an event payload to give child watchers a chance to attach to + // the new watcher. + // + // * `directory` a normalized path to be watched as a {String}. + // * `watcher` an unattached {Watcher}. + // * `createNative` callback to be invoked if no existing {NativeWatcher} covers the {Watcher}. It should + // synchronously return a new {NativeWatcher} instance watching {directory}. + attach (directory, watcher, createNative) { + const pathSegments = directory.split(path.sep).filter(segment => segment.length > 0) + + const attachToNew = () => { + const native = createNative() + const leaf = new RegistryWatcherNode(native) + this.tree = this.tree.insert(pathSegments, leaf) + + watcher.attachToNative(native, '') + + return native + } + + this.tree.lookup(pathSegments).when({ + parent: (parent, remaining) => { + // An existing NativeWatcher is watching a parent directory of the requested path. Attach this Watcher to + // it as a filtering watcher. + const native = parent.getNativeWatcher() + const subpath = remaining.length === 0 ? '' : path.join(...remaining) + + watcher.attachToNative(native, subpath) + }, + children: children => { + const newNative = attachToNew() + + // One or more NativeWatchers exist on child directories of the requested path. + for (let i = 0; i < children.length; i++) { + const child = children[i] + const childNative = child.getNativeWatcher() + childNative.reattachTo(newNative, directory) + childNative.dispose() + + // Don't await this Promise. Subscribers can listen for `onDidStop` to be notified if they choose. + childNative.stop() + } + }, + missing: attachToNew + }) + } +} From 908e5ad1e93112dc46a812bf46025fbdc2c30ca6 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 19 Jun 2017 07:36:07 -0400 Subject: [PATCH 03/94] FileSystemManager that hands out Watchers to subscribe to filesystem events --- src/atom-environment.coffee | 5 + src/filesystem-manager.js | 201 ++++++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 src/filesystem-manager.js diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index b37acddd1..12a1ad192 100644 --- a/src/atom-environment.coffee +++ b/src/atom-environment.coffee @@ -46,6 +46,7 @@ TextBuffer = require 'text-buffer' Gutter = require './gutter' TextEditorRegistry = require './text-editor-registry' AutoUpdateManager = require './auto-update-manager' +FileSystemManager = require './filesystem-manager' # Essential: Atom global for dealing with packages, themes, menus, and the window. # @@ -117,6 +118,9 @@ class AtomEnvironment extends Model # Private: An {AutoUpdateManager} instance autoUpdater: null + # Public: A {FileSystemManager} instance + filesystem: null + saveStateDebounceInterval: 1000 ### @@ -137,6 +141,7 @@ class AtomEnvironment extends Model @views = new ViewRegistry(this) TextEditor.setScheduler(@views) @notifications = new NotificationManager + @filesystem = new FileSystemManager @updateProcessEnv ?= updateProcessEnv # For testing @stateStore = new StateStore('AtomEnvironments', 1) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js new file mode 100644 index 000000000..de9efe36a --- /dev/null +++ b/src/filesystem-manager.js @@ -0,0 +1,201 @@ +/** @babel */ + +import fs from 'fs' +import path from 'path' + +import {Emitter, Disposable, CompositeDisposable} from 'event-kit' +import nsfw from 'nsfw' +const {MODIFIED, CREATED, DELETED, RENAMED} = nsfw.actions + +import NativeWatcherRegistry from './native-watcher-registry' + +// Private: Associate native watcher action type flags with descriptive String equivalents. +const ACTION_MAP = new Map([ + [nsfw.actions.MODIFIED, 'changed'], + [nsfw.actions.CREATED, 'added'], + [nsfw.actions.DELETED, 'deleted'], + [nsfw.actions.RENAMED, 'renamed'] +]) + +// Private: Interface with and normalize events from a native OS filesystem watcher. +class NativeWatcher { + + // Private: Initialize a native watcher on a path. + // + // Events will not be produced until {start()} is called. + constructor (normalizedPath) { + this.normalizedPath = normalizedPath + this.emitter = new Emitter() + + this.watcher = null + this.running = false + this.refCount = 0 + } + + // Private: Begin watching for filesystem events. + // + // Has no effect if the watcher has already been started. + async start () { + if (this.running) { + return + } + + this.watcher = await nsfw( + this.normalizedPath, + this.onEvents.bind(this), + { + debounceMS: 100, + errorCallback: this.onError.bind(this) + } + ) + + await this.watcher.start() + + this.running = true + this.emitter.emit('did-start') + } + + // Private: Return true if the underlying watcher has been started. + isRunning () { + return this.running + } + + // Private: Register a callback to be invoked when the filesystem watcher has been initialized. + // + // Returns: A {Disposable} to revoke the subscription. + onDidStart (callback) { + return this.emitter.on('did-start', callback) + } + + // Private: Register a callback to be invoked with normalized filesystem events as they arrive. + // + // Returns: A {Disposable} to revoke the subscription. + onDidChange (callback) { + return this.emitter.on('did-change', callback) + } + + // Private: Register a callback to be invoked when a {Watcher} should attach to a different {NativeWatcher}. + // + // Returns: A {Disposable} to revoke the subscription. + onShouldDetach (callback) { + return this.emitter.on('should-detach', callback) + } + + // Private: Register a callback to be invoked when the filesystem watcher has been initialized. + // + // Returns: A {Disposable} to revoke the subscription. + onDidStop (callback) { + return this.emitter.on('did-stop', callback) + } + + // Private: Broadcast an `onShouldDetach` event to prompt any {Watcher} instances bound here to attach to a new + // {NativeWatcher} instead. + reattachTo (other) { + this.emitter.emit('should-detach', other) + } + + // Private: Stop the native watcher and release any operating system resources associated with it. + // + // Has no effect if the watcher is not running. + async stop() { + if (!this.running) { + return + } + + await this.watcher.stop() + this.running = false + this.emitter.emit('did-stop') + } + + // Private: Callback function invoked by the native watcher when a debounced group of filesystem events arrive. + // Normalize and re-broadcast them to any subscribers. + // + // * `events` An Array of filesystem events. + onEvents (events) { + this.emitter.emit('did-change', events.map(event => { + const type = ACTION_MAP.get(event.action) || `unexpected (${event.action})` + const oldFileName = event.file || event.oldFile + const newFileName = event.newFile + const oldPath = path.join(event.directory, oldFileName) + const newPath = newFileName && path.join(event.directory, newFileName) + + return {oldPath, newPath, type} + })) + } + + // Private: Callback function invoked by the native watcher when an error occurs. + // + // * `err` The native filesystem error. + onError (err) { + // + } +} + +class Watcher { + constructor (watchedPath) { + this.watchedPath = watchedPath + this.normalizedPath = null + + this.emitter = new Emitter() + this.subs = new CompositeDisposable() + } + + onDidStart (callback) { + return this.emitter.on('did-start', callback) + } + + onDidChange (callback) { + return this.emitter.on('did-change', callback) + } + + attachToNative (native) { + this.subs.dispose() + + if (native.isRunning()) { + this.emitter.emit('did-start') + } else { + this.subs.add(native.onDidStart(payload => { + this.emitter.emit('did-start', payload) + })) + } + + this.subs.add(native.onDidChange(events => { + // TODO does event.oldPath resolve symlinks? + const filtered = events.filter(event => event.oldPath.startsWith(this.normalizedPath)) + + if (filtered.length > 0) { + this.emitter.emit('did-change', filtered) + } + })) + + this.subs.add(native.onShouldDetach( + this.attachToNative.bind(this) + )) + } + + dispose () { + this.emitter.dispose() + this.subs.dispose() + } +} + +export default class FileSystemManager { + constructor () { + this.nativeWatchers = new NativeWatcherRegistry() + } + + getWatcher (rootPath) { + const watcher = new Watcher(rootPath) + + (async () => { + const normalizedPath = await new Promise((resolve, reject) => { + fs.realpath(rootPath, (err, real) => (err ? reject(err) : resolve(real))) + }) + watcher.normalizedPath = normalizedPath + + this.nativeWatchers.attach(normalizedPath, watcher, () => new NativeWatcher(normalizedPath)) + })() + + return watcher + } +} From 366ee19bd951651cb9f56127e5afbd3fdb760de4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 14:38:56 -0400 Subject: [PATCH 04/94] :shirt: make the linter happy --- spec/native-watcher-registry-spec.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index fd8d898ec..e5e880667 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -1,7 +1,5 @@ /** @babel */ -import path from 'path' - import NativeWatcherRegistry from '../src/native-watcher-registry' class MockWatcher { @@ -31,11 +29,11 @@ class MockNative { this.attached = [] } - dispose() { + dispose () { this.disposed = true } - stop() { + stop () { this.stopped = true } } @@ -48,7 +46,7 @@ describe('NativeWatcherRegistry', function () { watcher = new MockWatcher() }) - it('attaches a Watcher to a newly created NativeWatcher for a new directory', function() { + it('attaches a Watcher to a newly created NativeWatcher for a new directory', function () { const NATIVE = new MockNative('created') registry.attach('/some/path', watcher, () => NATIVE) From bd76773412e609284b5cb372ed8545bb9705f1ea Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 14:48:39 -0400 Subject: [PATCH 05/94] :shirt: lint lint lint --- src/native-watcher-registry.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index e78c87c9c..c0b7d7c0d 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -19,7 +19,7 @@ class RegistryNode { // Returns: A {ParentResult} if the exact requested directory or a parent directory is being watched, a // {ChildrenResult} if one or more child paths are being watched, or a {MissingResult} if no relevant watchers // exist. - lookup(pathSegments) { + lookup (pathSegments) { if (pathSegments.length === 0) { return new ChildrenResult(this.leaves()) } @@ -40,7 +40,7 @@ class RegistryNode { // // Returns: The root of a new tree with the {RegistryWatcherNode} inserted at the correct location. Callers should // replace their node references with the returned value. - insert(pathSegments, leaf) { + insert (pathSegments, leaf) { if (pathSegments.length === 0) { return leaf } @@ -57,7 +57,7 @@ class RegistryNode { // Private: Discover all {RegistryWatcherNode} instances beneath this tree node. // // Returns: A possibly empty {Array} of {RegistryWatcherNode} instances that are the descendants of this node. - leaves() { + leaves () { const results = [] for (const p of Object.keys(this.children)) { results.push(...this.children[p].leaves()) @@ -87,14 +87,14 @@ class RegistryWatcherNode { // * `pathSegments` filesystem path of a new {Watcher} already split into an Array of directory names. // // Returns: A {ParentResult} referencing this node. - lookup(pathSegments) { + lookup (pathSegments) { return new ParentResult(this, pathSegments) } // Private: Discover this {RegistryWatcherNode} instance. // // Returns: An {Array} containing this node. - leaves() { + leaves () { return [this] } } From 9c9625eb766bb75f53d1a2e807cc4efa66f1e66e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 15:44:05 -0400 Subject: [PATCH 06/94] Helpers to promisify functions in specs --- spec/async-spec-helpers.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/async-spec-helpers.js b/spec/async-spec-helpers.js index 8bc36c913..84a592c6a 100644 --- a/spec/async-spec-helpers.js +++ b/spec/async-spec-helpers.js @@ -72,3 +72,27 @@ export function emitterEventPromise (emitter, event, timeout = 15000) { }) }) } + +export function promisify (original) { + return function (...args) { + return new Promise((resolve, reject) => { + args.push((err, ...results) => { + if (err) { + reject(err) + } else { + resolve(...results) + } + }) + + return original(...args) + }) + } +} + +export function promisifySome (obj, fnNames) { + const result = {} + for (fnName of fnNames) { + result[fnName] = promisify(obj[fnName]) + } + return result +} From e4c48a5c8c887878f6e410d61776a0281661a17a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 15:44:57 -0400 Subject: [PATCH 07/94] :shirt: for FileSystemManager --- src/filesystem-manager.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index de9efe36a..6bdbb9f01 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -3,9 +3,8 @@ import fs from 'fs' import path from 'path' -import {Emitter, Disposable, CompositeDisposable} from 'event-kit' +import {Emitter, CompositeDisposable} from 'event-kit' import nsfw from 'nsfw' -const {MODIFIED, CREATED, DELETED, RENAMED} = nsfw.actions import NativeWatcherRegistry from './native-watcher-registry' @@ -29,7 +28,6 @@ class NativeWatcher { this.watcher = null this.running = false - this.refCount = 0 } // Private: Begin watching for filesystem events. @@ -97,7 +95,7 @@ class NativeWatcher { // Private: Stop the native watcher and release any operating system resources associated with it. // // Has no effect if the watcher is not running. - async stop() { + async stop () { if (!this.running) { return } @@ -127,7 +125,7 @@ class NativeWatcher { // // * `err` The native filesystem error. onError (err) { - // + console.error(err) } } @@ -187,14 +185,15 @@ export default class FileSystemManager { getWatcher (rootPath) { const watcher = new Watcher(rootPath) - (async () => { + const init = async () => { const normalizedPath = await new Promise((resolve, reject) => { fs.realpath(rootPath, (err, real) => (err ? reject(err) : resolve(real))) }) watcher.normalizedPath = normalizedPath this.nativeWatchers.attach(normalizedPath, watcher, () => new NativeWatcher(normalizedPath)) - })() + } + init() return watcher } From 39085ce3dca2b977a9fb2dd74a94f1b0d38f8b9c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 15:45:36 -0400 Subject: [PATCH 08/94] First few FileSystemManager specs --- spec/filesystem-manager-spec.js | 137 ++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 spec/filesystem-manager-spec.js diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js new file mode 100644 index 000000000..dbc79fa52 --- /dev/null +++ b/spec/filesystem-manager-spec.js @@ -0,0 +1,137 @@ +/** @babel */ + +import {it, beforeEach, afterEach, promisifySome} from './async-spec-helpers' +import tempCb from 'temp' +import fsCb from 'fs-plus' +import path from 'path' + +import {CompositeDisposable} from 'event-kit' +import FileSystemManager from '../src/filesystem-manager' + +tempCb.track() + +const fs = promisifySome(fsCb, ['writeFile', 'mkdir', 'symlink', 'appendFile', 'realpath']) +const temp = promisifySome(tempCb, ['mkdir', 'cleanup']) + +describe('FileSystemManager', function () { + let subs, manager + + beforeEach(function () { + subs = new CompositeDisposable() + manager = new FileSystemManager() + }) + + afterEach(async function () { + subs.dispose() + + const cleanup = [temp.cleanup()] + const nativeWatchers = manager.nativeWatchers.tree.leaves().map(node => node.getNativeWatcher()) + for (const native of nativeWatchers) { + cleanup.push(native.stop()) + } + await Promise.all(cleanup) + }) + + function waitForEvent (fn) { + return new Promise(resolve => { + subs.add(fn(resolve)) + }) + } + + function waitForChanges (watcher, ...fileNames) { + const waiting = new Set(fileNames) + const relevantEvents = [] + + return new Promise(resolve => { + const sub = watcher.onDidChange(events => { + for (const event of events) { + if (waiting.delete(event.oldPath)) { + relevantEvents.push(event) + } + } + + if (waiting.size === 0) { + resolve(relevantEvents) + sub.dispose() + } + }) + }) + } + + describe('getWatcher()', function () { + it('broadcasts onDidStart when the watcher begins listening', async function () { + const rootDir = await temp.mkdir('atom-fsmanager-') + + const watcher = manager.getWatcher(rootDir) + await waitForEvent(cb => watcher.onDidStart(cb)) + }) + + it('reuses an existing native watcher and broadcasts onDidStart immediately if attached to an existing watcher', async function () { + const rootDir = await temp.mkdir('atom-fsmanager-') + + const watcher0 = manager.getWatcher(rootDir) + await waitForEvent(cb => watcher0.onDidStart(cb)) + + const watcher1 = manager.getWatcher(rootDir) + await waitForEvent(cb => watcher1.onDidStart(cb)) + + expect(watcher0.native).toBe(watcher1.native) + }) + + it('reuses an existing native watcher on a parent directory and filters events', async function () { + const rootDir = await temp.mkdir('atom-fsmanager-').then(fs.realpath) + + const rootFile = path.join(rootDir, 'rootfile.txt') + const subDir = path.join(rootDir, 'subdir') + const subFile = path.join(subDir, 'subfile.txt') + await Promise.all([ + fs.mkdir(subDir).then( + fs.writeFile(subFile, 'subfile\n', {encoding: 'utf8'}) + ), + fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}) + ]) + + const rootWatcher = manager.getWatcher(rootDir) + await waitForEvent(cb => rootWatcher.onDidStart(cb)) + + const childWatcher = manager.getWatcher(subDir) + await waitForEvent(cb => childWatcher.onDidStart(cb)) + + expect(rootWatcher.native).toBe(childWatcher.native) + + const firstRootChange = waitForChanges(rootWatcher, subFile) + const firstChildChange = waitForChanges(childWatcher, subFile) + + console.log(`changing ${subFile}`) + await fs.appendFile(subFile, 'changes\n', {encoding: 'utf8'}) + const firstPayloads = await Promise.all([firstRootChange, firstChildChange]) + + for (const events of firstPayloads) { + expect(events.length).toBe(1) + expect(events[0].oldPath).toBe(subFile) + } + + const nextRootEvent = waitForChanges(rootWatcher, rootFile) + await fs.appendFile(rootFile, 'changes\n', {encoding: 'utf8'}) + + const nextPayload = await nextRootEvent + + expect(nextPayload.length).toBe(1) + expect(nextPayload[0].oldPath).toBe(rootFile) + }) + + xit('adopts existing child watchers and filters events appropriately to them') + + describe('event normalization', function () { + xit('normalizes "changed" events') + xit('normalizes "added" events') + xit('normalizes "deleted" events') + xit('normalizes "renamed" events') + }) + + describe('symlinks', function () { + xit('reports events with symlink paths') + xit('uses the same native watcher even for symlink paths') + }) + }) +}) From 21e381033c65d62428499d2880bdfb8a63c0c1e9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 20 Jun 2017 15:45:55 -0400 Subject: [PATCH 09/94] Start native watchers when attached --- src/filesystem-manager.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 6bdbb9f01..5d4c6408a 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -155,6 +155,8 @@ class Watcher { this.subs.add(native.onDidStart(payload => { this.emitter.emit('did-start', payload) })) + + native.start() } this.subs.add(native.onDidChange(events => { From baf71492a23fc5d74e7a4bb90eccece425d2835a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:21:40 -0400 Subject: [PATCH 10/94] `.dispose()` all subscribers on a NativeWatcher --- src/filesystem-manager.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 5d4c6408a..7cea7f298 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -105,6 +105,11 @@ class NativeWatcher { this.emitter.emit('did-stop') } + // Private: Detach any event subscribers. + dispose () { + this.emitter.dispose() + } + // Private: Callback function invoked by the native watcher when a debounced group of filesystem events arrive. // Normalize and re-broadcast them to any subscribers. // From 8d86acf19cca7dc2713da3fc78b0ea5a74d08b85 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:21:55 -0400 Subject: [PATCH 11/94] Don't report errors after stop --- src/filesystem-manager.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 7cea7f298..8f98650e5 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -130,6 +130,10 @@ class NativeWatcher { // // * `err` The native filesystem error. onError (err) { + if (!this.isRunning()) { + return + } + console.error(err) } } From 9f518736e107adf12f6bcbc27a95d367efef9f74 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:22:13 -0400 Subject: [PATCH 12/94] Track the current NativeWatcher assigned to a Watcher --- src/filesystem-manager.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 8f98650e5..c043dc0b6 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -142,6 +142,7 @@ class Watcher { constructor (watchedPath) { this.watchedPath = watchedPath this.normalizedPath = null + this.native = null this.emitter = new Emitter() this.subs = new CompositeDisposable() @@ -157,6 +158,7 @@ class Watcher { attachToNative (native) { this.subs.dispose() + this.native = native if (native.isRunning()) { this.emitter.emit('did-start') From 12c961c8b9a894382a16422445f87280ba698ec4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:22:26 -0400 Subject: [PATCH 13/94] Maintain a Set of living NativeWatcher instances --- src/filesystem-manager.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index c043dc0b6..4f900f4f3 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -193,6 +193,7 @@ class Watcher { export default class FileSystemManager { constructor () { this.nativeWatchers = new NativeWatcherRegistry() + this.liveWatchers = new Set() } getWatcher (rootPath) { @@ -204,7 +205,17 @@ export default class FileSystemManager { }) watcher.normalizedPath = normalizedPath - this.nativeWatchers.attach(normalizedPath, watcher, () => new NativeWatcher(normalizedPath)) + this.nativeWatchers.attach(normalizedPath, watcher, () => { + const nativeWatcher = new NativeWatcher(normalizedPath) + + this.liveWatchers.add(nativeWatcher) + const sub = nativeWatcher.onDidStop(() => { + this.liveWatchers.delete(nativeWatcher) + sub.dispose() + }) + + return nativeWatcher + }) } init() From be681d132406c71fad1cf31deca16f7459b9bda9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:24:06 -0400 Subject: [PATCH 14/94] Use a private utility function to wait for all native watchers to stop --- spec/filesystem-manager-spec.js | 10 +++------- src/atom-environment.coffee | 2 +- src/filesystem-manager.js | 8 ++++++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index dbc79fa52..1863f9aea 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -6,7 +6,7 @@ import fsCb from 'fs-plus' import path from 'path' import {CompositeDisposable} from 'event-kit' -import FileSystemManager from '../src/filesystem-manager' +import FileSystemManager, {stopAllWatchers} from '../src/filesystem-manager' tempCb.track() @@ -24,12 +24,8 @@ describe('FileSystemManager', function () { afterEach(async function () { subs.dispose() - const cleanup = [temp.cleanup()] - const nativeWatchers = manager.nativeWatchers.tree.leaves().map(node => node.getNativeWatcher()) - for (const native of nativeWatchers) { - cleanup.push(native.stop()) - } - await Promise.all(cleanup) + await stopAllWatchers(manager) + await temp.cleanup() }) function waitForEvent (fn) { diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index 12a1ad192..7deaafbdb 100644 --- a/src/atom-environment.coffee +++ b/src/atom-environment.coffee @@ -46,7 +46,7 @@ TextBuffer = require 'text-buffer' Gutter = require './gutter' TextEditorRegistry = require './text-editor-registry' AutoUpdateManager = require './auto-update-manager' -FileSystemManager = require './filesystem-manager' +FileSystemManager = require('./filesystem-manager').default # Essential: Atom global for dealing with packages, themes, menus, and the window. # diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 4f900f4f3..4e0c93bdb 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -222,3 +222,11 @@ export default class FileSystemManager { return watcher } } + +// Private: Return a Promise that resolves when all {NativeWatcher} instances associated with a FileSystemManager +// have stopped listening. This is useful for `afterEach()` blocks in unit tests. +export function stopAllWatchers (manager) { + return Promise.all( + Array.from(manager.liveWatchers, watcher => watcher.stop()) + ) +} From 882095eea672fc984bfb273290d0124cbfa4a042 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 10:24:26 -0400 Subject: [PATCH 15/94] Test child watcher adoption --- spec/filesystem-manager-spec.js | 49 ++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 1863f9aea..63a19b49e 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -116,7 +116,54 @@ describe('FileSystemManager', function () { expect(nextPayload[0].oldPath).toBe(rootFile) }) - xit('adopts existing child watchers and filters events appropriately to them') + it('adopts existing child watchers and filters events appropriately to them', async function () { + const parentDir = await temp.mkdir('atom-fsmanager-').then(fs.realpath) + + // Create the directory tree + const rootFile = path.join(parentDir, 'rootfile.txt') + const subDir0 = path.join(parentDir, 'subdir0') + const subFile0 = path.join(subDir0, 'subfile1.txt') + const subDir1 = path.join(parentDir, 'subdir1') + const subFile1 = path.join(subDir1, 'subfile1.txt') + await Promise.all([ + fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}), + fs.mkdir(subDir0).then( + fs.writeFile(subFile0, 'subfile 0\n', {encoding: 'utf8'}) + ), + fs.mkdir(subDir1).then( + fs.writeFile(subFile1, 'subfile 1\n', {encoding: 'utf8'}) + ) + ]) + + // Begin the child watchers + const subWatcher0 = manager.getWatcher(subDir0) + const subWatcher1 = manager.getWatcher(subDir1) + + await Promise.all( + [subWatcher0, subWatcher1].map(watcher => waitForEvent(cb => watcher.onDidStart(cb))) + ) + expect(subWatcher0.native).not.toBe(subWatcher1.native) + + // Create the parent watcher + const parentWatcher = manager.getWatcher(parentDir) + await waitForEvent(cb => parentWatcher.onDidStart(cb)) + + expect(subWatcher0.native).toBe(parentWatcher.native) + expect(subWatcher1.native).toBe(parentWatcher.native) + + // Ensure events are filtered correctly + await Promise.all([ + fs.appendFile(rootFile, 'change\n', {encoding: 'utf8'}), + fs.appendFile(subFile0, 'change\n', {encoding: 'utf8'}), + fs.appendFile(subFile1, 'change\n', {encoding: 'utf8'}) + ]) + + await Promise.all([ + waitForChanges(subWatcher0, subFile0), + waitForChanges(subWatcher1, subFile1), + waitForChanges(parentWatcher, rootFile, subFile0, subFile1) + ]) + }) describe('event normalization', function () { xit('normalizes "changed" events') From 0325a77d591f12d56d604fde9b0657a2436dc1ba Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 11:08:29 -0400 Subject: [PATCH 16/94] Test NativeWatcher removal --- spec/native-watcher-registry-spec.js | 45 +++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index e5e880667..4830a421f 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -1,5 +1,8 @@ /** @babel */ +import path from 'path' +import {Emitter} from 'event-kit' + import NativeWatcherRegistry from '../src/native-watcher-registry' class MockWatcher { @@ -19,6 +22,8 @@ class MockNative { this.attached = [] this.disposed = false this.stopped = false + + this.emitter = new Emitter() } reattachTo (newNative) { @@ -29,12 +34,17 @@ class MockNative { this.attached = [] } + onDidStop (callback) { + return this.emitter.on('did-stop', callback) + } + dispose () { this.disposed = true } stop () { this.stopped = true + this.emitter.emit('did-stop') } } @@ -107,5 +117,38 @@ describe('NativeWatcherRegistry', function () { expect(EXISTING2.disposed).toBe(false) }) - it('removes NativeWatchers when all Watchers have been disposed') + describe('removing NativeWatchers', function () { + it('happens when they stop', function () { + const STOPPED = new MockNative('stopped') + const stoppedWatcher = new MockWatcher() + const stoppedPath = ['watcher', 'that', 'will', 'be', 'stopped'] + registry.attach(path.join(...stoppedPath), stoppedWatcher, () => STOPPED) + + const RUNNING = new MockNative('running') + const runningWatcher = new MockWatcher() + const runningPath = ['watcher', 'that', 'will', 'continue', 'to', 'exist'] + registry.attach(path.join(...runningPath), runningWatcher, () => RUNNING) + + STOPPED.stop() + + const runningNode = registry.tree.lookup(runningPath).when({ + parent: node => node, + missing: () => false, + children: () => false + }) + expect(runningNode).toBeTruthy() + expect(runningNode.getNativeWatcher()).toBe(RUNNING) + + const stoppedNode = registry.tree.lookup(stoppedPath).when({ + parent: () => false, + missing: () => true, + children: () => false + }) + expect(stoppedNode).toBe(true) + }) + + it("keeps a parent watcher that's still running") + + it('reassigns new child watchers when a parent watcher is stopped') + }) }) From f3a4c74158cb40bdccb8b52fbb3f189b0c5bdb19 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 11:09:01 -0400 Subject: [PATCH 17/94] :fire: console.log --- spec/filesystem-manager-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 63a19b49e..019c2b0a9 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -98,7 +98,6 @@ describe('FileSystemManager', function () { const firstRootChange = waitForChanges(rootWatcher, subFile) const firstChildChange = waitForChanges(childWatcher, subFile) - console.log(`changing ${subFile}`) await fs.appendFile(subFile, 'changes\n', {encoding: 'utf8'}) const firstPayloads = await Promise.all([firstRootChange, firstChildChange]) From 4194e7f3b5d41a99261dec5406e631f502c4b7d7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 11:33:49 -0400 Subject: [PATCH 18/94] Remove stopped watcher nodes with the power of RECURSION :sparkles: --- src/native-watcher-registry.js | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index c0b7d7c0d..adc86e6f7 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -54,6 +54,37 @@ class RegistryNode { return this } + // Private: Remove a {RegistryWatcherNode} by the exact watched directory. + // + // * `pathSegments` absolute pre-split filesystem path of the node to remove. + // + // Returns: The root of a new tree with the {RegistryWatcherNode} removed. Callers should replace their node + // references with the returned value. + remove (pathSegments) { + if (pathSegments.length === 0) { + // Attempt to remove a path with child watchers. Do nothing. + return this + } + + const pathKey = pathSegments[0] + const child = this.children[pathKey] + if (child === undefined) { + // Attempt to remove a path that isn't watched. Do nothing. + return this + } + + // Recurse + const newChild = child.remove(pathSegments.slice(1)) + if (newChild === null) { + delete this.children[pathKey] + } else { + this.children[pathKey] = newChild + } + + // Remove this node if all of its children have been removed + return Object.keys(this.children).length === 0 ? null : this + } + // Private: Discover all {RegistryWatcherNode} instances beneath this tree node. // // Returns: A possibly empty {Array} of {RegistryWatcherNode} instances that are the descendants of this node. @@ -91,6 +122,15 @@ class RegistryWatcherNode { return new ParentResult(this, pathSegments) } + // Private: Remove this leaf node if the watcher's exact path matches. + // + // * `pathSegments` filesystem path of the node to remove. + // + // Returns: {null} if the `pathSegments` are an exact match, {this} otherwise. + remove (pathSegments) { + return pathSegments.length === 0 ? null : this + } + // Private: Discover this {RegistryWatcherNode} instance. // // Returns: An {Array} containing this node. @@ -206,6 +246,11 @@ export default class NativeWatcherRegistry { const leaf = new RegistryWatcherNode(native) this.tree = this.tree.insert(pathSegments, leaf) + const sub = native.onDidStop(() => { + this.tree = this.tree.remove(pathSegments) + sub.dispose() + }) + watcher.attachToNative(native, '') return native From c8882ca92b639be77cdacc5a228d61d8af233622 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 21 Jun 2017 11:42:50 -0400 Subject: [PATCH 19/94] Start and stop NativeWatchers automatically using `onDidChange` subs --- src/filesystem-manager.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 4e0c93bdb..5f3a50bae 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -3,7 +3,7 @@ import fs from 'fs' import path from 'path' -import {Emitter, CompositeDisposable} from 'event-kit' +import {Emitter, Disposable, CompositeDisposable} from 'event-kit' import nsfw from 'nsfw' import NativeWatcherRegistry from './native-watcher-registry' @@ -65,11 +65,23 @@ class NativeWatcher { return this.emitter.on('did-start', callback) } - // Private: Register a callback to be invoked with normalized filesystem events as they arrive. + // Private: Register a callback to be invoked with normalized filesystem events as they arrive. Starts the watcher + // automatically if it is not already running. The watcher will be stopped automatically when all subscribers + // dispose their subscriptions. // // Returns: A {Disposable} to revoke the subscription. onDidChange (callback) { - return this.emitter.on('did-change', callback) + if (!this.isRunning()) { + this.start() + } + + const sub = this.emitter.on('did-change', callback) + return new Disposable(() => { + sub.dispose() + if (this.emitter.listenerCountForEventName('did-change') === 0) { + this.stop() + } + }) } // Private: Register a callback to be invoked when a {Watcher} should attach to a different {NativeWatcher}. From d858e37058dcbd302edd077549db612107078d4d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:32:54 -0400 Subject: [PATCH 20/94] Support pending specs with an empty body --- spec/async-spec-helpers.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/async-spec-helpers.js b/spec/async-spec-helpers.js index 84a592c6a..e3cf26fa7 100644 --- a/spec/async-spec-helpers.js +++ b/spec/async-spec-helpers.js @@ -20,6 +20,11 @@ export function afterEach (fn) { ['it', 'fit', 'ffit', 'fffit'].forEach(function (name) { module.exports[name] = function (description, fn) { + if (fn === undefined) { + global[name](description) + return + } + global[name](description, function () { const result = fn() if (result instanceof Promise) { From 246e87b660bec50d18e8e51cbef03483e0ef69a8 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:33:13 -0400 Subject: [PATCH 21/94] :shirt: keep standard happy --- spec/async-spec-helpers.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/async-spec-helpers.js b/spec/async-spec-helpers.js index e3cf26fa7..56550cd9f 100644 --- a/spec/async-spec-helpers.js +++ b/spec/async-spec-helpers.js @@ -34,7 +34,7 @@ export function afterEach (fn) { } }) -export async function conditionPromise (condition) { +export async function conditionPromise (condition) { const startTime = Date.now() while (true) { @@ -45,7 +45,7 @@ export async function conditionPromise (condition) { } if (Date.now() - startTime > 5000) { - throw new Error("Timed out waiting on condition") + throw new Error('Timed out waiting on condition') } } } @@ -96,7 +96,7 @@ export function promisify (original) { export function promisifySome (obj, fnNames) { const result = {} - for (fnName of fnNames) { + for (const fnName of fnNames) { result[fnName] = promisify(obj[fnName]) } return result From 53dcc00bfcda6787d0fc6ac280b91f376020f91b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:33:40 -0400 Subject: [PATCH 22/94] Don't cleanup temp between runs to prevent reused directory names --- spec/filesystem-manager-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 019c2b0a9..55024c305 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -25,7 +25,6 @@ describe('FileSystemManager', function () { subs.dispose() await stopAllWatchers(manager) - await temp.cleanup() }) function waitForEvent (fn) { From 6e6c0a5ef9d333faf4e72fd28120c45171e0dbb1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:35:06 -0400 Subject: [PATCH 23/94] Use getStartPromise() in specs --- spec/filesystem-manager-spec.js | 75 ++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 55024c305..a37aee108 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -27,12 +27,6 @@ describe('FileSystemManager', function () { await stopAllWatchers(manager) }) - function waitForEvent (fn) { - return new Promise(resolve => { - subs.add(fn(resolve)) - }) - } - function waitForChanges (watcher, ...fileNames) { const waiting = new Set(fileNames) const relevantEvents = [] @@ -54,21 +48,53 @@ describe('FileSystemManager', function () { } describe('getWatcher()', function () { - it('broadcasts onDidStart when the watcher begins listening', async function () { + it('resolves getStartPromise() when the watcher begins listening', async function () { const rootDir = await temp.mkdir('atom-fsmanager-') const watcher = manager.getWatcher(rootDir) - await waitForEvent(cb => watcher.onDidStart(cb)) + watcher.onDidChange(() => {}) + + await watcher.getStartPromise() }) - it('reuses an existing native watcher and broadcasts onDidStart immediately if attached to an existing watcher', async function () { + it('does not start actually watching until an onDidChange subscriber is registered', async function () { + const rootDir = await temp.mkdir('atom-fsmanager-') + const watcher = manager.getWatcher(rootDir) + + let started = false + const startPromise = watcher.getStartPromise().then(() => { + started = true + }) + + expect(watcher.native).toBe(null) + expect(watcher.normalizedPath).toBe(null) + expect(started).toBe(false) + + await watcher.getNormalizedPathPromise() + + expect(watcher.native).toBe(null) + expect(watcher.normalizedPath).not.toBe(null) + expect(started).toBe(false) + + watcher.onDidChange(() => {}) + await startPromise + + expect(watcher.native).not.toBe(null) + expect(started).toBe(true) + }) + + it('automatically stops and removes the watcher when all onDidChange subscribers dispose') + + it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { const rootDir = await temp.mkdir('atom-fsmanager-') const watcher0 = manager.getWatcher(rootDir) - await waitForEvent(cb => watcher0.onDidStart(cb)) + watcher0.onDidChange(() => {}) + await watcher0.getStartPromise() const watcher1 = manager.getWatcher(rootDir) - await waitForEvent(cb => watcher1.onDidStart(cb)) + watcher1.onDidChange(() => {}) + await watcher1.getStartPromise() expect(watcher0.native).toBe(watcher1.native) }) @@ -87,17 +113,20 @@ describe('FileSystemManager', function () { ]) const rootWatcher = manager.getWatcher(rootDir) - await waitForEvent(cb => rootWatcher.onDidStart(cb)) - const childWatcher = manager.getWatcher(subDir) - await waitForEvent(cb => childWatcher.onDidStart(cb)) expect(rootWatcher.native).toBe(childWatcher.native) const firstRootChange = waitForChanges(rootWatcher, subFile) const firstChildChange = waitForChanges(childWatcher, subFile) + await Promise.all([ + rootWatcher.getStartPromise(), + childWatcher.getStartPromise() + ]) + await fs.appendFile(subFile, 'changes\n', {encoding: 'utf8'}) + const firstPayloads = await Promise.all([firstRootChange, firstChildChange]) for (const events of firstPayloads) { @@ -123,6 +152,7 @@ describe('FileSystemManager', function () { const subFile0 = path.join(subDir0, 'subfile1.txt') const subDir1 = path.join(parentDir, 'subdir1') const subFile1 = path.join(subDir1, 'subfile1.txt') + await Promise.all([ fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}), fs.mkdir(subDir0).then( @@ -135,16 +165,23 @@ describe('FileSystemManager', function () { // Begin the child watchers const subWatcher0 = manager.getWatcher(subDir0) + const subWatcherChanges0 = waitForChanges(subWatcher0, subFile0) + const subWatcher1 = manager.getWatcher(subDir1) + const subWatcherChanges1 = waitForChanges(subWatcher1, subFile1) await Promise.all( - [subWatcher0, subWatcher1].map(watcher => waitForEvent(cb => watcher.onDidStart(cb))) + [subWatcher0, subWatcher1].map(watcher => { + return watcher.getStartPromise() + }) ) expect(subWatcher0.native).not.toBe(subWatcher1.native) // Create the parent watcher const parentWatcher = manager.getWatcher(parentDir) - await waitForEvent(cb => parentWatcher.onDidStart(cb)) + const parentWatcherChanges = waitForChanges(parentWatcher, rootFile, subFile0, subFile1) + + await parentWatcher.getStartPromise() expect(subWatcher0.native).toBe(parentWatcher.native) expect(subWatcher1.native).toBe(parentWatcher.native) @@ -157,9 +194,9 @@ describe('FileSystemManager', function () { ]) await Promise.all([ - waitForChanges(subWatcher0, subFile0), - waitForChanges(subWatcher1, subFile1), - waitForChanges(parentWatcher, rootFile, subFile0, subFile1) + subWatcherChanges0, + subWatcherChanges1, + parentWatcherChanges ]) }) From 9c8ed35b26c3ccc78fadd0ede991d4d16e0c5657 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:36:29 -0400 Subject: [PATCH 24/94] Provide native watcher creation function to the NativeWatcherRegistry constructor --- spec/native-watcher-registry-spec.js | 153 +++++++++++++++++++-------- src/native-watcher-registry.js | 18 ++-- 2 files changed, 119 insertions(+), 52 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 4830a421f..d76d91547 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -1,15 +1,22 @@ /** @babel */ +import {it, beforeEach} from './async-spec-helpers' + import path from 'path' import {Emitter} from 'event-kit' import NativeWatcherRegistry from '../src/native-watcher-registry' class MockWatcher { - constructor () { + constructor (normalizedPath) { + this.normalizedPath = normalizedPath this.native = null } + getNormalizedPathPromise () { + return Promise.resolve(this.normalizedPath) + } + attachToNative (native) { this.native = native this.native.attached.push(this) @@ -49,85 +56,143 @@ class MockNative { } describe('NativeWatcherRegistry', function () { - let registry, watcher + let createNative, registry beforeEach(function () { - registry = new NativeWatcherRegistry() - watcher = new MockWatcher() + registry = new NativeWatcherRegistry(normalizedPath => createNative(normalizedPath)) }) - it('attaches a Watcher to a newly created NativeWatcher for a new directory', function () { + it('attaches a Watcher to a newly created NativeWatcher for a new directory', async function () { + const watcher = new MockWatcher(path.join('some', 'path')) const NATIVE = new MockNative('created') - registry.attach('/some/path', watcher, () => NATIVE) + createNative = () => NATIVE + + await registry.attach(watcher) expect(watcher.native).toBe(NATIVE) }) - it('reuses an existing NativeWatcher on the same directory', function () { + it('reuses an existing NativeWatcher on the same directory', async function () { const EXISTING = new MockNative('existing') - registry.attach('/existing/path', new MockWatcher(), () => EXISTING) + const existingPath = path.join('existing', 'path') + let firstTime = true + createNative = () => { + if (firstTime) { + firstTime = false + return EXISTING + } - registry.attach('/existing/path', watcher, () => new MockNative('no')) + return new MockNative('nope') + } + await registry.attach(new MockWatcher(existingPath)) + + const watcher = new MockWatcher(existingPath) + await registry.attach(watcher) expect(watcher.native).toBe(EXISTING) }) - it('attaches to an existing NativeWatcher on a parent directory', function () { + it('attaches to an existing NativeWatcher on a parent directory', async function () { const EXISTING = new MockNative('existing') - registry.attach('/existing/path', new MockWatcher(), () => EXISTING) + const parentDir = path.join('existing', 'path') + const subDir = path.join(parentDir, 'sub', 'directory') + let firstTime = true + createNative = () => { + if (firstTime) { + firstTime = false + return EXISTING + } - registry.attach('/existing/path/sub/directory/', watcher, () => new MockNative('no')) + return new MockNative('nope') + } + await registry.attach(new MockWatcher(parentDir)) + + const watcher = new MockWatcher(subDir) + await registry.attach(watcher) expect(watcher.native).toBe(EXISTING) }) - it('adopts Watchers from NativeWatchers on child directories', function () { - const EXISTING0 = new MockNative('existing0') - const watcher0 = new MockWatcher() - registry.attach('/existing/path/child/directory/zero', watcher0, () => EXISTING0) + it('adopts Watchers from NativeWatchers on child directories', async function () { + const parentDir = path.join('existing', 'path') + const childDir0 = path.join(parentDir, 'child', 'directory', 'zero') + const childDir1 = path.join(parentDir, 'child', 'directory', 'one') + const otherDir = path.join('another', 'path') - const EXISTING1 = new MockNative('existing1') - const watcher1 = new MockWatcher() - registry.attach('/existing/path/child/directory/one', watcher1, () => EXISTING1) + const CHILD0 = new MockNative('existing0') + const CHILD1 = new MockNative('existing1') + const OTHER = new MockNative('existing2') + const PARENT = new MockNative('parent') - const EXISTING2 = new MockNative('existing2') - const watcher2 = new MockWatcher() - registry.attach('/another/path', watcher2, () => EXISTING2) + createNative = dir => { + if (dir === childDir0) { + return CHILD0 + } else if (dir === childDir1) { + return CHILD1 + } else if (dir === otherDir) { + return OTHER + } else if (dir === parentDir) { + return PARENT + } else { + throw new Error(`Unexpected path: ${dir}`) + } + } - expect(watcher0.native).toBe(EXISTING0) - expect(watcher1.native).toBe(EXISTING1) - expect(watcher2.native).toBe(EXISTING2) + const watcher0 = new MockWatcher(childDir0) + await registry.attach(watcher0) + + const watcher1 = new MockWatcher(childDir1) + await registry.attach(watcher1) + + const watcher2 = new MockWatcher(otherDir) + await registry.attach(watcher2) + + expect(watcher0.native).toBe(CHILD0) + expect(watcher1.native).toBe(CHILD1) + expect(watcher2.native).toBe(OTHER) // Consolidate all three watchers beneath the same native watcher on the parent directory - const CREATED = new MockNative('created') - registry.attach('/existing/path/', watcher, () => CREATED) + const watcher = new MockWatcher(parentDir) + await registry.attach(watcher) - expect(watcher.native).toBe(CREATED) + expect(watcher.native).toBe(PARENT) - expect(watcher0.native).toBe(CREATED) - expect(EXISTING0.stopped).toBe(true) - expect(EXISTING0.disposed).toBe(true) + expect(watcher0.native).toBe(PARENT) + expect(CHILD0.stopped).toBe(true) + expect(CHILD0.disposed).toBe(true) - expect(watcher1.native).toBe(CREATED) - expect(EXISTING1.stopped).toBe(true) - expect(EXISTING1.disposed).toBe(true) + expect(watcher1.native).toBe(PARENT) + expect(CHILD1.stopped).toBe(true) + expect(CHILD1.disposed).toBe(true) - expect(watcher2.native).toBe(EXISTING2) - expect(EXISTING2.stopped).toBe(false) - expect(EXISTING2.disposed).toBe(false) + expect(watcher2.native).toBe(OTHER) + expect(OTHER.stopped).toBe(false) + expect(OTHER.disposed).toBe(false) }) describe('removing NativeWatchers', function () { - it('happens when they stop', function () { + it('happens when they stop', async function () { const STOPPED = new MockNative('stopped') - const stoppedWatcher = new MockWatcher() - const stoppedPath = ['watcher', 'that', 'will', 'be', 'stopped'] - registry.attach(path.join(...stoppedPath), stoppedWatcher, () => STOPPED) - const RUNNING = new MockNative('running') - const runningWatcher = new MockWatcher() + + const stoppedPath = ['watcher', 'that', 'will', 'be', 'stopped'] const runningPath = ['watcher', 'that', 'will', 'continue', 'to', 'exist'] - registry.attach(path.join(...runningPath), runningWatcher, () => RUNNING) + + createNative = dir => { + if (dir === path.join(...stoppedPath)) { + return STOPPED + } else if (dir === path.join(...runningPath)) { + return RUNNING + } else { + throw new Error(`Unexpected path: ${dir}`) + } + } + + const stoppedWatcher = new MockWatcher(path.join(...stoppedPath)) + await registry.attach(stoppedWatcher) + + const runningWatcher = new MockWatcher(path.join(...runningPath)) + await registry.attach(runningWatcher) STOPPED.stop() diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index adc86e6f7..169201310 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -221,8 +221,12 @@ class ChildrenResult { export default class NativeWatcherRegistry { // Private: Instantiate an empty registry. - constructor () { + // + // * `createNative` {Function} that will be called with a normalized filesystem path to create a new native + // filesystem watcher. + constructor (createNative) { this.tree = new RegistryNode() + this.createNative = createNative } // Private: Attach a watcher to a directory, assigning it a {NativeWatcher}. If a suitable {NativeWatcher} already @@ -234,15 +238,13 @@ export default class NativeWatcherRegistry { // be broadcast on each with the new parent watcher as an event payload to give child watchers a chance to attach to // the new watcher. // - // * `directory` a normalized path to be watched as a {String}. // * `watcher` an unattached {Watcher}. - // * `createNative` callback to be invoked if no existing {NativeWatcher} covers the {Watcher}. It should - // synchronously return a new {NativeWatcher} instance watching {directory}. - attach (directory, watcher, createNative) { - const pathSegments = directory.split(path.sep).filter(segment => segment.length > 0) + async attach (watcher) { + const normalizedDirectory = await watcher.getNormalizedPathPromise() + const pathSegments = normalizedDirectory.split(path.sep).filter(segment => segment.length > 0) const attachToNew = () => { - const native = createNative() + const native = this.createNative(normalizedDirectory) const leaf = new RegistryWatcherNode(native) this.tree = this.tree.insert(pathSegments, leaf) @@ -272,7 +274,7 @@ export default class NativeWatcherRegistry { for (let i = 0; i < children.length; i++) { const child = children[i] const childNative = child.getNativeWatcher() - childNative.reattachTo(newNative, directory) + childNative.reattachTo(newNative, normalizedDirectory) childNative.dispose() // Don't await this Promise. Subscribers can listen for `onDidStop` to be notified if they choose. From 80a9126fdba0a51ca5a54b172906d8aeb19d6c80 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:37:46 -0400 Subject: [PATCH 25/94] Start NativeWatchers lazily and stop them opportunistically --- src/filesystem-manager.js | 109 +++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 5f3a50bae..8da8bdab0 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -151,21 +151,60 @@ class NativeWatcher { } class Watcher { - constructor (watchedPath) { + constructor (watchedPath, nativeWatcherRegistry) { this.watchedPath = watchedPath + this.nativeWatcherRegistry = nativeWatcherRegistry + this.normalizedPath = null this.native = null + this.changeCallbacks = new Map() + + this.normalizedPathPromise = new Promise((resolve, reject) => { + fs.realpath(watchedPath, (err, real) => { + if (err) { + reject(err) + return + } + + this.normalizedPath = real + resolve(real) + }) + }) + + this.startPromise = new Promise(resolve => { + this.resolveStartPromise = resolve + }) this.emitter = new Emitter() this.subs = new CompositeDisposable() } - onDidStart (callback) { - return this.emitter.on('did-start', callback) + getNormalizedPathPromise () { + return this.normalizedPathPromise + } + + getStartPromise () { + return this.startPromise } onDidChange (callback) { - return this.emitter.on('did-change', callback) + if (this.native) { + const sub = this.native.onDidChange(events => this.onNativeEvents(events, callback)) + this.changeCallbacks.set(callback, sub) + + this.native.start() + } else { + // Attach and retry + this.nativeWatcherRegistry.attach(this).then(() => { + this.onDidChange(callback) + }) + } + + return new Disposable(() => { + const sub = this.changeCallbacks.get(callback) + this.changeCallbacks.delete(callback) + sub.dispose() + }) } attachToNative (native) { @@ -173,30 +212,45 @@ class Watcher { this.native = native if (native.isRunning()) { - this.emitter.emit('did-start') + this.resolveStartPromise() } else { - this.subs.add(native.onDidStart(payload => { - this.emitter.emit('did-start', payload) + this.subs.add(native.onDidStart(() => { + this.resolveStartPromise() })) + } + // Transfer any native event subscriptions to the new NativeWatcher. + for (const [callback, formerSub] of this.changeCallbacks) { + const newSub = native.onDidChange(events => this.onNativeEvents(events, callback)) + this.changeCallbacks.set(callback, newSub) + formerSub.dispose() + } + + if (this.changeCallbacks.size > 0) { native.start() } - this.subs.add(native.onDidChange(events => { - // TODO does event.oldPath resolve symlinks? - const filtered = events.filter(event => event.oldPath.startsWith(this.normalizedPath)) - - if (filtered.length > 0) { - this.emitter.emit('did-change', filtered) + this.subs.add(native.onShouldDetach(replacement => { + if (replacement !== native) { + this.attachToNative(replacement) } })) + } - this.subs.add(native.onShouldDetach( - this.attachToNative.bind(this) - )) + onNativeEvents (events, callback) { + // TODO does event.oldPath resolve symlinks? + const filtered = events.filter(event => event.oldPath.startsWith(this.normalizedPath)) + + if (filtered.length > 0) { + callback(filtered) + } } dispose () { + for (const sub of this.changeCallbacks.values()) { + sub.dispose() + } + this.emitter.dispose() this.subs.dispose() } @@ -204,20 +258,10 @@ class Watcher { export default class FileSystemManager { constructor () { - this.nativeWatchers = new NativeWatcherRegistry() this.liveWatchers = new Set() - } - getWatcher (rootPath) { - const watcher = new Watcher(rootPath) - - const init = async () => { - const normalizedPath = await new Promise((resolve, reject) => { - fs.realpath(rootPath, (err, real) => (err ? reject(err) : resolve(real))) - }) - watcher.normalizedPath = normalizedPath - - this.nativeWatchers.attach(normalizedPath, watcher, () => { + this.nativeWatchers = new NativeWatcherRegistry( + normalizedPath => { const nativeWatcher = new NativeWatcher(normalizedPath) this.liveWatchers.add(nativeWatcher) @@ -227,11 +271,12 @@ export default class FileSystemManager { }) return nativeWatcher - }) - } - init() + } + ) + } - return watcher + getWatcher (rootPath) { + return new Watcher(rootPath, this.nativeWatchers) } } From 7ec79a00fc76297dccc767967be178ae1e0e20b6 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 08:39:36 -0400 Subject: [PATCH 26/94] Set running = false before the asynchronous stop operation --- src/filesystem-manager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 8da8bdab0..3096e237a 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -111,9 +111,9 @@ class NativeWatcher { if (!this.running) { return } + this.running = false await this.watcher.stop() - this.running = false this.emitter.emit('did-stop') } From 6d17fc880d2d3f876078a35aee4e2b5f629e50bf Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 09:00:16 -0400 Subject: [PATCH 27/94] Opportunistic native watcher stopping --- spec/filesystem-manager-spec.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index a37aee108..bece1d09e 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -83,7 +83,23 @@ describe('FileSystemManager', function () { expect(started).toBe(true) }) - it('automatically stops and removes the watcher when all onDidChange subscribers dispose') + it('automatically stops and removes the watcher when all onDidChange subscribers dispose', async function () { + const dir = await temp.mkdir('atom-fsmanager-') + const watcher = manager.getWatcher(dir) + + const sub0 = watcher.onDidChange(() => {}) + const sub1 = watcher.onDidChange(() => {}) + + await watcher.getStartPromise() + expect(watcher.native).not.toBe(null) + expect(watcher.native.isRunning()).toBe(true) + + sub0.dispose() + expect(watcher.native.isRunning()).toBe(true) + + sub1.dispose() + expect(watcher.native.isRunning()).toBe(false) + }) it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { const rootDir = await temp.mkdir('atom-fsmanager-') From b34a9d69720e940f4e8be4435a341abdec0b0118 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 12:37:26 -0400 Subject: [PATCH 28/94] Only resolve the waitForChanges promise once --- spec/filesystem-manager-spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index bece1d09e..29e0ca435 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -29,6 +29,7 @@ describe('FileSystemManager', function () { function waitForChanges (watcher, ...fileNames) { const waiting = new Set(fileNames) + let fired = false const relevantEvents = [] return new Promise(resolve => { @@ -39,7 +40,8 @@ describe('FileSystemManager', function () { } } - if (waiting.size === 0) { + if (!fired && waiting.size === 0) { + fired = true resolve(relevantEvents) sub.dispose() } From f75aa1ae030a795c5507c293bbb98274172c07d0 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 12:44:07 -0400 Subject: [PATCH 29/94] Use onWillStop() instead of onDidStop() This will prevent new Watchers from attaching to NativeWatchers that are in the process of stopping. --- spec/filesystem-manager-spec.js | 92 ++++++++++++++++++---------- spec/native-watcher-registry-spec.js | 6 +- src/filesystem-manager.js | 62 ++++++++++++++----- src/native-watcher-registry.js | 4 +- 4 files changed, 110 insertions(+), 54 deletions(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 29e0ca435..380419f7f 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -93,14 +93,17 @@ describe('FileSystemManager', function () { const sub1 = watcher.onDidChange(() => {}) await watcher.getStartPromise() - expect(watcher.native).not.toBe(null) - expect(watcher.native.isRunning()).toBe(true) + const native = watcher.native + expect(native).not.toBe(null) + expect(native.isRunning()).toBe(true) sub0.dispose() - expect(watcher.native.isRunning()).toBe(true) + expect(watcher.native).toBe(native) + expect(native.isRunning()).toBe(true) sub1.dispose() - expect(watcher.native.isRunning()).toBe(false) + expect(watcher.native).toBeNull() + expect(native.isRunning()).toBe(false) }) it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { @@ -117,48 +120,73 @@ describe('FileSystemManager', function () { expect(watcher0.native).toBe(watcher1.native) }) - it('reuses an existing native watcher on a parent directory and filters events', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-').then(fs.realpath) + it("reuses existing native watchers even while they're still starting", async function () { + const rootDir = await temp.mkdir('atom-fsmanager-') + const watcher0 = manager.getWatcher(rootDir) + watcher0.onDidChange(() => {}) + await watcher0.getAttachedPromise() + expect(watcher0.native.isRunning()).toBe(false) + + const watcher1 = manager.getWatcher(rootDir) + watcher1.onDidChange(() => {}) + await watcher1.getAttachedPromise() + + expect(watcher0.native).toBe(watcher1.native) + + await Promise.all([watcher0.getStartPromise(), watcher1.getStartPromise()]) + }) + + it("doesn't attach new watchers to a native watcher that's stopping", async function () { + const rootDir = await temp.mkdir('atom-fsmanager-') + + const watcher0 = manager.getWatcher(rootDir) + const sub = watcher0.onDidChange(() => {}) + await watcher0.getStartPromise() + const native0 = watcher0.native + + sub.dispose() + + const watcher1 = manager.getWatcher(rootDir) + watcher1.onDidChange(() => {}) + + expect(watcher1.native).not.toBe(native0) + }) + + it('reuses an existing native watcher on a parent directory and filters events', async function () { + const rootDir = await temp.mkdir('atom-fsmanager-0-').then(fs.realpath) const rootFile = path.join(rootDir, 'rootfile.txt') const subDir = path.join(rootDir, 'subdir') const subFile = path.join(subDir, 'subfile.txt') - await Promise.all([ - fs.mkdir(subDir).then( - fs.writeFile(subFile, 'subfile\n', {encoding: 'utf8'}) - ), - fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}) - ]) + await fs.mkdir(subDir) + + // Keep the watchers alive with an undisposed subscription const rootWatcher = manager.getWatcher(rootDir) + rootWatcher.onDidChange(() => {}) const childWatcher = manager.getWatcher(subDir) - - expect(rootWatcher.native).toBe(childWatcher.native) - - const firstRootChange = waitForChanges(rootWatcher, subFile) - const firstChildChange = waitForChanges(childWatcher, subFile) + childWatcher.onDidChange(() => {}) await Promise.all([ rootWatcher.getStartPromise(), childWatcher.getStartPromise() ]) - await fs.appendFile(subFile, 'changes\n', {encoding: 'utf8'}) + expect(rootWatcher.native).toBe(childWatcher.native) + expect(rootWatcher.native.isRunning()).toBe(true) - const firstPayloads = await Promise.all([firstRootChange, firstChildChange]) + const firstChanges = Promise.all([ + waitForChanges(rootWatcher, subFile), + waitForChanges(childWatcher, subFile) + ]) - for (const events of firstPayloads) { - expect(events.length).toBe(1) - expect(events[0].oldPath).toBe(subFile) - } + await fs.writeFile(subFile, 'subfile\n', {encoding: 'utf8'}) + await firstChanges const nextRootEvent = waitForChanges(rootWatcher, rootFile) - await fs.appendFile(rootFile, 'changes\n', {encoding: 'utf8'}) + await fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}) - const nextPayload = await nextRootEvent - - expect(nextPayload.length).toBe(1) - expect(nextPayload[0].oldPath).toBe(rootFile) + await nextRootEvent }) it('adopts existing child watchers and filters events appropriately to them', async function () { @@ -181,17 +209,17 @@ describe('FileSystemManager', function () { ) ]) - // Begin the child watchers + // Begin the child watchers and keep them alive const subWatcher0 = manager.getWatcher(subDir0) + subWatcher0.onDidChange(() => {}) const subWatcherChanges0 = waitForChanges(subWatcher0, subFile0) const subWatcher1 = manager.getWatcher(subDir1) + subWatcher1.onDidChange(() => {}) const subWatcherChanges1 = waitForChanges(subWatcher1, subFile1) await Promise.all( - [subWatcher0, subWatcher1].map(watcher => { - return watcher.getStartPromise() - }) + [subWatcher0, subWatcher1].map(watcher => watcher.getStartPromise()) ) expect(subWatcher0.native).not.toBe(subWatcher1.native) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index d76d91547..206aefc76 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -41,8 +41,8 @@ class MockNative { this.attached = [] } - onDidStop (callback) { - return this.emitter.on('did-stop', callback) + onWillStop (callback) { + return this.emitter.on('will-stop', callback) } dispose () { @@ -51,7 +51,7 @@ class MockNative { stop () { this.stopped = true - this.emitter.emit('did-stop') + this.emitter.emit('will-stop') } } diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 3096e237a..277c7cbfe 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -16,6 +16,14 @@ const ACTION_MAP = new Map([ [nsfw.actions.RENAMED, 'renamed'] ]) +// Private: Possible states of a {NativeWatcher}. +export const WATCHER_STATE = { + STOPPED: Symbol('stopped'), + STARTING: Symbol('starting'), + RUNNING: Symbol('running'), + STOPPING: Symbol('stopping') +} + // Private: Interface with and normalize events from a native OS filesystem watcher. class NativeWatcher { @@ -27,16 +35,17 @@ class NativeWatcher { this.emitter = new Emitter() this.watcher = null - this.running = false + this.state = WATCHER_STATE.STOPPED } // Private: Begin watching for filesystem events. // // Has no effect if the watcher has already been started. async start () { - if (this.running) { + if (this.state !== WATCHER_STATE.STOPPED) { return } + this.state = WATCHER_STATE.STARTING this.watcher = await nsfw( this.normalizedPath, @@ -49,13 +58,14 @@ class NativeWatcher { await this.watcher.start() - this.running = true + + this.state = WATCHER_STATE.RUNNING this.emitter.emit('did-start') } - // Private: Return true if the underlying watcher has been started. + // Private: Return true if the underlying watcher is actively listening for filesystem events. isRunning () { - return this.running + return this.state === WATCHER_STATE.RUNNING } // Private: Register a callback to be invoked when the filesystem watcher has been initialized. @@ -71,9 +81,7 @@ class NativeWatcher { // // Returns: A {Disposable} to revoke the subscription. onDidChange (callback) { - if (!this.isRunning()) { - this.start() - } + this.start() const sub = this.emitter.on('did-change', callback) return new Disposable(() => { @@ -91,7 +99,14 @@ class NativeWatcher { return this.emitter.on('should-detach', callback) } - // Private: Register a callback to be invoked when the filesystem watcher has been initialized. + // Private: Register a callback to be invoked when a {NativeWatcher} is about to be stopped. + // + // Returns: A {Disposable} to revoke the subscription. + onWillStop (callback) { + return this.emitter.on('will-stop', callback) + } + + // Private: Register a callback to be invoked when the filesystem watcher has been stopped. // // Returns: A {Disposable} to revoke the subscription. onDidStop (callback) { @@ -108,12 +123,15 @@ class NativeWatcher { // // Has no effect if the watcher is not running. async stop () { - if (!this.running) { + if (this.state !== WATCHER_STATE.RUNNING) { return } - this.running = false + this.state = WATCHER_STATE.STOPPING + this.emitter.emit('will-stop') await this.watcher.stop() + this.state = WATCHER_STATE.STOPPED + this.emitter.emit('did-stop') } @@ -171,6 +189,9 @@ class Watcher { }) }) + this.attachedPromise = new Promise(resolve => { + this.resolveAttachedPromise = resolve + }) this.startPromise = new Promise(resolve => { this.resolveStartPromise = resolve }) @@ -183,6 +204,10 @@ class Watcher { return this.normalizedPathPromise } + getAttachedPromise () { + return this.attachedPromise + } + getStartPromise () { return this.startPromise } @@ -194,7 +219,7 @@ class Watcher { this.native.start() } else { - // Attach and retry + // Attach to a new native listener and retry this.nativeWatcherRegistry.attach(this).then(() => { this.onDidChange(callback) }) @@ -226,15 +251,18 @@ class Watcher { formerSub.dispose() } - if (this.changeCallbacks.size > 0) { - native.start() - } - this.subs.add(native.onShouldDetach(replacement => { if (replacement !== native) { this.attachToNative(replacement) } })) + + this.subs.add(native.onWillStop(() => { + this.subs.dispose() + this.native = null + })) + + this.resolveAttachedPromise() } onNativeEvents (events, callback) { @@ -265,7 +293,7 @@ export default class FileSystemManager { const nativeWatcher = new NativeWatcher(normalizedPath) this.liveWatchers.add(nativeWatcher) - const sub = nativeWatcher.onDidStop(() => { + const sub = nativeWatcher.onWillStop(() => { this.liveWatchers.delete(nativeWatcher) sub.dispose() }) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 169201310..95a5a6483 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -248,8 +248,8 @@ export default class NativeWatcherRegistry { const leaf = new RegistryWatcherNode(native) this.tree = this.tree.insert(pathSegments, leaf) - const sub = native.onDidStop(() => { - this.tree = this.tree.remove(pathSegments) + const sub = native.onWillStop(() => { + this.tree = this.tree.remove(pathSegments) || new RegistryNode() sub.dispose() }) From c2810b626c7407caee775f765286357ea227f12d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 13:39:47 -0400 Subject: [PATCH 30/94] Propagate errors to subscribers with an onDidError callback --- src/filesystem-manager.js | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 277c7cbfe..47ec98af4 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -113,6 +113,13 @@ class NativeWatcher { return this.emitter.on('did-stop', callback) } + // Private: Register a callback to be invoked with any errors reported from the watcher. + // + // Returns: A {Disposable} to revoke the subscription. + onDidError (callback) { + return this.emitter.on('did-error', callback) + } + // Private: Broadcast an `onShouldDetach` event to prompt any {Watcher} instances bound here to attach to a new // {NativeWatcher} instead. reattachTo (other) { @@ -160,11 +167,7 @@ class NativeWatcher { // // * `err` The native filesystem error. onError (err) { - if (!this.isRunning()) { - return - } - - console.error(err) + this.emitter.emit('did-error', err) } } @@ -232,6 +235,10 @@ class Watcher { }) } + onDidError (callback) { + return this.emitter.on('did-error', callback) + } + attachToNative (native) { this.subs.dispose() this.native = native @@ -251,6 +258,10 @@ class Watcher { formerSub.dispose() } + this.subs.add(native.onDidError(err => { + this.emitter.emit('did-error', err) + })) + this.subs.add(native.onShouldDetach(replacement => { if (replacement !== native) { this.attachToNative(replacement) From 60e6da9097156c43589002d42cb3927f1b2067ec Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 22 Jun 2017 14:04:29 -0400 Subject: [PATCH 31/94] Weird-ass concurrent mkdir() error --- spec/filesystem-manager-spec.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/spec/filesystem-manager-spec.js b/spec/filesystem-manager-spec.js index 380419f7f..fda6c32f8 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/filesystem-manager-spec.js @@ -11,7 +11,7 @@ import FileSystemManager, {stopAllWatchers} from '../src/filesystem-manager' tempCb.track() const fs = promisifySome(fsCb, ['writeFile', 'mkdir', 'symlink', 'appendFile', 'realpath']) -const temp = promisifySome(tempCb, ['mkdir', 'cleanup']) +const temp = promisifySome(tempCb, ['mkdir']) describe('FileSystemManager', function () { let subs, manager @@ -51,7 +51,7 @@ describe('FileSystemManager', function () { describe('getWatcher()', function () { it('resolves getStartPromise() when the watcher begins listening', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-') + const rootDir = await temp.mkdir('atom-fsmanager-test-') const watcher = manager.getWatcher(rootDir) watcher.onDidChange(() => {}) @@ -60,7 +60,7 @@ describe('FileSystemManager', function () { }) it('does not start actually watching until an onDidChange subscriber is registered', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-') + const rootDir = await temp.mkdir('atom-fsmanager-test-') const watcher = manager.getWatcher(rootDir) let started = false @@ -86,7 +86,7 @@ describe('FileSystemManager', function () { }) it('automatically stops and removes the watcher when all onDidChange subscribers dispose', async function () { - const dir = await temp.mkdir('atom-fsmanager-') + const dir = await temp.mkdir('atom-fsmanager-test-') const watcher = manager.getWatcher(dir) const sub0 = watcher.onDidChange(() => {}) @@ -107,7 +107,7 @@ describe('FileSystemManager', function () { }) it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-') + const rootDir = await temp.mkdir('atom-fsmanager-test-') const watcher0 = manager.getWatcher(rootDir) watcher0.onDidChange(() => {}) @@ -121,7 +121,7 @@ describe('FileSystemManager', function () { }) it("reuses existing native watchers even while they're still starting", async function () { - const rootDir = await temp.mkdir('atom-fsmanager-') + const rootDir = await temp.mkdir('atom-fsmanager-test-') const watcher0 = manager.getWatcher(rootDir) watcher0.onDidChange(() => {}) @@ -138,7 +138,7 @@ describe('FileSystemManager', function () { }) it("doesn't attach new watchers to a native watcher that's stopping", async function () { - const rootDir = await temp.mkdir('atom-fsmanager-') + const rootDir = await temp.mkdir('atom-fsmanager-test-') const watcher0 = manager.getWatcher(rootDir) const sub = watcher0.onDidChange(() => {}) @@ -154,7 +154,7 @@ describe('FileSystemManager', function () { }) it('reuses an existing native watcher on a parent directory and filters events', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-0-').then(fs.realpath) + const rootDir = await temp.mkdir('atom-fsmanager-test-').then(fs.realpath) const rootFile = path.join(rootDir, 'rootfile.txt') const subDir = path.join(rootDir, 'subdir') const subFile = path.join(subDir, 'subfile.txt') @@ -190,23 +190,21 @@ describe('FileSystemManager', function () { }) it('adopts existing child watchers and filters events appropriately to them', async function () { - const parentDir = await temp.mkdir('atom-fsmanager-').then(fs.realpath) + const parentDir = await temp.mkdir('atom-fsmanager-test-').then(fs.realpath) // Create the directory tree const rootFile = path.join(parentDir, 'rootfile.txt') const subDir0 = path.join(parentDir, 'subdir0') - const subFile0 = path.join(subDir0, 'subfile1.txt') + const subFile0 = path.join(subDir0, 'subfile0.txt') const subDir1 = path.join(parentDir, 'subdir1') const subFile1 = path.join(subDir1, 'subfile1.txt') + await fs.mkdir(subDir0) + await fs.mkdir(subDir1) await Promise.all([ fs.writeFile(rootFile, 'rootfile\n', {encoding: 'utf8'}), - fs.mkdir(subDir0).then( - fs.writeFile(subFile0, 'subfile 0\n', {encoding: 'utf8'}) - ), - fs.mkdir(subDir1).then( - fs.writeFile(subFile1, 'subfile 1\n', {encoding: 'utf8'}) - ) + fs.writeFile(subFile0, 'subfile 0\n', {encoding: 'utf8'}), + fs.writeFile(subFile1, 'subfile 1\n', {encoding: 'utf8'}) ]) // Begin the child watchers and keep them alive From a5f217fd516d530dfcb2e3c0cb7ca8e7686855b9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 23 Jun 2017 09:33:47 -0400 Subject: [PATCH 32/94] WIP work on rewatching child directories --- spec/native-watcher-registry-spec.js | 62 +++++++++++++++++++++++++++- src/native-watcher-registry.js | 35 ++++++++++------ 2 files changed, 82 insertions(+), 15 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 206aefc76..107e0efd3 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -212,8 +212,66 @@ describe('NativeWatcherRegistry', function () { expect(stoppedNode).toBe(true) }) - it("keeps a parent watcher that's still running") + it('reassigns new child watchers when a parent watcher is stopped', async function () { + const CHILD0 = new MockNative('child0') + const CHILD1 = new MockNative('child1') + const PARENT = new MockNative('parent') - it('reassigns new child watchers when a parent watcher is stopped') + const parentDir = path.join('parent') + const childDir0 = path.join(parentDir, 'child0') + const childDir1 = path.join(parentDir, 'child1') + + createNative = dir => { + if (dir === parentDir) { + return PARENT + } else if (dir === childDir0) { + return CHILD0 + } else if (dir === childDir1) { + return CHILD1 + } else { + throw new Error(`Unexpected directory ${dir}`) + } + } + + const parentWatcher = new MockWatcher(parentDir) + const childWatcher0 = new MockWatcher(childDir0) + const childWatcher1 = new MockWatcher(childDir1) + + await registry.attach(parentWatcher) + await Promise.all([ + registry.attach(childWatcher0), + registry.attach(childWatcher1) + ]) + + // All three watchers should share the parent watcher's native watcher. + expect(parentWatcher.native).toBe(PARENT) + expect(childWatcher0.native).toBe(PARENT) + expect(childWatcher1.native).toBe(PARENT) + + // Stopping the parent should detach and recreate the child watchers. + // (Here, they'll be the same watcher instances used before, because of the fake createNative implementation.) + PARENT.stop() + + expect(childWatcher0.native).toBe(CHILD0) + expect(childWatcher1.native).toBe(CHILD1) + + expect(registry.tree.lookup(['parent']).when({ + parent: () => false, + missing: () => false, + children: () => true + })).toBe(true) + + expect(registry.tree.lookup(['parent', 'child0']).when({ + parent: () => true, + missing: () => false, + children: () => false + })).toBe(true) + + expect(registry.tree.lookup(['parent', 'child1']).when({ + parent: () => true, + missing: () => false, + children: () => false + })).toBe(true) + }) }) }) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 95a5a6483..3d83bbf82 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -21,7 +21,7 @@ class RegistryNode { // exist. lookup (pathSegments) { if (pathSegments.length === 0) { - return new ChildrenResult(this.leaves()) + return new ChildrenResult(this.leaves([])) } const child = this.children[pathSegments[0]] @@ -85,13 +85,17 @@ class RegistryNode { return Object.keys(this.children).length === 0 ? null : this } - // Private: Discover all {RegistryWatcherNode} instances beneath this tree node. + // Private: Discover all {RegistryWatcherNode} instances beneath this tree node and the child paths + // that they are watching. // - // Returns: A possibly empty {Array} of {RegistryWatcherNode} instances that are the descendants of this node. - leaves () { + // * `prefix` {Array} of intermediate path segments to prepend to the resulting child paths. + // + // Returns: A possibly empty {Array} of `{node, path}` objects describing {RegistryWatcherNode} + // instances beneath this node. + leaves (prefix) { const results = [] for (const p of Object.keys(this.children)) { - results.push(...this.children[p].leaves()) + results.push(...this.children[p].leaves(prefix + [p])) } return results } @@ -104,8 +108,11 @@ class RegistryWatcherNode { // Private: Allocate a new node to track a {NativeWatcher}. // // * `nativeWatcher` An existing {NativeWatcher} instance. - constructor (nativeWatcher) { + // * `childPaths` {Array} of child directories that are currently the responsibility of this + // {NativeWatcher}, if any + constructor (nativeWatcher, childPaths) { this.nativeWatcher = nativeWatcher + this.childPaths = new Set(childPaths) } // Private: Accessor for the {NativeWatcher}. @@ -133,9 +140,11 @@ class RegistryWatcherNode { // Private: Discover this {RegistryWatcherNode} instance. // - // Returns: An {Array} containing this node. - leaves () { - return [this] + // * `prefix` {Array} of intermediate path segments to prepend to the resulting child paths. + // + // Returns: An {Array} containing a `{node, path}` object describing this node. + leaves (prefix) { + return [{node: this, path: prefix}] } } @@ -243,9 +252,9 @@ export default class NativeWatcherRegistry { const normalizedDirectory = await watcher.getNormalizedPathPromise() const pathSegments = normalizedDirectory.split(path.sep).filter(segment => segment.length > 0) - const attachToNew = () => { + const attachToNew = (childPaths) => { const native = this.createNative(normalizedDirectory) - const leaf = new RegistryWatcherNode(native) + const leaf = new RegistryWatcherNode(native, childPaths) this.tree = this.tree.insert(pathSegments, leaf) const sub = native.onWillStop(() => { @@ -268,7 +277,7 @@ export default class NativeWatcherRegistry { watcher.attachToNative(native, subpath) }, children: children => { - const newNative = attachToNew() + const newNative = attachToNew([]) // One or more NativeWatchers exist on child directories of the requested path. for (let i = 0; i < children.length; i++) { @@ -281,7 +290,7 @@ export default class NativeWatcherRegistry { childNative.stop() } }, - missing: attachToNew + missing: () => attachToNew([]) }) } } From 0c5674a56c29754b8f4fa281cc29c79ea1237a9d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 23 Jun 2017 16:08:38 -0400 Subject: [PATCH 33/94] Split subtrees into child watchers on parent watcher removal --- spec/native-watcher-registry-spec.js | 28 +++-- src/native-watcher-registry.js | 177 +++++++++++++++++++-------- 2 files changed, 140 insertions(+), 65 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 107e0efd3..6efa6c219 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -17,9 +17,14 @@ class MockWatcher { return Promise.resolve(this.normalizedPath) } - attachToNative (native) { - this.native = native - this.native.attached.push(this) + attachToNative (native, nativePath) { + if (this.normalizedPath.startsWith(nativePath)) { + if (this.native) { + this.native.attached = this.native.attached.filter(each => each !== this) + } + this.native = native + this.native.attached.push(this) + } } } @@ -33,12 +38,10 @@ class MockNative { this.emitter = new Emitter() } - reattachTo (newNative) { + reattachTo (newNative, nativePath) { for (const watcher of this.attached) { - watcher.attachToNative(newNative) + watcher.attachToNative(newNative, nativePath) } - - this.attached = [] } onWillStop (callback) { @@ -196,7 +199,7 @@ describe('NativeWatcherRegistry', function () { STOPPED.stop() - const runningNode = registry.tree.lookup(runningPath).when({ + const runningNode = registry.tree.root.lookup(runningPath).when({ parent: node => node, missing: () => false, children: () => false @@ -204,7 +207,7 @@ describe('NativeWatcherRegistry', function () { expect(runningNode).toBeTruthy() expect(runningNode.getNativeWatcher()).toBe(RUNNING) - const stoppedNode = registry.tree.lookup(stoppedPath).when({ + const stoppedNode = registry.tree.root.lookup(stoppedPath).when({ parent: () => false, missing: () => true, children: () => false @@ -249,25 +252,24 @@ describe('NativeWatcherRegistry', function () { expect(childWatcher1.native).toBe(PARENT) // Stopping the parent should detach and recreate the child watchers. - // (Here, they'll be the same watcher instances used before, because of the fake createNative implementation.) PARENT.stop() expect(childWatcher0.native).toBe(CHILD0) expect(childWatcher1.native).toBe(CHILD1) - expect(registry.tree.lookup(['parent']).when({ + expect(registry.tree.root.lookup(['parent']).when({ parent: () => false, missing: () => false, children: () => true })).toBe(true) - expect(registry.tree.lookup(['parent', 'child0']).when({ + expect(registry.tree.root.lookup(['parent', 'child0']).when({ parent: () => true, missing: () => false, children: () => false })).toBe(true) - expect(registry.tree.lookup(['parent', 'child1']).when({ + expect(registry.tree.root.lookup(['parent', 'child1']).when({ parent: () => true, missing: () => false, children: () => false diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 3d83bbf82..3ce127a85 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -2,6 +2,63 @@ import path from 'path' +class RegistryTree { + + constructor (basePathSegments, createNative) { + this.basePathSegments = basePathSegments + this.root = new RegistryNode() + this.createNative = createNative + } + + add (pathSegments, attachToNative) { + const absolutePathSegments = this.basePathSegments.concat(pathSegments) + const absolutePath = path.join(...absolutePathSegments) + + const attachToNew = (childPaths) => { + const native = this.createNative(absolutePath) + const leaf = new RegistryWatcherNode(native, absolutePathSegments, childPaths) + this.root = this.root.insert(pathSegments, leaf) + + const sub = native.onWillStop(() => { + sub.dispose() + this.root = this.root.remove(pathSegments, this.createNative) || new RegistryNode() + }) + + attachToNative(native, absolutePath) + return native + } + + this.root.lookup(pathSegments).when({ + parent: (parent, remaining) => { + // An existing NativeWatcher is watching the same directory or a parent directory of the requested path. + // Attach this Watcher to it as a filtering watcher and record it as a dependent child path. + const native = parent.getNativeWatcher() + parent.addChildPath(remaining) + attachToNative(native, path.join(...parent.getAbsolutePathSegments())) + }, + children: children => { + // One or more NativeWatchers exist on child directories of the requested path. Create a new native watcher + // on the parent directory, note the subscribed child paths, and cleanly stop the child native watchers. + const newNative = attachToNew(children.map(child => child.path)) + + for (let i = 0; i < children.length; i++) { + const childNode = children[i].node + const childNative = childNode.getNativeWatcher() + childNative.reattachTo(newNative, absolutePath) + childNative.dispose() + childNative.stop() + } + }, + missing: () => attachToNew([]) + }) + } + + getRoot () { + return this.root + } + +} + // Private: Non-leaf node in a tree used by the {NativeWatcherRegistry} to cover the allocated {Watcher} instances with // the most efficient set of {NativeWatcher} instances possible. Each {RegistryNode} maps to a directory in the // filesystem tree. @@ -57,10 +114,12 @@ class RegistryNode { // Private: Remove a {RegistryWatcherNode} by the exact watched directory. // // * `pathSegments` absolute pre-split filesystem path of the node to remove. + // * `createSplitNative` callback to be invoked with each child path segment {Array} if the {RegistryWatcherNode} + // is split into child watchers rather than removed outright. See {RegistryWatcherNode.remove}. // // Returns: The root of a new tree with the {RegistryWatcherNode} removed. Callers should replace their node // references with the returned value. - remove (pathSegments) { + remove (pathSegments, createSplitNative) { if (pathSegments.length === 0) { // Attempt to remove a path with child watchers. Do nothing. return this @@ -74,7 +133,7 @@ class RegistryNode { } // Recurse - const newChild = child.remove(pathSegments.slice(1)) + const newChild = child.remove(pathSegments.slice(1), createSplitNative) if (newChild === null) { delete this.children[pathKey] } else { @@ -95,7 +154,7 @@ class RegistryNode { leaves (prefix) { const results = [] for (const p of Object.keys(this.children)) { - results.push(...this.children[p].leaves(prefix + [p])) + results.push(...this.children[p].leaves(prefix.concat([p]))) } return results } @@ -108,11 +167,38 @@ class RegistryWatcherNode { // Private: Allocate a new node to track a {NativeWatcher}. // // * `nativeWatcher` An existing {NativeWatcher} instance. + // * `absolutePathSegments` The absolute path to this {NativeWatcher}'s directory as an {Array} of + // path segments. // * `childPaths` {Array} of child directories that are currently the responsibility of this - // {NativeWatcher}, if any - constructor (nativeWatcher, childPaths) { + // {NativeWatcher}, if any. Directories are represented as arrays of the path segments between this + // node's directory and the watched child path. + constructor (nativeWatcher, absolutePathSegments, childPaths) { this.nativeWatcher = nativeWatcher - this.childPaths = new Set(childPaths) + this.absolutePathSegments = absolutePathSegments + + // Store child paths as joined strings so they work as Set members. + this.childPaths = new Set() + for (let i = 0; i < childPaths.length; i++) { + this.childPaths.add(path.join(...childPaths[i])) + } + } + + // Private: Assume responsibility for a new child path. If this node is removed, it will instead + // split into a subtree with a new {RegistryWatcherNode} for each child path. + // + // * `childPathSegments` the {Array} of path segments between this node's directory and the watched + // child directory. + addChildPath (childPathSegments) { + this.childPaths.add(path.join(...childPathSegments)) + } + + // Private: Stop assuming responsbility for a previously assigned child path. If this node is + // removed, the named child path will no longer be allocated a {RegistryWatcherNode}. + // + // * `childPathSegments` the {Array} of path segments between this node's directory and the no longer + // watched child directory. + removeChildPath (childPathSegments) { + this.childPaths.delete(path.join(...childPathSegments)) } // Private: Accessor for the {NativeWatcher}. @@ -120,22 +206,47 @@ class RegistryWatcherNode { return this.nativeWatcher } + getAbsolutePathSegments () { + return this.absolutePathSegments + } + // Private: Identify how this watcher relates to a request to watch a directory tree. // // * `pathSegments` filesystem path of a new {Watcher} already split into an Array of directory names. - // + //g // Returns: A {ParentResult} referencing this node. lookup (pathSegments) { return new ParentResult(this, pathSegments) } - // Private: Remove this leaf node if the watcher's exact path matches. + // Private: Remove this leaf node if the watcher's exact path matches. If this node is covering additional + // {Watcher} instances on child paths, it will be split into a subtree. // // * `pathSegments` filesystem path of the node to remove. + // * `createSplitNative` callback invoked with each {Array} of absolute child path segments to create a native + // watcher on a subtree of this node. // - // Returns: {null} if the `pathSegments` are an exact match, {this} otherwise. - remove (pathSegments) { - return pathSegments.length === 0 ? null : this + // Returns: If `pathSegments` match this watcher's path exactly, returns `null` if this node has no `childPaths` + // or a new {RegistryNode} on a newly allocated subtree if it did. If `pathSegments` does not match the watcher's + // path, it's an attempt to remove a subnode that doesn't exist, so the remove call has no effect and returns + // `this` unaltered. + remove (pathSegments, createSplitNative) { + if (pathSegments.length !== 0) { + return this + } else if (this.childPaths.size > 0) { + let newSubTree = new RegistryTree(this.absolutePathSegments, createSplitNative) + + for (const childPath of this.childPaths) { + const childPathSegments = childPath.split(path.sep) + newSubTree.add(childPathSegments, (native, attachmentPath) => { + this.nativeWatcher.reattachTo(native, attachmentPath) + }) + } + + return newSubTree.getRoot() + } else { + return null + } } // Private: Discover this {RegistryWatcherNode} instance. @@ -234,8 +345,7 @@ export default class NativeWatcherRegistry { // * `createNative` {Function} that will be called with a normalized filesystem path to create a new native // filesystem watcher. constructor (createNative) { - this.tree = new RegistryNode() - this.createNative = createNative + this.tree = new RegistryTree([], createNative) } // Private: Attach a watcher to a directory, assigning it a {NativeWatcher}. If a suitable {NativeWatcher} already @@ -252,45 +362,8 @@ export default class NativeWatcherRegistry { const normalizedDirectory = await watcher.getNormalizedPathPromise() const pathSegments = normalizedDirectory.split(path.sep).filter(segment => segment.length > 0) - const attachToNew = (childPaths) => { - const native = this.createNative(normalizedDirectory) - const leaf = new RegistryWatcherNode(native, childPaths) - this.tree = this.tree.insert(pathSegments, leaf) - - const sub = native.onWillStop(() => { - this.tree = this.tree.remove(pathSegments) || new RegistryNode() - sub.dispose() - }) - - watcher.attachToNative(native, '') - - return native - } - - this.tree.lookup(pathSegments).when({ - parent: (parent, remaining) => { - // An existing NativeWatcher is watching a parent directory of the requested path. Attach this Watcher to - // it as a filtering watcher. - const native = parent.getNativeWatcher() - const subpath = remaining.length === 0 ? '' : path.join(...remaining) - - watcher.attachToNative(native, subpath) - }, - children: children => { - const newNative = attachToNew([]) - - // One or more NativeWatchers exist on child directories of the requested path. - for (let i = 0; i < children.length; i++) { - const child = children[i] - const childNative = child.getNativeWatcher() - childNative.reattachTo(newNative, normalizedDirectory) - childNative.dispose() - - // Don't await this Promise. Subscribers can listen for `onDidStop` to be notified if they choose. - childNative.stop() - } - }, - missing: () => attachToNew([]) + this.tree.add(pathSegments, (native, nativePath) => { + watcher.attachToNative(native, nativePath) }) } } From 2b79295d0b0db887d252a566899b30c224955f27 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 23 Jun 2017 16:09:34 -0400 Subject: [PATCH 34/94] (Untested) work to adapt to the registry API changes --- src/filesystem-manager.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 47ec98af4..28d1aed2b 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -122,8 +122,11 @@ class NativeWatcher { // Private: Broadcast an `onShouldDetach` event to prompt any {Watcher} instances bound here to attach to a new // {NativeWatcher} instead. - reattachTo (other) { - this.emitter.emit('should-detach', other) + // + // * `replacement` the new {NativeWatcher} instance that a live {Watcher} instance should reattach to instead. + // * `watchedPath` absolute path watched by the new {NativeWatcher}. + reattachTo (replacement, watchedPath) { + this.emitter.emit('should-detach', {replacement, watchedPath}) } // Private: Stop the native watcher and release any operating system resources associated with it. @@ -262,8 +265,8 @@ class Watcher { this.emitter.emit('did-error', err) })) - this.subs.add(native.onShouldDetach(replacement => { - if (replacement !== native) { + this.subs.add(native.onShouldDetach(({replacement, watchedPath}) => { + if (replacement !== native && this.normalizedPath.startsWith(watchedPath)) { this.attachToNative(replacement) } })) From 6cc3e4b6d2ec11ea5c356a294b033a8a8ec17b42 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:00:22 -0400 Subject: [PATCH 35/94] Test case for consolidating child watchers during split --- spec/native-watcher-registry-spec.js | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 6efa6c219..c3427d2c6 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -275,5 +275,64 @@ describe('NativeWatcherRegistry', function () { children: () => false })).toBe(true) }) + + it('consolidates children when splitting a parent watcher', async function () { + const CHILD0 = new MockNative('child0') + const PARENT = new MockNative('parent') + + const parentDir = path.join('parent') + const childDir0 = path.join(parentDir, 'child0') + const childDir1 = path.join(parentDir, 'child0', 'child1') + + createNative = dir => { + if (dir === parentDir) { + return PARENT + } else if (dir === childDir0) { + return CHILD0 + } else { + throw new Error(`Unexpected directory ${dir}`) + } + } + + const parentWatcher = new MockWatcher(parentDir) + const childWatcher0 = new MockWatcher(childDir0) + const childWatcher1 = new MockWatcher(childDir1) + + await registry.attach(parentWatcher) + await Promise.all([ + registry.attach(childWatcher0), + registry.attach(childWatcher1) + ]) + + // All three watchers should share the parent watcher's native watcher. + expect(parentWatcher.native).toBe(PARENT) + expect(childWatcher0.native).toBe(PARENT) + expect(childWatcher1.native).toBe(PARENT) + + // Stopping the parent should detach and create the child watchers. Both child watchers should + // share the same native watcher. + PARENT.stop() + + expect(childWatcher0.native).toBe(CHILD0) + expect(childWatcher1.native).toBe(CHILD0) + + expect(registry.tree.root.lookup(['parent']).when({ + parent: () => false, + missing: () => false, + children: () => true + })).toBe(true) + + expect(registry.tree.root.lookup(['parent', 'child0']).when({ + parent: () => true, + missing: () => false, + children: () => false + })).toBe(true) + + expect(registry.tree.root.lookup(['parent', 'child0', 'child1']).when({ + parent: () => true, + missing: () => false, + children: () => false + })).toBe(true) + }) }) }) From 0b17b3524457ba3a25c2ea645507dcd0881d1702 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:00:35 -0400 Subject: [PATCH 36/94] :shirt: :burn: whitespace --- src/filesystem-manager.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/filesystem-manager.js b/src/filesystem-manager.js index 28d1aed2b..14273423c 100644 --- a/src/filesystem-manager.js +++ b/src/filesystem-manager.js @@ -58,7 +58,6 @@ class NativeWatcher { await this.watcher.start() - this.state = WATCHER_STATE.RUNNING this.emitter.emit('did-start') } From d4edc6b8949b7bdb844e2b566e58a72901af20ee Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:01:04 -0400 Subject: [PATCH 37/94] Extra character for some reason? --- src/native-watcher-registry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 3ce127a85..9b78f5505 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -213,7 +213,7 @@ class RegistryWatcherNode { // Private: Identify how this watcher relates to a request to watch a directory tree. // // * `pathSegments` filesystem path of a new {Watcher} already split into an Array of directory names. - //g + // // Returns: A {ParentResult} referencing this node. lookup (pathSegments) { return new ParentResult(this, pathSegments) From ba7275dc4fb6e21f44977bfe09c5432af868bd9e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:16:35 -0400 Subject: [PATCH 38/94] Dump the tree structure to a string for debugging --- src/native-watcher-registry.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 9b78f5505..c3ba01d39 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -57,6 +57,10 @@ class RegistryTree { return this.root } + print () { + return this.root.print() + } + } // Private: Non-leaf node in a tree used by the {NativeWatcherRegistry} to cover the allocated {Watcher} instances with @@ -158,6 +162,19 @@ class RegistryNode { } return results } + + print (indent = 0) { + let spaces = '' + for (let i = 0; i < indent; i++) { + spaces += ' ' + } + + let result = '' + for (const p of Object.keys(this.children)) { + result += `${spaces}${p}\n${this.children[p].print(indent + 2)}` + } + return result + } } // Private: Leaf node within a {NativeWatcherRegistry} tree. Represents a directory that is covered by a @@ -257,6 +274,20 @@ class RegistryWatcherNode { leaves (prefix) { return [{node: this, path: prefix}] } + + print (indent = 0) { + let result = '' + for (let i = 0; i < indent; i++) { + result += ' ' + } + result += '[watcher' + if (this.childPaths.size > 0) { + result += ` +${this.childPaths.size}` + } + result += ']\n' + + return result + } } // Private: A {RegisteryNode} traversal result that's returned when neither a directory, its children, nor its parents From 2ae70aac08db432721037e3e0aaf06a2deeb527a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:51:45 -0400 Subject: [PATCH 39/94] Document RegistryTree. --- src/native-watcher-registry.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index c3ba01d39..6a88fe9f4 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -2,14 +2,35 @@ import path from 'path' +// Private: Map userland filesystem watcher subscriptions efficiently to deliver filesystem change notifications to +// each watcher with the most efficient coverage of native watchers. +// +// * If two watchers subscribe to the same directory, use a single native watcher for each. +// * Re-use a native watcher watching a parent directory for a watcher on a child directory. If the parent directory +// watcher is removed, it will be split into child watchers. +// * If any child directories already being watched, stop and replace them with a watcher on the parent directory. +// +// Uses a Trie whose structure mirrors the directory structure. class RegistryTree { + // Private: Construct a tree with no native watchers. + // + // * `basePathSegments` the position of this tree's root relative to the filesystem's root as an {Array} of directory + // names. + // * `createNative` {Function} used to construct new native watchers. It should accept an absolute path as an argument + // and return a new {NativeWatcher}. constructor (basePathSegments, createNative) { this.basePathSegments = basePathSegments this.root = new RegistryNode() this.createNative = createNative } + // Private: Identify the native watcher that should be used to produce events at a watched path, creating a new one + // if necessary. + // + // * `pathSegments` the path to watch represented as an {Array} of directory names relative to this {RegistryTree}'s + // root. + // * `attachToNative` {Function} invoked with the appropriate native watcher and the absolute path to its watch root. add (pathSegments, attachToNative) { const absolutePathSegments = this.basePathSegments.concat(pathSegments) const absolutePath = path.join(...absolutePathSegments) @@ -53,10 +74,12 @@ class RegistryTree { }) } + // Private: Access the root node of the tree. getRoot () { return this.root } + // Private: Return a {String} representation of this tree's structure for diagnostics and testing. print () { return this.root.print() } From 2d8f812f5681181533febd0176d0645bf2f1bb35 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 26 Jun 2017 09:56:13 -0400 Subject: [PATCH 40/94] More documentation touchups. --- src/native-watcher-registry.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 6a88fe9f4..77da22669 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -86,9 +86,9 @@ class RegistryTree { } -// Private: Non-leaf node in a tree used by the {NativeWatcherRegistry} to cover the allocated {Watcher} instances with -// the most efficient set of {NativeWatcher} instances possible. Each {RegistryNode} maps to a directory in the -// filesystem tree. +// Private: Non-leaf node in a {RegistryTree} used by the {NativeWatcherRegistry} to cover the allocated {Watcher} +// instances with the most efficient set of {NativeWatcher} instances possible. Each {RegistryNode} maps to a directory +// in the filesystem tree. class RegistryNode { // Private: Construct a new, empty node representing a node with no watchers. @@ -138,7 +138,7 @@ class RegistryNode { return this } - // Private: Remove a {RegistryWatcherNode} by the exact watched directory. + // Private: Remove a {RegistryWatcherNode} by its exact watched directory. // // * `pathSegments` absolute pre-split filesystem path of the node to remove. // * `createSplitNative` callback to be invoked with each child path segment {Array} if the {RegistryWatcherNode} @@ -186,6 +186,7 @@ class RegistryNode { return results } + // Private: Return a {String} representation of this subtree for diagnostics and testing. print (indent = 0) { let spaces = '' for (let i = 0; i < indent; i++) { @@ -246,6 +247,7 @@ class RegistryWatcherNode { return this.nativeWatcher } + // Private: Return the absolute path watched by this {NativeWatcher} as an {Array} of directory names. getAbsolutePathSegments () { return this.absolutePathSegments } @@ -298,6 +300,8 @@ class RegistryWatcherNode { return [{node: this, path: prefix}] } + // Private: Return a {String} representation of this watcher for diagnostics and testing. Indicates the number of + // child paths that this node's {NativeWatcher} is responsible for. print (indent = 0) { let result = '' for (let i = 0; i < indent; i++) { @@ -316,6 +320,7 @@ class RegistryWatcherNode { // Private: A {RegisteryNode} traversal result that's returned when neither a directory, its children, nor its parents // are present in the tree. class MissingResult { + // Private: Instantiate a new {MissingResult}. // // * `lastParent` the final succesfully traversed {RegistryNode}. From 7aeca7fc8ca5f93c4ab870aa8d67dc7692c2bd95 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 31 Jul 2017 14:52:11 -0400 Subject: [PATCH 41/94] :fire: FileSystemManager --- src/atom-environment.coffee | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/atom-environment.coffee b/src/atom-environment.coffee index 7deaafbdb..b37acddd1 100644 --- a/src/atom-environment.coffee +++ b/src/atom-environment.coffee @@ -46,7 +46,6 @@ TextBuffer = require 'text-buffer' Gutter = require './gutter' TextEditorRegistry = require './text-editor-registry' AutoUpdateManager = require './auto-update-manager' -FileSystemManager = require('./filesystem-manager').default # Essential: Atom global for dealing with packages, themes, menus, and the window. # @@ -118,9 +117,6 @@ class AtomEnvironment extends Model # Private: An {AutoUpdateManager} instance autoUpdater: null - # Public: A {FileSystemManager} instance - filesystem: null - saveStateDebounceInterval: 1000 ### @@ -141,7 +137,6 @@ class AtomEnvironment extends Model @views = new ViewRegistry(this) TextEditor.setScheduler(@views) @notifications = new NotificationManager - @filesystem = new FileSystemManager @updateProcessEnv ?= updateProcessEnv # For testing @stateStore = new StateStore('AtomEnvironments', 1) From 7aab9925a89480960c300a7ada6dc3deaa82fb80 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 31 Jul 2017 14:54:10 -0400 Subject: [PATCH 42/94] Rename filesystem-manager to path-watcher --- ...{filesystem-manager.js => path-watcher.js} | 48 ++++++++----------- 1 file changed, 20 insertions(+), 28 deletions(-) rename src/{filesystem-manager.js => path-watcher.js} (92%) diff --git a/src/filesystem-manager.js b/src/path-watcher.js similarity index 92% rename from src/filesystem-manager.js rename to src/path-watcher.js index 14273423c..4a0e435ec 100644 --- a/src/filesystem-manager.js +++ b/src/path-watcher.js @@ -24,6 +24,22 @@ export const WATCHER_STATE = { STOPPING: Symbol('stopping') } +const LIVE = new Set() + +const REGISTRY = new NativeWatcherRegistry( + normalizedPath => { + const nativeWatcher = new NativeWatcher(normalizedPath) + + LIVE.add(nativeWatcher) + const sub = nativeWatcher.onWillStop(() => { + LIVE.delete(nativeWatcher) + sub.dispose() + }) + + return nativeWatcher + } +) + // Private: Interface with and normalize events from a native OS filesystem watcher. class NativeWatcher { @@ -173,8 +189,8 @@ class NativeWatcher { } } -class Watcher { - constructor (watchedPath, nativeWatcherRegistry) { +export default class PathWatcher { + constructor (watchedPath, options) { this.watchedPath = watchedPath this.nativeWatcherRegistry = nativeWatcherRegistry @@ -297,34 +313,10 @@ class Watcher { } } -export default class FileSystemManager { - constructor () { - this.liveWatchers = new Set() - - this.nativeWatchers = new NativeWatcherRegistry( - normalizedPath => { - const nativeWatcher = new NativeWatcher(normalizedPath) - - this.liveWatchers.add(nativeWatcher) - const sub = nativeWatcher.onWillStop(() => { - this.liveWatchers.delete(nativeWatcher) - sub.dispose() - }) - - return nativeWatcher - } - ) - } - - getWatcher (rootPath) { - return new Watcher(rootPath, this.nativeWatchers) - } -} - // Private: Return a Promise that resolves when all {NativeWatcher} instances associated with a FileSystemManager // have stopped listening. This is useful for `afterEach()` blocks in unit tests. -export function stopAllWatchers (manager) { +export function stopAllWatchers () { return Promise.all( - Array.from(manager.liveWatchers, watcher => watcher.stop()) + Array.from(LIVE, watcher => watcher.stop()) ) } From 99d6f911cfdf1d0849f004ff49ff9a1810b8ee70 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 31 Jul 2017 16:24:56 -0400 Subject: [PATCH 43/94] Rename the filesystem-manager-spec too --- ...m-manager-spec.js => path-watcher-spec.js} | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) rename spec/{filesystem-manager-spec.js => path-watcher-spec.js} (83%) diff --git a/spec/filesystem-manager-spec.js b/spec/path-watcher-spec.js similarity index 83% rename from spec/filesystem-manager-spec.js rename to spec/path-watcher-spec.js index fda6c32f8..6d56c93de 100644 --- a/spec/filesystem-manager-spec.js +++ b/spec/path-watcher-spec.js @@ -6,25 +6,23 @@ import fsCb from 'fs-plus' import path from 'path' import {CompositeDisposable} from 'event-kit' -import FileSystemManager, {stopAllWatchers} from '../src/filesystem-manager' +import PathWatcher, {stopAllWatchers} from '../src/path-watcher' tempCb.track() const fs = promisifySome(fsCb, ['writeFile', 'mkdir', 'symlink', 'appendFile', 'realpath']) const temp = promisifySome(tempCb, ['mkdir']) -describe('FileSystemManager', function () { - let subs, manager +describe('PathWatcher', function () { + let subs beforeEach(function () { subs = new CompositeDisposable() - manager = new FileSystemManager() }) afterEach(async function () { subs.dispose() - - await stopAllWatchers(manager) + await stopAllWatchers() }) function waitForChanges (watcher, ...fileNames) { @@ -49,11 +47,11 @@ describe('FileSystemManager', function () { }) } - describe('getWatcher()', function () { + describe('new PatchWatcher()', function () { it('resolves getStartPromise() when the watcher begins listening', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher = manager.getWatcher(rootDir) + const watcher = new PathWatcher(rootDir) watcher.onDidChange(() => {}) await watcher.getStartPromise() @@ -61,7 +59,7 @@ describe('FileSystemManager', function () { it('does not start actually watching until an onDidChange subscriber is registered', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher = manager.getWatcher(rootDir) + const watcher = new PathWatcher(rootDir) let started = false const startPromise = watcher.getStartPromise().then(() => { @@ -87,7 +85,7 @@ describe('FileSystemManager', function () { it('automatically stops and removes the watcher when all onDidChange subscribers dispose', async function () { const dir = await temp.mkdir('atom-fsmanager-test-') - const watcher = manager.getWatcher(dir) + const watcher = new PathWatcher(dir) const sub0 = watcher.onDidChange(() => {}) const sub1 = watcher.onDidChange(() => {}) @@ -109,11 +107,11 @@ describe('FileSystemManager', function () { it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = manager.getWatcher(rootDir) + const watcher0 = new PathWatcher(rootDir) watcher0.onDidChange(() => {}) await watcher0.getStartPromise() - const watcher1 = manager.getWatcher(rootDir) + const watcher1 = new PathWatcher(rootDir) watcher1.onDidChange(() => {}) await watcher1.getStartPromise() @@ -123,12 +121,12 @@ describe('FileSystemManager', function () { it("reuses existing native watchers even while they're still starting", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = manager.getWatcher(rootDir) + const watcher0 = new PathWatcher(rootDir) watcher0.onDidChange(() => {}) await watcher0.getAttachedPromise() expect(watcher0.native.isRunning()).toBe(false) - const watcher1 = manager.getWatcher(rootDir) + const watcher1 = new PathWatcher(rootDir) watcher1.onDidChange(() => {}) await watcher1.getAttachedPromise() @@ -140,14 +138,14 @@ describe('FileSystemManager', function () { it("doesn't attach new watchers to a native watcher that's stopping", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = manager.getWatcher(rootDir) + const watcher0 = new PathWatcher(rootDir) const sub = watcher0.onDidChange(() => {}) await watcher0.getStartPromise() const native0 = watcher0.native sub.dispose() - const watcher1 = manager.getWatcher(rootDir) + const watcher1 = new PathWatcher(rootDir) watcher1.onDidChange(() => {}) expect(watcher1.native).not.toBe(native0) @@ -162,9 +160,9 @@ describe('FileSystemManager', function () { await fs.mkdir(subDir) // Keep the watchers alive with an undisposed subscription - const rootWatcher = manager.getWatcher(rootDir) + const rootWatcher = new PathWatcher(rootDir) rootWatcher.onDidChange(() => {}) - const childWatcher = manager.getWatcher(subDir) + const childWatcher = new PathWatcher(subDir) childWatcher.onDidChange(() => {}) await Promise.all([ @@ -208,11 +206,11 @@ describe('FileSystemManager', function () { ]) // Begin the child watchers and keep them alive - const subWatcher0 = manager.getWatcher(subDir0) + const subWatcher0 = new PathWatcher(subDir0) subWatcher0.onDidChange(() => {}) const subWatcherChanges0 = waitForChanges(subWatcher0, subFile0) - const subWatcher1 = manager.getWatcher(subDir1) + const subWatcher1 = new PathWatcher(subDir1) subWatcher1.onDidChange(() => {}) const subWatcherChanges1 = waitForChanges(subWatcher1, subFile1) @@ -222,7 +220,7 @@ describe('FileSystemManager', function () { expect(subWatcher0.native).not.toBe(subWatcher1.native) // Create the parent watcher - const parentWatcher = manager.getWatcher(parentDir) + const parentWatcher = new PathWatcher(parentDir) const parentWatcherChanges = waitForChanges(parentWatcher, rootFile, subFile0, subFile1) await parentWatcher.getStartPromise() @@ -243,17 +241,5 @@ describe('FileSystemManager', function () { parentWatcherChanges ]) }) - - describe('event normalization', function () { - xit('normalizes "changed" events') - xit('normalizes "added" events') - xit('normalizes "deleted" events') - xit('normalizes "renamed" events') - }) - - describe('symlinks', function () { - xit('reports events with symlink paths') - xit('uses the same native watcher even for symlink paths') - }) }) }) From 4f0b52d2ab6faaa3ebec2bd88c960fd369d9a7cd Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 10:56:17 -0400 Subject: [PATCH 44/94] Move the global watcher registry to a lazily initialized manager --- src/path-watcher.js | 67 +++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 4a0e435ec..92d961584 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -24,22 +24,6 @@ export const WATCHER_STATE = { STOPPING: Symbol('stopping') } -const LIVE = new Set() - -const REGISTRY = new NativeWatcherRegistry( - normalizedPath => { - const nativeWatcher = new NativeWatcher(normalizedPath) - - LIVE.add(nativeWatcher) - const sub = nativeWatcher.onWillStop(() => { - LIVE.delete(nativeWatcher) - sub.dispose() - }) - - return nativeWatcher - } -) - // Private: Interface with and normalize events from a native OS filesystem watcher. class NativeWatcher { @@ -189,8 +173,8 @@ class NativeWatcher { } } -export default class PathWatcher { - constructor (watchedPath, options) { +export class PathWatcher { + constructor (nativeWatcherRegistry, watchedPath, options) { this.watchedPath = watchedPath this.nativeWatcherRegistry = nativeWatcherRegistry @@ -313,10 +297,51 @@ export default class PathWatcher { } } +class PathWatcherManager { + static instance () { + if (!PathWatcherManager.theManager) { + PathWatcherManager.theManager = new PathWatcherManager() + } + return PathWatcherManager.theManager + } + + constructor () { + this.live = new Set() + this.nativeRegistry = new NativeWatcherRegistry( + normalizedPath => { + const nativeWatcher = new NativeWatcher(normalizedPath) + + this.live.add(nativeWatcher) + const sub = nativeWatcher.onWillStop(() => { + this.live.delete(nativeWatcher) + sub.dispose() + }) + + return nativeWatcher + } + ) + } + + createWatcher (rootPath, options, eventCallback) { + console.log(`watching root path = ${rootPath}`) + const watcher = new PathWatcher(this.nativeRegistry, rootPath, options) + watcher.onDidChange(eventCallback) + return watcher + } + + stopAllWatchers () { + return Promise.all( + Array.from(this.live, watcher => watcher.stop()) + ) + } +} + +export default function watchPath (rootPath, options, eventCallback) { + return PathWatcherManager.instance().createWatcher(rootPath, options, eventCallback) +} + // Private: Return a Promise that resolves when all {NativeWatcher} instances associated with a FileSystemManager // have stopped listening. This is useful for `afterEach()` blocks in unit tests. export function stopAllWatchers () { - return Promise.all( - Array.from(LIVE, watcher => watcher.stop()) - ) + return PathWatcherManager.instance().stopAllWatchers() } From 9c874c921e31098bf9a6216fb6056dd46f9377b2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 10:56:50 -0400 Subject: [PATCH 45/94] Use the watchPath API in specs --- spec/path-watcher-spec.js | 89 +++++++-------------------------------- 1 file changed, 15 insertions(+), 74 deletions(-) diff --git a/spec/path-watcher-spec.js b/spec/path-watcher-spec.js index 6d56c93de..2530e1561 100644 --- a/spec/path-watcher-spec.js +++ b/spec/path-watcher-spec.js @@ -6,7 +6,7 @@ import fsCb from 'fs-plus' import path from 'path' import {CompositeDisposable} from 'event-kit' -import PathWatcher, {stopAllWatchers} from '../src/path-watcher' +import watchPath, {stopAllWatchers} from '../src/path-watcher' tempCb.track() @@ -47,72 +47,21 @@ describe('PathWatcher', function () { }) } - describe('new PatchWatcher()', function () { + describe('watchPath()', function () { it('resolves getStartPromise() when the watcher begins listening', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher = new PathWatcher(rootDir) - watcher.onDidChange(() => {}) - + const watcher = watchPath(rootDir, {}, () => {}) await watcher.getStartPromise() }) - it('does not start actually watching until an onDidChange subscriber is registered', async function () { - const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher = new PathWatcher(rootDir) - - let started = false - const startPromise = watcher.getStartPromise().then(() => { - started = true - }) - - expect(watcher.native).toBe(null) - expect(watcher.normalizedPath).toBe(null) - expect(started).toBe(false) - - await watcher.getNormalizedPathPromise() - - expect(watcher.native).toBe(null) - expect(watcher.normalizedPath).not.toBe(null) - expect(started).toBe(false) - - watcher.onDidChange(() => {}) - await startPromise - - expect(watcher.native).not.toBe(null) - expect(started).toBe(true) - }) - - it('automatically stops and removes the watcher when all onDidChange subscribers dispose', async function () { - const dir = await temp.mkdir('atom-fsmanager-test-') - const watcher = new PathWatcher(dir) - - const sub0 = watcher.onDidChange(() => {}) - const sub1 = watcher.onDidChange(() => {}) - - await watcher.getStartPromise() - const native = watcher.native - expect(native).not.toBe(null) - expect(native.isRunning()).toBe(true) - - sub0.dispose() - expect(watcher.native).toBe(native) - expect(native.isRunning()).toBe(true) - - sub1.dispose() - expect(watcher.native).toBeNull() - expect(native.isRunning()).toBe(false) - }) - it('reuses an existing native watcher and resolves getStartPromise immediately if attached to a running watcher', async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = new PathWatcher(rootDir) - watcher0.onDidChange(() => {}) + const watcher0 = watchPath(rootDir, {}, () => {}) await watcher0.getStartPromise() - const watcher1 = new PathWatcher(rootDir) - watcher1.onDidChange(() => {}) + const watcher1 = watchPath(rootDir, {}, () => {}) await watcher1.getStartPromise() expect(watcher0.native).toBe(watcher1.native) @@ -121,13 +70,11 @@ describe('PathWatcher', function () { it("reuses existing native watchers even while they're still starting", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = new PathWatcher(rootDir) - watcher0.onDidChange(() => {}) + const watcher0 = watchPath(rootDir, {}, () => {}) await watcher0.getAttachedPromise() expect(watcher0.native.isRunning()).toBe(false) - const watcher1 = new PathWatcher(rootDir) - watcher1.onDidChange(() => {}) + const watcher1 = watchPath(rootDir, {}, () => {}) await watcher1.getAttachedPromise() expect(watcher0.native).toBe(watcher1.native) @@ -138,15 +85,13 @@ describe('PathWatcher', function () { it("doesn't attach new watchers to a native watcher that's stopping", async function () { const rootDir = await temp.mkdir('atom-fsmanager-test-') - const watcher0 = new PathWatcher(rootDir) - const sub = watcher0.onDidChange(() => {}) + const watcher0 = watchPath(rootDir, {}, () => {}) await watcher0.getStartPromise() const native0 = watcher0.native - sub.dispose() + watcher0.dispose() - const watcher1 = new PathWatcher(rootDir) - watcher1.onDidChange(() => {}) + const watcher1 = watchPath(rootDir, {}, () => {}) expect(watcher1.native).not.toBe(native0) }) @@ -160,10 +105,8 @@ describe('PathWatcher', function () { await fs.mkdir(subDir) // Keep the watchers alive with an undisposed subscription - const rootWatcher = new PathWatcher(rootDir) - rootWatcher.onDidChange(() => {}) - const childWatcher = new PathWatcher(subDir) - childWatcher.onDidChange(() => {}) + const rootWatcher = watchPath(rootDir, {}, () => {}) + const childWatcher = watchPath(subDir, {}, () => {}) await Promise.all([ rootWatcher.getStartPromise(), @@ -206,12 +149,10 @@ describe('PathWatcher', function () { ]) // Begin the child watchers and keep them alive - const subWatcher0 = new PathWatcher(subDir0) - subWatcher0.onDidChange(() => {}) + const subWatcher0 = watchPath(subDir0, {}, () => {}) const subWatcherChanges0 = waitForChanges(subWatcher0, subFile0) - const subWatcher1 = new PathWatcher(subDir1) - subWatcher1.onDidChange(() => {}) + const subWatcher1 = watchPath(subDir1, {}, () => {}) const subWatcherChanges1 = waitForChanges(subWatcher1, subFile1) await Promise.all( @@ -220,7 +161,7 @@ describe('PathWatcher', function () { expect(subWatcher0.native).not.toBe(subWatcher1.native) // Create the parent watcher - const parentWatcher = new PathWatcher(parentDir) + const parentWatcher = watchPath(parentDir, {}, () => {}) const parentWatcherChanges = waitForChanges(parentWatcher, rootFile, subFile0, subFile1) await parentWatcher.getStartPromise() From 3c967b07efe4df5b4345afc4009ea971bb38924a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 10:57:16 -0400 Subject: [PATCH 46/94] Use a cross-platform way to generate absolute paths for specs --- spec/native-watcher-registry-spec.js | 32 ++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index c3427d2c6..4144afbe4 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -7,6 +7,24 @@ import {Emitter} from 'event-kit' import NativeWatcherRegistry from '../src/native-watcher-registry' +function findRootDirectory () { + let current = process.cwd() + while (true) { + let next = path.resolve(current, '..') + if (next === current) { + return next + } else { + current = next + } + } +} +const ROOT = findRootDirectory() + +function absolute(parts) { + const candidate = path.join(...parts) + return path.isAbsolute(candidate) ? candidate : path.join(ROOT, candidate) +} + class MockWatcher { constructor (normalizedPath) { this.normalizedPath = normalizedPath @@ -66,7 +84,7 @@ describe('NativeWatcherRegistry', function () { }) it('attaches a Watcher to a newly created NativeWatcher for a new directory', async function () { - const watcher = new MockWatcher(path.join('some', 'path')) + const watcher = new MockWatcher(absolute('some', 'path')) const NATIVE = new MockNative('created') createNative = () => NATIVE @@ -77,7 +95,7 @@ describe('NativeWatcherRegistry', function () { it('reuses an existing NativeWatcher on the same directory', async function () { const EXISTING = new MockNative('existing') - const existingPath = path.join('existing', 'path') + const existingPath = absolute('existing', 'path') let firstTime = true createNative = () => { if (firstTime) { @@ -97,7 +115,7 @@ describe('NativeWatcherRegistry', function () { it('attaches to an existing NativeWatcher on a parent directory', async function () { const EXISTING = new MockNative('existing') - const parentDir = path.join('existing', 'path') + const parentDir = absolute('existing', 'path') const subDir = path.join(parentDir, 'sub', 'directory') let firstTime = true createNative = () => { @@ -117,10 +135,10 @@ describe('NativeWatcherRegistry', function () { }) it('adopts Watchers from NativeWatchers on child directories', async function () { - const parentDir = path.join('existing', 'path') + const parentDir = absolute('existing', 'path') const childDir0 = path.join(parentDir, 'child', 'directory', 'zero') const childDir1 = path.join(parentDir, 'child', 'directory', 'one') - const otherDir = path.join('another', 'path') + const otherDir = absolute('another', 'path') const CHILD0 = new MockNative('existing0') const CHILD1 = new MockNative('existing1') @@ -220,7 +238,7 @@ describe('NativeWatcherRegistry', function () { const CHILD1 = new MockNative('child1') const PARENT = new MockNative('parent') - const parentDir = path.join('parent') + const parentDir = absolute('parent') const childDir0 = path.join(parentDir, 'child0') const childDir1 = path.join(parentDir, 'child1') @@ -280,7 +298,7 @@ describe('NativeWatcherRegistry', function () { const CHILD0 = new MockNative('child0') const PARENT = new MockNative('parent') - const parentDir = path.join('parent') + const parentDir = absolute('parent') const childDir0 = path.join(parentDir, 'child0') const childDir1 = path.join(parentDir, 'child0', 'child1') From 6fdeedd4abd2ca86811dcd7daa1d73c3145341a7 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 11:12:23 -0400 Subject: [PATCH 47/94] Introduce a helper to re-join split absolute paths regardless of platform --- src/native-watcher-registry.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 77da22669..fa880b452 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -2,6 +2,12 @@ import path from 'path' +// Private: re-join the segments split from an absolute path to form another absolute path. +function absolute(...parts) { + const candidate = path.join(...parts) + return path.isAbsolute(candidate) ? candidate : path.join(path.sep, candidate) +} + // Private: Map userland filesystem watcher subscriptions efficiently to deliver filesystem change notifications to // each watcher with the most efficient coverage of native watchers. // @@ -10,7 +16,7 @@ import path from 'path' // watcher is removed, it will be split into child watchers. // * If any child directories already being watched, stop and replace them with a watcher on the parent directory. // -// Uses a Trie whose structure mirrors the directory structure. +// Uses a trie whose structure mirrors the directory structure. class RegistryTree { // Private: Construct a tree with no native watchers. @@ -33,7 +39,7 @@ class RegistryTree { // * `attachToNative` {Function} invoked with the appropriate native watcher and the absolute path to its watch root. add (pathSegments, attachToNative) { const absolutePathSegments = this.basePathSegments.concat(pathSegments) - const absolutePath = path.join(...absolutePathSegments) + const absolutePath = absolute(...absolutePathSegments) const attachToNew = (childPaths) => { const native = this.createNative(absolutePath) @@ -55,7 +61,7 @@ class RegistryTree { // Attach this Watcher to it as a filtering watcher and record it as a dependent child path. const native = parent.getNativeWatcher() parent.addChildPath(remaining) - attachToNative(native, path.join(...parent.getAbsolutePathSegments())) + attachToNative(native, absolute(...parent.getAbsolutePathSegments())) }, children: children => { // One or more NativeWatchers exist on child directories of the requested path. Create a new native watcher From 3fab3fed36a25f61e996f01148dd02eb6ab1eeaa Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 11:21:56 -0400 Subject: [PATCH 48/94] Consistent path handling in specs --- spec/native-watcher-registry-spec.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 4144afbe4..1663f3290 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -20,7 +20,7 @@ function findRootDirectory () { } const ROOT = findRootDirectory() -function absolute(parts) { +function absolute(...parts) { const candidate = path.join(...parts) return path.isAbsolute(candidate) ? candidate : path.join(ROOT, candidate) } @@ -196,28 +196,30 @@ describe('NativeWatcherRegistry', function () { const STOPPED = new MockNative('stopped') const RUNNING = new MockNative('running') - const stoppedPath = ['watcher', 'that', 'will', 'be', 'stopped'] - const runningPath = ['watcher', 'that', 'will', 'continue', 'to', 'exist'] + const stoppedPath = absolute('watcher', 'that', 'will', 'be', 'stopped') + const stoppedPathParts = stoppedPath.split(path.sep).filter(part => part.length > 0) + const runningPath = absolute('watcher', 'that', 'will', 'continue', 'to', 'exist') + const runningPathParts = runningPath.split(path.sep).filter(part => part.length > 0) createNative = dir => { - if (dir === path.join(...stoppedPath)) { + if (dir === stoppedPath) { return STOPPED - } else if (dir === path.join(...runningPath)) { + } else if (dir === runningPath) { return RUNNING } else { throw new Error(`Unexpected path: ${dir}`) } } - const stoppedWatcher = new MockWatcher(path.join(...stoppedPath)) + const stoppedWatcher = new MockWatcher(stoppedPath) await registry.attach(stoppedWatcher) - const runningWatcher = new MockWatcher(path.join(...runningPath)) + const runningWatcher = new MockWatcher(runningPath) await registry.attach(runningWatcher) STOPPED.stop() - const runningNode = registry.tree.root.lookup(runningPath).when({ + const runningNode = registry.tree.root.lookup(runningPathParts).when({ parent: node => node, missing: () => false, children: () => false @@ -225,7 +227,7 @@ describe('NativeWatcherRegistry', function () { expect(runningNode).toBeTruthy() expect(runningNode.getNativeWatcher()).toBe(RUNNING) - const stoppedNode = registry.tree.root.lookup(stoppedPath).when({ + const stoppedNode = registry.tree.root.lookup(stoppedPathParts).when({ parent: () => false, missing: () => true, children: () => false From 53ea43001945f2cac87c8d0639aef984bb9f93b8 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 11:22:03 -0400 Subject: [PATCH 49/94] Update spec name --- spec/path-watcher-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/path-watcher-spec.js b/spec/path-watcher-spec.js index 2530e1561..74d7980aa 100644 --- a/spec/path-watcher-spec.js +++ b/spec/path-watcher-spec.js @@ -13,7 +13,7 @@ tempCb.track() const fs = promisifySome(fsCb, ['writeFile', 'mkdir', 'symlink', 'appendFile', 'realpath']) const temp = promisifySome(tempCb, ['mkdir']) -describe('PathWatcher', function () { +describe('watchPath', function () { let subs beforeEach(function () { From afdb2f13a6fd7ef1442c0bc6ab50cf22e4d4d248 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 13:54:24 -0400 Subject: [PATCH 50/94] Doooooocs --- src/path-watcher.js | 105 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 3 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 92d961584..6e6801f8c 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -173,7 +173,22 @@ class NativeWatcher { } } +// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Construct these by calling +// {watchPath}. +// +// Maybe PathWatchers may be backed by the same native watcher to conserve operation system resources. Native watchers are +// started when at least one subscription is registered, and stopped when all subscriptions are disposed. +// +// Acts as a {Disposable}. +// export class PathWatcher { + + // Private: Instantiate a new PathWatcher. Call {watchPath} instead. + // + // * `nativeWatcherRegistry` {NativeWatcherRegistry} used to find and consolidate redundant watchers. + // * `watchedPath` {String} containing the absolute path to the root of the watched filesystem tree. + // * `options` See {watchPath} for options. + // constructor (nativeWatcherRegistry, watchedPath, options) { this.watchedPath = watchedPath this.nativeWatcherRegistry = nativeWatcherRegistry @@ -205,18 +220,45 @@ export class PathWatcher { this.subs = new CompositeDisposable() } + // Private: Return a {Promise} that will resolve with the normalized root path. getNormalizedPathPromise () { return this.normalizedPathPromise } + // Private: Return a {Promise} that will resolve the first time that this watcher is attached to a native watcher. getAttachedPromise () { return this.attachedPromise } + // Extended: Return a {Promise} that will resolve when the underlying native watcher is ready to begin sending events. + // When testing filesystem watchers, it's important to await this promise before making filesystem changes that you + // intend to assert about because there will be a delay between the instantiation of the watcher and the activation + // of the underlying OS resources that feed it events. + // + // ```js + // const {watchPath} = require('atom') + // const ROOT = path.join(__dirname, 'fixtures') + // const FILE = path.join(ROOT, 'filename.txt') + // + // describe('something', function () { + // it("doesn't miss events", async function () { + // const watcher = watchPath(ROOT, {}, events => {}) + // await watcher.getStartPromise() + // fs.writeFile(FILE, 'contents\n', err => { + // // The watcher is listening and the event should be received asyncronously + // } + // }) + // }) + // ``` getStartPromise () { return this.startPromise } + // Private: Attach another {Function} to be called with each batch of filesystem events. See {watchPath} for the + // spec of the callback's argument. + // + // Returns a {Disposable} that will stop the underlying watcher when all callbacks mapped to it have been disposed. + // onDidChange (callback) { if (this.native) { const sub = this.native.onDidChange(events => this.onNativeEvents(events, callback)) @@ -237,10 +279,15 @@ export class PathWatcher { }) } + // Extended: Invoke a {Function} when any errors related to this watcher are reported. + // + // Returns a {Disposable}. + // onDidError (callback) { return this.emitter.on('did-error', callback) } + // Private: Wire this watcher to an operating system-level native watcher implementation. attachToNative (native) { this.subs.dispose() this.native = native @@ -278,15 +325,19 @@ export class PathWatcher { this.resolveAttachedPromise() } + // Private: Invoked when the attached native watcher creates a batch of native filesystem events. The native watcher's + // events may include events for paths above this watcher's root path, so filter them to only include the relevant + // ones, then re-broadcast them to our subscribers. onNativeEvents (events, callback) { - // TODO does event.oldPath resolve symlinks? - const filtered = events.filter(event => event.oldPath.startsWith(this.normalizedPath)) + const filtered = events.filter(event => event.path.startsWith(this.normalizedPath)) if (filtered.length > 0) { callback(filtered) } } + // Extended: Unsubscribe all subscribers from filesystem events. The native watcher resources may take some time to + // be cleaned up, but the watcher will stop broadcasting events immediately. dispose () { for (const sub of this.changeCallbacks.values()) { sub.dispose() @@ -297,7 +348,12 @@ export class PathWatcher { } } +// Private: Globally tracked state used to de-duplicate related [PathWatchers]{PathWatcher}. class PathWatcherManager { + + // Private: Access or lazily initialize the singleton manager instance. + // + // Returns the one and only {PathWatcherManager}. static instance () { if (!PathWatcherManager.theManager) { PathWatcherManager.theManager = new PathWatcherManager() @@ -305,6 +361,7 @@ class PathWatcherManager { return PathWatcherManager.theManager } + // Private: Initialize global {PathWatcher} state. constructor () { this.live = new Set() this.nativeRegistry = new NativeWatcherRegistry( @@ -322,13 +379,16 @@ class PathWatcherManager { ) } + // Private: Create a {PathWatcher} tied to this global state. See {watchPath} for detailed arguments. createWatcher (rootPath, options, eventCallback) { - console.log(`watching root path = ${rootPath}`) const watcher = new PathWatcher(this.nativeRegistry, rootPath, options) watcher.onDidChange(eventCallback) return watcher } + // Private: Stop all living watchers. + // + // Returns a {Promise} that resolves when all native watcher resources are disposed. stopAllWatchers () { return Promise.all( Array.from(this.live, watcher => watcher.stop()) @@ -336,6 +396,45 @@ class PathWatcherManager { } } +// Extended: Invoke a callback with each filesystem event that occurs beneath a specified path. If you only need to +// watch events within the project's root paths, use {Project::onDidChangeFiles} instead. +// +// watchPath handles the efficient re-use of operating system resources across living watchers. Watching the same path +// more than once, or the child of a watched path, will re-use the existing native watcher. +// +// * `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. +// * `options` Control the watcher's behavior. +// * `recursive` If true, passing the path to a directory will recursively watch all changes beneath that +// directory. If false, only the file or directory itself will be watched. +// * `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. +// * `events` {Array} of objects that describe the events that have occurred. +// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, +// or `"renamed"`. +// * `path` {String} containing the absolute path to the filesystem entry that was acted upon. +// * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. +// +// Returns a {PathWatcher}. Note that every {PathWatcher} is a {Disposable}, so they can be managed by +// [CompositeDisposables]{CompositeDisposable} if desired. +// +// ```js +// const {watchPath} = require('atom') +// +// const disposable = watchPath('/var/log', {}, events => { +// console.log(`Received batch of ${events.length} events.`) +// for (const event of events) { +// console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" +// console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched +// if (event.type === 'renamed') { +// console.log(`.. renamed from: ${event.oldPath}`) +// } +// } +// }) +// +// // Immediately stop receiving filesystem events. If this is the last watcher, asynchronously release any OS +// // resources required to subscribe to these events. +// disposable.dispose() +// ``` +// export default function watchPath (rootPath, options, eventCallback) { return PathWatcherManager.instance().createWatcher(rootPath, options, eventCallback) } From ba11070d16994595845a8e3d17a6469170331bd1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 14:01:15 -0400 Subject: [PATCH 51/94] Translate nsfw events to the events we're advertising --- src/path-watcher.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 6e6801f8c..476582597 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -161,7 +161,16 @@ class NativeWatcher { const oldPath = path.join(event.directory, oldFileName) const newPath = newFileName && path.join(event.directory, newFileName) - return {oldPath, newPath, type} + const payload = {type} + + if (event.file) { + payload.path = path.join(event.directory, event.file) + } else { + payload.oldPath = path.join(event.directory, event.oldFile) + payload.path = path.join(event.directory, event.newFile) + } + + return payload })) } From 67a8ba2a046f6f6abe40589e554b1e746e6ba69c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 14:03:33 -0400 Subject: [PATCH 52/94] Adjust specs for the changed event shape --- spec/path-watcher-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/path-watcher-spec.js b/spec/path-watcher-spec.js index 74d7980aa..2b451fc8c 100644 --- a/spec/path-watcher-spec.js +++ b/spec/path-watcher-spec.js @@ -33,7 +33,7 @@ describe('watchPath', function () { return new Promise(resolve => { const sub = watcher.onDidChange(events => { for (const event of events) { - if (waiting.delete(event.oldPath)) { + if (waiting.delete(event.path)) { relevantEvents.push(event) } } From 697dfaf3b39688f60f1f863b5d51a1f73249228f Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 14:13:44 -0400 Subject: [PATCH 53/94] Re-export watchPath --- exports/atom.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/exports/atom.js b/exports/atom.js index 9ad4f60c2..3611958bf 100644 --- a/exports/atom.js +++ b/exports/atom.js @@ -7,6 +7,7 @@ import BufferedNodeProcess from '../src/buffered-node-process' import BufferedProcess from '../src/buffered-process' import GitRepository from '../src/git-repository' import Notification from '../src/notification' +import watchPath from '../src/path-watcher' const atomExport = { BufferedNodeProcess, @@ -20,7 +21,8 @@ const atomExport = { Directory, Emitter, Disposable, - CompositeDisposable + CompositeDisposable, + watchPath } // Shell integration is required by both Squirrel and Settings-View From b3f327b0b334c05e72b3ae475211ff106f193bfc Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 14:38:58 -0400 Subject: [PATCH 54/94] Implement `atom.project.onDidChangeFiles` --- src/project.coffee | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/project.coffee b/src/project.coffee index bf497e1db..81bf1954a 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -4,6 +4,7 @@ _ = require 'underscore-plus' fs = require 'fs-plus' {Emitter, Disposable} = require 'event-kit' TextBuffer = require 'text-buffer' +watchPath = require('./path-watcher').default DefaultDirectoryProvider = require './default-directory-provider' Model = require './model' @@ -28,11 +29,13 @@ class Project extends Model @repositoryPromisesByPath = new Map() @repositoryProviders = [new GitRepositoryProvider(this, config)] @loadPromisesByPath = {} + @watchersByPath = {} @consumeServices(packageManager) destroyed: -> buffer.destroy() for buffer in @buffers.slice() repository?.destroy() for repository in @repositories.slice() + watcher.dispose() for _, watcher in @watchersByPath @rootDirectories = [] @repositories = [] @@ -114,6 +117,26 @@ class Project extends Model callback(buffer) for buffer in @getBuffers() @onDidAddBuffer callback + # Public: Invoke the given callback when any filesystem change occurs within an open + # project path. + # + # To watch paths outside of open projects, use the {watchPaths} function. + # + # * `callback` {Function} to be called with batches of filesystem events reported by + # the operating system. + # * `events` An {Array} of objects the describe filesystem events. + # * `type` {String} describing the filesystem action that occurred. One of `"created"`, + # `"modified"`, `"deleted"`, or `"renamed"`. + # * `path` {String} containing the absolute path to the filesystem entry + # that was acted upon. + # * `oldPath` For rename events, {String} containing the filesystem entry's + # former absolute path. + # + # Returns a {PathWatcher} that may be used like a {Disposable} to manage + # this event subscription. + onDidChangeFiles: (callback) -> + @emitter.on 'did-change-files', callback + ### Section: Accessing the git repository ### @@ -171,6 +194,7 @@ class Project extends Model repository?.destroy() for repository in @repositories @rootDirectories = [] @repositories = [] + watcher.dispose() for _, watcher in @watchersByPath @addPath(projectPath, emitEvent: false) for projectPath in projectPaths @@ -186,6 +210,11 @@ class Project extends Model return if existingDirectory.getPath() is directory.getPath() @rootDirectories.push(directory) + @watchersByPath[directory] = watchPath directory.getPath(), {}, (events) => + @emitter.emit 'did-change-files', events + + for root, watcher in @watchersByPath + watcher.dispose() unless @rootDirectoryies.includes root repo = null for provider in @repositoryProviders @@ -220,6 +249,7 @@ class Project extends Model [removedDirectory] = @rootDirectories.splice(indexToRemove, 1) [removedRepository] = @repositories.splice(indexToRemove, 1) removedRepository?.destroy() unless removedRepository in @repositories + @watchersByPath[projectPath]?.dispose() @emitter.emit "did-change-paths", @getPaths() true else From ee9ad53d91a9e8d274bbf51ef42ca83f5f4d9509 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 15:55:08 -0400 Subject: [PATCH 55/94] :fire: unused variables --- src/path-watcher.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 476582597..723ee8788 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -156,11 +156,6 @@ class NativeWatcher { onEvents (events) { this.emitter.emit('did-change', events.map(event => { const type = ACTION_MAP.get(event.action) || `unexpected (${event.action})` - const oldFileName = event.file || event.oldFile - const newFileName = event.newFile - const oldPath = path.join(event.directory, oldFileName) - const newPath = newFileName && path.join(event.directory, newFileName) - const payload = {type} if (event.file) { From 1285a89a4b0da95eff0e8d293252ec64671f40cc Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 15:55:28 -0400 Subject: [PATCH 56/94] Reset @watchersByPath on atom.project.setPaths --- src/project.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/project.coffee b/src/project.coffee index 81bf1954a..aceb89cb9 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -194,7 +194,9 @@ class Project extends Model repository?.destroy() for repository in @repositories @rootDirectories = [] @repositories = [] + watcher.dispose() for _, watcher in @watchersByPath + @watchersByPath = {} @addPath(projectPath, emitEvent: false) for projectPath in projectPaths From f005fcdca105e04e3fcb97f9c836956aaf12a98a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 15:55:39 -0400 Subject: [PATCH 57/94] Use directory.getPath() as the object key --- src/project.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project.coffee b/src/project.coffee index aceb89cb9..22f6c2bf7 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -212,7 +212,7 @@ class Project extends Model return if existingDirectory.getPath() is directory.getPath() @rootDirectories.push(directory) - @watchersByPath[directory] = watchPath directory.getPath(), {}, (events) => + @watchersByPath[directory.getPath()] = watchPath directory.getPath(), {}, (events) => @emitter.emit 'did-change-files', events for root, watcher in @watchersByPath From 00346614a0d4b3b18e97638ee9bd406f96f5b11e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 15:58:38 -0400 Subject: [PATCH 58/94] Spec for onDidChangeFiles --- spec/project-spec.coffee | 51 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index a1a1dc189..4e144fe58 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -548,6 +548,57 @@ describe "Project", -> atom.project.removePath(ftpURI) expect(atom.project.getPaths()).toEqual [] + describe ".onDidChangeFiles()", -> + sub = [] + events = [] + checkCallback = -> + + beforeEach -> + sub = atom.project.onDidChangeFiles (incoming) -> + events.push incoming... + checkCallback() + + afterEach -> + sub.dispose() + + waitForEvents = (paths) -> + remaining = new Set(fs.realpathSync(path) for path in paths) + new Promise (resolve, reject) -> + checkCallback = -> + remaining.delete(event.path) for event in events + resolve() if remaining.size is 0 + + expire = -> + checkCallback = -> + console.error "Paths not seen:", Array.from(remaining) + reject(new Error('Expired before all expected events were delivered.')) + + checkCallback() + setTimeout expire, 2000 + + it "reports filesystem changes within project paths", -> + dirOne = temp.mkdirSync('atom-spec-project') + fileOne = path.join(dirOne, 'file-one.txt') + fileTwo = path.join(dirOne, 'file-two.txt') + dirTwo = temp.mkdirSync('atom-spec-project') + fileThree = path.join(dirTwo, 'file-three.txt') + + atom.project.setPaths([dirOne]) + + waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() + + runs -> + expect(atom.project.watchersByPath[dirTwo]).toEqual undefined + + fs.writeFileSync fileThree, "three\n" + fs.writeFileSync fileTwo, "two\n" + fs.writeFileSync fileOne, "one\n" + + waitsForPromise -> waitForEvents [fileOne, fileTwo] + + runs -> + expect(events.some (event) -> event.path is fileThree).toBeFalsy() + describe ".onDidAddBuffer()", -> it "invokes the callback with added text buffers", -> buffers = [] From c7a47a9e89743f07947e73356908cf687088bfa5 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 1 Aug 2017 16:40:54 -0400 Subject: [PATCH 59/94] Use module.exports to not break Joanna horribly --- exports/atom.js | 2 +- spec/native-watcher-registry-spec.js | 2 +- spec/path-watcher-spec.js | 2 +- src/native-watcher-registry.js | 8 ++++---- src/path-watcher.js | 21 ++++++++++----------- src/project.coffee | 2 +- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/exports/atom.js b/exports/atom.js index 3611958bf..d7ca55909 100644 --- a/exports/atom.js +++ b/exports/atom.js @@ -7,7 +7,7 @@ import BufferedNodeProcess from '../src/buffered-node-process' import BufferedProcess from '../src/buffered-process' import GitRepository from '../src/git-repository' import Notification from '../src/notification' -import watchPath from '../src/path-watcher' +import {watchPath} from '../src/path-watcher' const atomExport = { BufferedNodeProcess, diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 1663f3290..5cd4225ef 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -5,7 +5,7 @@ import {it, beforeEach} from './async-spec-helpers' import path from 'path' import {Emitter} from 'event-kit' -import NativeWatcherRegistry from '../src/native-watcher-registry' +import {NativeWatcherRegistry} from '../src/native-watcher-registry' function findRootDirectory () { let current = process.cwd() diff --git a/spec/path-watcher-spec.js b/spec/path-watcher-spec.js index 2b451fc8c..a805886e3 100644 --- a/spec/path-watcher-spec.js +++ b/spec/path-watcher-spec.js @@ -6,7 +6,7 @@ import fsCb from 'fs-plus' import path from 'path' import {CompositeDisposable} from 'event-kit' -import watchPath, {stopAllWatchers} from '../src/path-watcher' +import {watchPath, stopAllWatchers} from '../src/path-watcher' tempCb.track() diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index fa880b452..db3876d6b 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -1,6 +1,4 @@ -/** @babel */ - -import path from 'path' +const path = require('path') // Private: re-join the segments split from an absolute path to form another absolute path. function absolute(...parts) { @@ -403,7 +401,7 @@ class ChildrenResult { // 2. Subscribing to an existing {NativeWatcher} on a parent of a desired directory. // 3. Replacing multiple {NativeWatcher} instances on child directories with a single new {NativeWatcher} on the // parent. -export default class NativeWatcherRegistry { +class NativeWatcherRegistry { // Private: Instantiate an empty registry. // @@ -432,3 +430,5 @@ export default class NativeWatcherRegistry { }) } } + +module.exports = {NativeWatcherRegistry} diff --git a/src/path-watcher.js b/src/path-watcher.js index 723ee8788..4282a653f 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -1,12 +1,9 @@ -/** @babel */ +const fs = require('fs') +const path = require('path') -import fs from 'fs' -import path from 'path' - -import {Emitter, Disposable, CompositeDisposable} from 'event-kit' -import nsfw from 'nsfw' - -import NativeWatcherRegistry from './native-watcher-registry' +const {Emitter, Disposable, CompositeDisposable} = require('event-kit') +const nsfw = require('nsfw') +const {NativeWatcherRegistry} = require('./native-watcher-registry') // Private: Associate native watcher action type flags with descriptive String equivalents. const ACTION_MAP = new Map([ @@ -185,7 +182,7 @@ class NativeWatcher { // // Acts as a {Disposable}. // -export class PathWatcher { +class PathWatcher { // Private: Instantiate a new PathWatcher. Call {watchPath} instead. // @@ -439,12 +436,14 @@ class PathWatcherManager { // disposable.dispose() // ``` // -export default function watchPath (rootPath, options, eventCallback) { +function watchPath (rootPath, options, eventCallback) { return PathWatcherManager.instance().createWatcher(rootPath, options, eventCallback) } // Private: Return a Promise that resolves when all {NativeWatcher} instances associated with a FileSystemManager // have stopped listening. This is useful for `afterEach()` blocks in unit tests. -export function stopAllWatchers () { +function stopAllWatchers () { return PathWatcherManager.instance().stopAllWatchers() } + +module.exports = {watchPath, stopAllWatchers} diff --git a/src/project.coffee b/src/project.coffee index 22f6c2bf7..a28fc34a4 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -4,7 +4,7 @@ _ = require 'underscore-plus' fs = require 'fs-plus' {Emitter, Disposable} = require 'event-kit' TextBuffer = require 'text-buffer' -watchPath = require('./path-watcher').default +{watchPath} = require('./path-watcher') DefaultDirectoryProvider = require './default-directory-provider' Model = require './model' From 05a4f1f6fbb54f3f7e934d3ef1efefb7fb728ae0 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 08:29:10 -0400 Subject: [PATCH 60/94] :shirt: standard.js in script/test --- script/test | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/script/test b/script/test index fde922e94..dbb0f5c1f 100755 --- a/script/test +++ b/script/test @@ -90,7 +90,6 @@ for (let packageName in CONFIG.appMetadata.packageDependencies) { const pkgJsonPath = path.join(repositoryPackagePath, 'package.json') const nodeModulesPath = path.join(repositoryPackagePath, 'node_modules') - const nodeModulesBackupPath = path.join(repositoryPackagePath, 'node_modules.bak') let finalize = () => null if (require(pkgJsonPath).atomTestRunner) { console.log(`Installing test runner dependencies for ${packageName}`.bold.green) @@ -136,12 +135,16 @@ function runBenchmarkTests (callback) { let testSuitesToRun = testSuitesForPlatform(process.platform) -function testSuitesForPlatform(platform) { - switch(platform) { - case 'darwin': return [runCoreMainProcessTests, runCoreRenderProcessTests, runBenchmarkTests].concat(packageTestSuites) - case 'win32': return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] - case 'linux': return [runCoreMainProcessTests] - default: return [] +function testSuitesForPlatform (platform) { + switch (platform) { + case 'darwin': + return [runCoreMainProcessTests, runCoreRenderProcessTests, runBenchmarkTests].concat(packageTestSuites) + case 'win32': + return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] + case 'linux': + return [runCoreMainProcessTests] + default: + return [] } } From 320664a35939aeafa71c25b12925e9b687bbdca9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 09:04:33 -0400 Subject: [PATCH 61/94] Remove an export I missed --- src/path-watcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 4282a653f..17dab6d67 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -14,7 +14,7 @@ const ACTION_MAP = new Map([ ]) // Private: Possible states of a {NativeWatcher}. -export const WATCHER_STATE = { +const WATCHER_STATE = { STOPPED: Symbol('stopped'), STARTING: Symbol('starting'), RUNNING: Symbol('running'), From 318708bb421687442ce526d91ec4e8a08613c5af Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 09:50:57 -0400 Subject: [PATCH 62/94] wip --- script/lib/generate-startup-snapshot.js | 6 +++++- src/native-watcher-registry.js | 2 ++ src/path-watcher.js | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/script/lib/generate-startup-snapshot.js b/script/lib/generate-startup-snapshot.js index 9ea3abf93..3f504b22a 100644 --- a/script/lib/generate-startup-snapshot.js +++ b/script/lib/generate-startup-snapshot.js @@ -23,11 +23,12 @@ module.exports = function (packagedAppPath) { process.stdout.write(`Generating snapshot script at "${snapshotScriptPath}" (${++processedFiles})`) const relativePath = path.relative(baseDirPath, modulePath) - return ( + const result = ( modulePath.endsWith('.node') || coreModules.has(modulePath) || (relativePath.startsWith(path.join('..', 'src')) && relativePath.endsWith('-element.js')) || relativePath.startsWith(path.join('..', 'node_modules', 'dugite')) || + relativePath.startsWith(path.join('..', 'node_modules', 'nsfw')) || relativePath === path.join('..', 'exports', 'atom.js') || relativePath === path.join('..', 'src', 'electron-shims.js') || relativePath === path.join('..', 'src', 'safe-clipboard.js') || @@ -39,6 +40,7 @@ module.exports = function (packagedAppPath) { relativePath === path.join('..', 'node_modules', 'decompress-zip', 'lib', 'decompress-zip.js') || relativePath === path.join('..', 'node_modules', 'debug', 'node.js') || relativePath === path.join('..', 'node_modules', 'fs-extra', 'lib', 'index.js') || + relativePath === path.join('..', 'node_modules', 'github', 'node_modules', 'fs-extra', 'lib', 'index.js') || relativePath === path.join('..', 'node_modules', 'git-utils', 'lib', 'git.js') || relativePath === path.join('..', 'node_modules', 'glob', 'glob.js') || relativePath === path.join('..', 'node_modules', 'graceful-fs', 'graceful-fs.js') || @@ -69,6 +71,8 @@ module.exports = function (packagedAppPath) { relativePath === path.join('..', 'node_modules', 'tmp', 'lib', 'tmp.js') || relativePath === path.join('..', 'node_modules', 'tree-view', 'node_modules', 'minimatch', 'minimatch.js') ) + fs.appendFileSync('snapshot-files.txt', `${relativePath} = ${result}\n`) + return result } }).then((snapshotScript) => { fs.writeFileSync(snapshotScriptPath, snapshotScript) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index db3876d6b..274b30cde 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -1,3 +1,5 @@ +/** @babel */ + const path = require('path') // Private: re-join the segments split from an absolute path to form another absolute path. diff --git a/src/path-watcher.js b/src/path-watcher.js index 17dab6d67..77eb87ecf 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -1,3 +1,5 @@ +/** @babel */ + const fs = require('fs') const path = require('path') From 1f567137028dbef68ca22749219a31d6435abaa9 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 11:50:07 -0400 Subject: [PATCH 63/94] Un-exclude nsfw --- script/lib/generate-startup-snapshot.js | 1 - 1 file changed, 1 deletion(-) diff --git a/script/lib/generate-startup-snapshot.js b/script/lib/generate-startup-snapshot.js index 3f504b22a..6fe7b519c 100644 --- a/script/lib/generate-startup-snapshot.js +++ b/script/lib/generate-startup-snapshot.js @@ -28,7 +28,6 @@ module.exports = function (packagedAppPath) { coreModules.has(modulePath) || (relativePath.startsWith(path.join('..', 'src')) && relativePath.endsWith('-element.js')) || relativePath.startsWith(path.join('..', 'node_modules', 'dugite')) || - relativePath.startsWith(path.join('..', 'node_modules', 'nsfw')) || relativePath === path.join('..', 'exports', 'atom.js') || relativePath === path.join('..', 'src', 'electron-shims.js') || relativePath === path.join('..', 'src', 'safe-clipboard.js') || From 931b4e70554e64bbc2039de45b625af0ad1bf0f4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 12:33:06 -0400 Subject: [PATCH 64/94] :shirt: lint lint lint --- src/native-watcher-registry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native-watcher-registry.js b/src/native-watcher-registry.js index 274b30cde..a779f78f5 100644 --- a/src/native-watcher-registry.js +++ b/src/native-watcher-registry.js @@ -3,7 +3,7 @@ const path = require('path') // Private: re-join the segments split from an absolute path to form another absolute path. -function absolute(...parts) { +function absolute (...parts) { const candidate = path.join(...parts) return path.isAbsolute(candidate) ? candidate : path.join(path.sep, candidate) } From b9080bcec551c14073d30ff41ab8cf3c98420036 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 12:59:38 -0400 Subject: [PATCH 65/94] Don't use "path" as a variable name --- spec/project-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 4e144fe58..75fc2caf8 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -562,7 +562,7 @@ describe "Project", -> sub.dispose() waitForEvents = (paths) -> - remaining = new Set(fs.realpathSync(path) for path in paths) + remaining = new Set(fs.realpathSync(p) for p in paths) new Promise (resolve, reject) -> checkCallback = -> remaining.delete(event.path) for event in events From 1e739aa891df66d65cd5208056a3e77083470e5e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Wed, 2 Aug 2017 14:55:12 -0400 Subject: [PATCH 66/94] Prepend a drive root to lookup paths on Windows --- spec/native-watcher-registry-spec.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index 5cd4225ef..da361dbb7 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -20,11 +20,16 @@ function findRootDirectory () { } const ROOT = findRootDirectory() -function absolute(...parts) { +function absolute (...parts) { const candidate = path.join(...parts) return path.isAbsolute(candidate) ? candidate : path.join(ROOT, candidate) } +function prependRoot (...parts) { + const candidate = path.join(...parts) + return path.isAbsolute(candidate) ? parts : [ROOT].concat(parts) +} + class MockWatcher { constructor (normalizedPath) { this.normalizedPath = normalizedPath @@ -277,19 +282,19 @@ describe('NativeWatcherRegistry', function () { expect(childWatcher0.native).toBe(CHILD0) expect(childWatcher1.native).toBe(CHILD1) - expect(registry.tree.root.lookup(['parent']).when({ + expect(registry.tree.root.lookup(prependRoot('parent')).when({ parent: () => false, missing: () => false, children: () => true })).toBe(true) - expect(registry.tree.root.lookup(['parent', 'child0']).when({ + expect(registry.tree.root.lookup(prependRoot('parent', 'child0')).when({ parent: () => true, missing: () => false, children: () => false })).toBe(true) - expect(registry.tree.root.lookup(['parent', 'child1']).when({ + expect(registry.tree.root.lookup(prependRoot('parent', 'child1')).when({ parent: () => true, missing: () => false, children: () => false @@ -336,19 +341,19 @@ describe('NativeWatcherRegistry', function () { expect(childWatcher0.native).toBe(CHILD0) expect(childWatcher1.native).toBe(CHILD0) - expect(registry.tree.root.lookup(['parent']).when({ + expect(registry.tree.root.lookup(prependRoot('parent')).when({ parent: () => false, missing: () => false, children: () => true })).toBe(true) - expect(registry.tree.root.lookup(['parent', 'child0']).when({ + expect(registry.tree.root.lookup(prependRoot('parent', 'child0')).when({ parent: () => true, missing: () => false, children: () => false })).toBe(true) - expect(registry.tree.root.lookup(['parent', 'child0', 'child1']).when({ + expect(registry.tree.root.lookup(prependRoot('parent', 'child0', 'child1')).when({ parent: () => true, missing: () => false, children: () => false From b6a3c5c6d21ac5d5ca5e9337986b63f5b7da377c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 08:51:44 -0400 Subject: [PATCH 67/94] Consistently split paths in test cases --- spec/native-watcher-registry-spec.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/spec/native-watcher-registry-spec.js b/spec/native-watcher-registry-spec.js index da361dbb7..bc657f496 100644 --- a/spec/native-watcher-registry-spec.js +++ b/spec/native-watcher-registry-spec.js @@ -25,9 +25,8 @@ function absolute (...parts) { return path.isAbsolute(candidate) ? candidate : path.join(ROOT, candidate) } -function prependRoot (...parts) { - const candidate = path.join(...parts) - return path.isAbsolute(candidate) ? parts : [ROOT].concat(parts) +function parts (fullPath) { + return fullPath.split(path.sep).filter(part => part.length > 0) } class MockWatcher { @@ -282,19 +281,19 @@ describe('NativeWatcherRegistry', function () { expect(childWatcher0.native).toBe(CHILD0) expect(childWatcher1.native).toBe(CHILD1) - expect(registry.tree.root.lookup(prependRoot('parent')).when({ + expect(registry.tree.root.lookup(parts(parentDir)).when({ parent: () => false, missing: () => false, children: () => true })).toBe(true) - expect(registry.tree.root.lookup(prependRoot('parent', 'child0')).when({ + expect(registry.tree.root.lookup(parts(childDir0)).when({ parent: () => true, missing: () => false, children: () => false })).toBe(true) - expect(registry.tree.root.lookup(prependRoot('parent', 'child1')).when({ + expect(registry.tree.root.lookup(parts(childDir1)).when({ parent: () => true, missing: () => false, children: () => false @@ -341,19 +340,19 @@ describe('NativeWatcherRegistry', function () { expect(childWatcher0.native).toBe(CHILD0) expect(childWatcher1.native).toBe(CHILD0) - expect(registry.tree.root.lookup(prependRoot('parent')).when({ + expect(registry.tree.root.lookup(parts(parentDir)).when({ parent: () => false, missing: () => false, children: () => true })).toBe(true) - expect(registry.tree.root.lookup(prependRoot('parent', 'child0')).when({ + expect(registry.tree.root.lookup(parts(childDir0)).when({ parent: () => true, missing: () => false, children: () => false })).toBe(true) - expect(registry.tree.root.lookup(prependRoot('parent', 'child0', 'child1')).when({ + expect(registry.tree.root.lookup(parts(childDir1)).when({ parent: () => true, missing: () => false, children: () => false From 3e3ab737483fed9175b3468666fa65c1d07ac454 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 07:48:22 -0700 Subject: [PATCH 68/94] Use distinct names for spec directories --- spec/project-spec.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 75fc2caf8..6c849cab1 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -577,10 +577,10 @@ describe "Project", -> setTimeout expire, 2000 it "reports filesystem changes within project paths", -> - dirOne = temp.mkdirSync('atom-spec-project') + dirOne = temp.mkdirSync('atom-spec-project-one') fileOne = path.join(dirOne, 'file-one.txt') fileTwo = path.join(dirOne, 'file-two.txt') - dirTwo = temp.mkdirSync('atom-spec-project') + dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') atom.project.setPaths([dirOne]) From 3deb26b6a08619f54a21e756392c39a66155647e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 11:52:15 -0400 Subject: [PATCH 69/94] console.log debugging --- spec/project-spec.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 6c849cab1..16b88893f 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -583,12 +583,14 @@ describe "Project", -> dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') + console.log "Setting project paths to #{dirOne}" atom.project.setPaths([dirOne]) waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() runs -> expect(atom.project.watchersByPath[dirTwo]).toEqual undefined + console.log "Watching #{dirOne} but not #{dirTwo}" fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" @@ -597,6 +599,7 @@ describe "Project", -> waitsForPromise -> waitForEvents [fileOne, fileTwo] runs -> + console.log "Events seen:\n#{require('util').inspect events}" expect(events.some (event) -> event.path is fileThree).toBeFalsy() describe ".onDidAddBuffer()", -> From dedf5193cd9615366515055f6a2254fb9d7c0ccd Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 13:11:09 -0400 Subject: [PATCH 70/94] Does stderr work there... ? --- spec/project-spec.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 16b88893f..47de4c11e 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -583,14 +583,14 @@ describe "Project", -> dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') - console.log "Setting project paths to #{dirOne}" + process.stderr.write "Setting project paths to #{dirOne}\n" atom.project.setPaths([dirOne]) waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() runs -> expect(atom.project.watchersByPath[dirTwo]).toEqual undefined - console.log "Watching #{dirOne} but not #{dirTwo}" + process.stderr.write "Watching #{dirOne} but not #{dirTwo}\n" fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" @@ -599,7 +599,7 @@ describe "Project", -> waitsForPromise -> waitForEvents [fileOne, fileTwo] runs -> - console.log "Events seen:\n#{require('util').inspect events}" + process.stderr.write "Events seen:\n#{require('util').inspect events}\n" expect(events.some (event) -> event.path is fileThree).toBeFalsy() describe ".onDidAddBuffer()", -> From 654cb26819f419fa179a4348333935b5dd26ce3e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 14:20:24 -0400 Subject: [PATCH 71/94] Only run render process tests on Windows for the moment --- script/test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/script/test b/script/test index 2f22b1e0a..bf3a53361 100755 --- a/script/test +++ b/script/test @@ -154,7 +154,8 @@ function testSuitesForPlatform (platform) { case 'darwin': return [runCoreMainProcessTests, runCoreRenderProcessTests, runBenchmarkTests].concat(packageTestSuites) case 'win32': - return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] + // return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] + return (process.arch === 'x64') ? [runCoreRenderProcessTests] : [] case 'linux': return [runCoreMainProcessTests] default: From 263adde3776fe9f1aca58903be5d999f8219e5e1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 14:22:54 -0400 Subject: [PATCH 72/94] Use async cleanup to avoid ENOTEMPTY on Windows --- spec/project-spec.coffee | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 47de4c11e..3d1ec0369 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -14,7 +14,13 @@ describe "Project", -> waits(1) afterEach -> - temp.cleanupSync() + waitsForPromise -> + new Promise (resolve, reject) -> + temp.cleanup err -> + if err? + reject(err) + else + resolve() describe "serialization", -> deserializedProject = null From bb91bb58e5646165ee88973c2ab405364e331522 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 14:30:40 -0400 Subject: [PATCH 73/94] Okay fine let's do this the dumb way --- script/test | 10 +++++++++- spec/project-spec.coffee | 9 ++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/script/test b/script/test index bf3a53361..bbfda912b 100755 --- a/script/test +++ b/script/test @@ -69,7 +69,15 @@ function runCoreRenderProcessTests (callback) { console.log('Executing core render process tests'.bold.green) const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) - cp.on('error', error => { callback(error) }) + cp.on('error', error => { + try { + const projectSpecLog = fs.readFileSync('project-spec.log', {encoding: 'utf8'}) + console.log(`project-spec log:\n${projectSpecLog}\n`) + } catch (e) { + console.error(`Unable to open log file:\n${e.stack}`) + } + callback(error) + }) cp.on('close', exitCode => { callback(null, exitCode) }) } diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 3d1ec0369..5dcaaeddd 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -6,6 +6,9 @@ path = require 'path' {Directory} = require 'pathwatcher' GitRepository = require '../src/git-repository' +logToFile = text -> + fs.appendFileSync 'project-spec.log', text + describe "Project", -> beforeEach -> atom.project.setPaths([atom.project.getDirectories()[0]?.resolve('dir')]) @@ -589,14 +592,14 @@ describe "Project", -> dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') - process.stderr.write "Setting project paths to #{dirOne}\n" + logToFile "Setting project paths to #{dirOne}\n" atom.project.setPaths([dirOne]) waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() runs -> expect(atom.project.watchersByPath[dirTwo]).toEqual undefined - process.stderr.write "Watching #{dirOne} but not #{dirTwo}\n" + logToFile "Watching #{dirOne} but not #{dirTwo}\n" fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" @@ -605,7 +608,7 @@ describe "Project", -> waitsForPromise -> waitForEvents [fileOne, fileTwo] runs -> - process.stderr.write "Events seen:\n#{require('util').inspect events}\n" + logToFile "Events seen:\n#{require('util').inspect events}\n" expect(events.some (event) -> event.path is fileThree).toBeFalsy() describe ".onDidAddBuffer()", -> From 94c91c57b1879e3c200d17f884062f2af1953d6d Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 15:32:18 -0400 Subject: [PATCH 74/94] Explicitly put the logfile in ${HOME} --- script/test | 2 +- spec/project-spec.coffee | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/script/test b/script/test index bbfda912b..5aa3dd2b8 100755 --- a/script/test +++ b/script/test @@ -71,7 +71,7 @@ function runCoreRenderProcessTests (callback) { const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) cp.on('error', error => { try { - const projectSpecLog = fs.readFileSync('project-spec.log', {encoding: 'utf8'}) + const projectSpecLog = fs.readFileSync(path.join(process.env.HOME, 'project-spec.log'), {encoding: 'utf8'}) console.log(`project-spec log:\n${projectSpecLog}\n`) } catch (e) { console.error(`Unable to open log file:\n${e.stack}`) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 5dcaaeddd..97f1f0417 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -6,8 +6,8 @@ path = require 'path' {Directory} = require 'pathwatcher' GitRepository = require '../src/git-repository' -logToFile = text -> - fs.appendFileSync 'project-spec.log', text +logToFile = (text) -> + fs.appendFileSync path.join(process.env.HOME, 'project-spec.log'), text describe "Project", -> beforeEach -> @@ -19,7 +19,7 @@ describe "Project", -> afterEach -> waitsForPromise -> new Promise (resolve, reject) -> - temp.cleanup err -> + temp.cleanup (err) -> if err? reject(err) else From 83f022b6189ed02d19ada6d3805b1a8ed9e29882 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 11:48:13 -0400 Subject: [PATCH 75/94] Add a fileSystemWatcher config key to use as a feature flag --- src/config-schema.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/config-schema.js b/src/config-schema.js index 39f058555..fb0164766 100644 --- a/src/config-schema.js +++ b/src/config-schema.js @@ -308,6 +308,21 @@ const configSchema = { description: 'Warn before opening files larger than this number of megabytes.', type: 'number', default: 40 + }, + fileSystemWatcher: { + description: 'Choose the underlying implementation used to watch for filesystem changes. Emulating changes will miss any events caused by applications other than Atom, but may help prevent crashes or freezes.', + type: 'string', + default: 'native', + enum: [ + { + value: 'native', + description: 'Native operating system APIs' + }, + { + value: 'atom', + description: 'Emulated with Atom events' + } + ] } } }, From 418fe48bad161ba16883b1d37a53f87ade573629 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 11:48:36 -0400 Subject: [PATCH 76/94] Emulate a "filesystem watcher" by subscribing to Atom events --- src/path-watcher.js | 198 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 174 insertions(+), 24 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 77eb87ecf..a12bb9944 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -23,6 +23,144 @@ const WATCHER_STATE = { STOPPING: Symbol('stopping') } +// Private: Emulate a "filesystem watcher" by subscribing to Atom events like buffers being saved. This will miss +// any changes made to files outside of Atom, but it also has no overhead. +class AtomBackend { + async start (rootPath, eventCallback, errorCallback) { + const getRealPath = givenPath => { + return new Promise(resolve => { + fs.realpath(givenPath, (err, resolvedPath) => { + err ? resolve(null) : resolve(resolvedPath) + }) + }) + } + + this.subs = new CompositeDisposable() + + this.subs.add(atom.workspace.observeTextEditors(async editor => { + let realPath = await getRealPath(editor.getPath()) + if (!realPath || !realPath.startsWith(rootPath)) { + return + } + + const announce = (type, oldPath) => { + const payload = {type, path: realPath} + if (oldPath) payload.oldPath = oldPath + eventCallback([payload]) + } + + const buffer = editor.getBuffer() + + this.subs.add(buffer.onDidConflict(() => announce('modified'))) + this.subs.add(buffer.onDidReload(() => announce('modified'))) + this.subs.add(buffer.onDidSave(event => { + if (event.path === realPath) { + announce('modified') + } else { + const oldPath = realPath + realPath = event.path + announce('renamed', oldPath) + } + })) + + this.subs.add(buffer.onDidDelete(() => announce('deleted'))) + + this.subs.add(buffer.onDidChangePath(newPath => { + if (newPath !== realPath) { + const oldPath = realPath + realPath = newPath + announce('renamed', oldPath) + } + })) + })) + + // Giant-ass brittle hack to hook files (and eventually directories) created from the TreeView. + const treeViewPackage = await atom.packages.getLoadedPackage('tree-view') + if (!treeViewPackage) return + await treeViewPackage.activationPromise + const treeViewModule = treeViewPackage.mainModule + if (!treeViewModule) return + const treeView = treeViewModule.getTreeViewInstance() + + const isOpenInEditor = async eventPath => { + const openPaths = await Promise.all( + atom.workspace.getTextEditors().map(editor => getRealPath(editor.getPath())) + ) + return openPaths.includes(eventPath) + } + + this.subs.add(treeView.onFileCreated(async event => { + const realPath = await getRealPath(event.path) + if (!realPath) return + + eventCallback([{type: 'added', path: realPath}]) + })) + + this.subs.add(treeView.onEntryDeleted(async event => { + const realPath = await getRealPath(event.path) + if (!realPath || isOpenInEditor(realPath)) return + + eventCallback([{type: 'deleted', path: realPath}]) + })) + + this.subs.add(treeView.onEntryMoved(async event => { + const [realNewPath, realOldPath] = await Promise.all([ + getRealPath(event.newPath), + getRealPath(event.initialPath) + ]) + if (!realNewPath || !realOldPath || isOpenInEditor(realNewPath) || isOpenInEditor(realOldPath)) return + + eventCallback([{type: 'renamed', path: realNewPath, oldPath: realOldPath}]) + })) + } + + async stop () { + this.subs && this.subs.dispose() + } +} + +// Private: Implement a native watcher by translating events from an NSFW watcher. +class NSFWBackend { + async start (rootPath, eventCallback, errorCallback) { + const handler = events => { + eventCallback(events.map(event => { + const type = ACTION_MAP.get(event.action) || `unexpected (${event.action})` + const payload = {type} + + if (event.file) { + payload.path = path.join(event.directory, event.file) + } else { + payload.oldPath = path.join(event.directory, event.oldFile) + payload.path = path.join(event.directory, event.newFile) + } + + return payload + })) + } + + this.watcher = await nsfw( + rootPath, + handler, + {debounceMS: 100, errorCallback} + ) + + await this.watcher.start() + } + + stop () { + return this.watcher.stop() + } +} + +// Private: Map configuration settings from the feature flag to backend implementations. +const BACKENDS = { + atom: AtomBackend, + native: NSFWBackend +} + +// Private: the backend implementation to fall back to if the config setting is invalid. +const DEFAULT_BACKEND = BACKENDS.nsfw + // Private: Interface with and normalize events from a native OS filesystem watcher. class NativeWatcher { @@ -32,9 +170,39 @@ class NativeWatcher { constructor (normalizedPath) { this.normalizedPath = normalizedPath this.emitter = new Emitter() + this.subs = new CompositeDisposable() - this.watcher = null + this.backend = null this.state = WATCHER_STATE.STOPPED + + this.onEvents = this.onEvents.bind(this) + this.onError = this.onError.bind(this) + + this.subs.add(atom.config.onDidChange('core.fileSystemWatcher', async () => { + if (this.state === WATCHER_STATE.STARTING) { + // Wait for this watcher to finish starting. + await new Promise(resolve => { + const sub = this.onDidStart(() => { + sub.dispose() + resolve() + }) + }) + } + + // Re-read the config setting in case it's changed again while we were waiting for the watcher + // to start. + const Backend = this.getCurrentBackend() + if (this.state === WATCHER_STATE.RUNNING && !(this.backend instanceof Backend)) { + await this.stop() + await this.start() + } + })) + } + + // Private: Read the `core.fileSystemWatcher` setting to determine the filesystem backend to use. + getCurrentBackend () { + const setting = atom.config.get('core.fileSystemWatcher') + return BACKENDS[setting] || DEFAULT_BACKEND } // Private: Begin watching for filesystem events. @@ -46,16 +214,10 @@ class NativeWatcher { } this.state = WATCHER_STATE.STARTING - this.watcher = await nsfw( - this.normalizedPath, - this.onEvents.bind(this), - { - debounceMS: 100, - errorCallback: this.onError.bind(this) - } - ) + const Backend = this.getCurrentBackend() - await this.watcher.start() + this.backend = new Backend() + await this.backend.start(this.normalizedPath, this.onEvents, this.onError) this.state = WATCHER_STATE.RUNNING this.emitter.emit('did-start') @@ -137,7 +299,7 @@ class NativeWatcher { this.state = WATCHER_STATE.STOPPING this.emitter.emit('will-stop') - await this.watcher.stop() + await this.backend.stop() this.state = WATCHER_STATE.STOPPED this.emitter.emit('did-stop') @@ -153,19 +315,7 @@ class NativeWatcher { // // * `events` An Array of filesystem events. onEvents (events) { - this.emitter.emit('did-change', events.map(event => { - const type = ACTION_MAP.get(event.action) || `unexpected (${event.action})` - const payload = {type} - - if (event.file) { - payload.path = path.join(event.directory, event.file) - } else { - payload.oldPath = path.join(event.directory, event.oldFile) - payload.path = path.join(event.directory, event.newFile) - } - - return payload - })) + this.emitter.emit('did-change', events) } // Private: Callback function invoked by the native watcher when an error occurs. From 095f6da379168f7c8676f1f45fe0237a39984c91 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 18:15:39 -0400 Subject: [PATCH 77/94] Default to the appveyor home dir --- script/test | 2 +- spec/project-spec.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/test b/script/test index 5aa3dd2b8..0146123a1 100755 --- a/script/test +++ b/script/test @@ -71,7 +71,7 @@ function runCoreRenderProcessTests (callback) { const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) cp.on('error', error => { try { - const projectSpecLog = fs.readFileSync(path.join(process.env.HOME, 'project-spec.log'), {encoding: 'utf8'}) + const projectSpecLog = fs.readFileSync(path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) console.log(`project-spec log:\n${projectSpecLog}\n`) } catch (e) { console.error(`Unable to open log file:\n${e.stack}`) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 97f1f0417..6e5a8c048 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -7,7 +7,7 @@ path = require 'path' GitRepository = require '../src/git-repository' logToFile = (text) -> - fs.appendFileSync path.join(process.env.HOME, 'project-spec.log'), text + fs.appendFileSync path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), text describe "Project", -> beforeEach -> From a84694fac114648b6e1c5b70730a2106b00eeabf Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 18:21:11 -0400 Subject: [PATCH 78/94] Stop watchers in an afterEach block --- spec/project-spec.coffee | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 6e5a8c048..66c5417a1 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -4,6 +4,7 @@ Project = require '../src/project' fs = require 'fs-plus' path = require 'path' {Directory} = require 'pathwatcher' +{stopAllWatchers} = require '../src/path-watcher' GitRepository = require '../src/git-repository' logToFile = (text) -> @@ -17,13 +18,8 @@ describe "Project", -> waits(1) afterEach -> - waitsForPromise -> - new Promise (resolve, reject) -> - temp.cleanup (err) -> - if err? - reject(err) - else - resolve() + waitsForPromise -> stopAllWatchers() + runs -> temp.cleanupSync() describe "serialization", -> deserializedProject = null From 7b61d4f62f150ede30a57ed84ce93846d96d242a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 18:21:24 -0400 Subject: [PATCH 79/94] Log a few more things --- spec/project-spec.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 66c5417a1..02cd31875 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -560,6 +560,7 @@ describe "Project", -> beforeEach -> sub = atom.project.onDidChangeFiles (incoming) -> + logToFile "Events received: #{require('util').inspect incoming}" events.push incoming... checkCallback() @@ -597,6 +598,7 @@ describe "Project", -> expect(atom.project.watchersByPath[dirTwo]).toEqual undefined logToFile "Watching #{dirOne} but not #{dirTwo}\n" + logToFile "Writing #{fileOne}, #{fileTwo}, #{fileThree}" fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" fs.writeFileSync fileOne, "one\n" From dc9cb76fa4b6dc10471191b2af6758e0f2c49f75 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 19:07:52 -0400 Subject: [PATCH 80/94] tfw your diagnostic tests don't even run because of a linter error --- script/test | 2 +- spec/project-spec.coffee | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/script/test b/script/test index 0146123a1..f02d26214 100755 --- a/script/test +++ b/script/test @@ -71,7 +71,7 @@ function runCoreRenderProcessTests (callback) { const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) cp.on('error', error => { try { - const projectSpecLog = fs.readFileSync(path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) + const projectSpecLog = fs.readFileSync(path.join(process.env.HOME or 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) console.log(`project-spec log:\n${projectSpecLog}\n`) } catch (e) { console.error(`Unable to open log file:\n${e.stack}`) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 02cd31875..91c902e2d 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -8,7 +8,7 @@ path = require 'path' GitRepository = require '../src/git-repository' logToFile = (text) -> - fs.appendFileSync path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), text + fs.appendFileSync path.join(process.env.HOME or 'C:\\Users\\appveyor', 'project-spec.log'), text describe "Project", -> beforeEach -> From 08a7fab4f917b53cff604543ef067ad1d5480547 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 19:49:50 -0400 Subject: [PATCH 81/94] Grrr --- script/test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/test b/script/test index f02d26214..0146123a1 100755 --- a/script/test +++ b/script/test @@ -71,7 +71,7 @@ function runCoreRenderProcessTests (callback) { const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) cp.on('error', error => { try { - const projectSpecLog = fs.readFileSync(path.join(process.env.HOME or 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) + const projectSpecLog = fs.readFileSync(path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) console.log(`project-spec log:\n${projectSpecLog}\n`) } catch (e) { console.error(`Unable to open log file:\n${e.stack}`) From 3b57d2a259fc18984d8bcae0323ecf28bc7fda9a Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 3 Aug 2017 20:29:01 -0400 Subject: [PATCH 82/94] Let's see if we're still green without diagnostics! --- script/test | 3 +-- spec/project-spec.coffee | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/script/test b/script/test index 0146123a1..b61b1aca4 100755 --- a/script/test +++ b/script/test @@ -162,8 +162,7 @@ function testSuitesForPlatform (platform) { case 'darwin': return [runCoreMainProcessTests, runCoreRenderProcessTests, runBenchmarkTests].concat(packageTestSuites) case 'win32': - // return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] - return (process.arch === 'x64') ? [runCoreRenderProcessTests] : [] + return (process.arch === 'x64') ? [runCoreMainProcessTests, runCoreRenderProcessTests] : [runCoreMainProcessTests] case 'linux': return [runCoreMainProcessTests] default: diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index 91c902e2d..aa074bbe2 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -7,9 +7,6 @@ path = require 'path' {stopAllWatchers} = require '../src/path-watcher' GitRepository = require '../src/git-repository' -logToFile = (text) -> - fs.appendFileSync path.join(process.env.HOME or 'C:\\Users\\appveyor', 'project-spec.log'), text - describe "Project", -> beforeEach -> atom.project.setPaths([atom.project.getDirectories()[0]?.resolve('dir')]) @@ -560,7 +557,6 @@ describe "Project", -> beforeEach -> sub = atom.project.onDidChangeFiles (incoming) -> - logToFile "Events received: #{require('util').inspect incoming}" events.push incoming... checkCallback() @@ -589,16 +585,13 @@ describe "Project", -> dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') - logToFile "Setting project paths to #{dirOne}\n" atom.project.setPaths([dirOne]) waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() runs -> expect(atom.project.watchersByPath[dirTwo]).toEqual undefined - logToFile "Watching #{dirOne} but not #{dirTwo}\n" - logToFile "Writing #{fileOne}, #{fileTwo}, #{fileThree}" fs.writeFileSync fileThree, "three\n" fs.writeFileSync fileTwo, "two\n" fs.writeFileSync fileOne, "one\n" @@ -606,7 +599,6 @@ describe "Project", -> waitsForPromise -> waitForEvents [fileOne, fileTwo] runs -> - logToFile "Events seen:\n#{require('util').inspect events}\n" expect(events.some (event) -> event.path is fileThree).toBeFalsy() describe ".onDidAddBuffer()", -> From ffb3b0b4624ce6a52e609d5242a085d57334fc27 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 10:37:01 -0400 Subject: [PATCH 83/94] Missed the logfile reporting --- script/test | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/script/test b/script/test index b61b1aca4..2f22b1e0a 100755 --- a/script/test +++ b/script/test @@ -69,15 +69,7 @@ function runCoreRenderProcessTests (callback) { console.log('Executing core render process tests'.bold.green) const cp = childProcess.spawn(executablePath, testArguments, {stdio: 'inherit', env: testEnv}) - cp.on('error', error => { - try { - const projectSpecLog = fs.readFileSync(path.join(process.env.HOME || 'C:\\Users\\appveyor', 'project-spec.log'), {encoding: 'utf8'}) - console.log(`project-spec log:\n${projectSpecLog}\n`) - } catch (e) { - console.error(`Unable to open log file:\n${e.stack}`) - } - callback(error) - }) + cp.on('error', error => { callback(error) }) cp.on('close', exitCode => { callback(null, exitCode) }) } From ea91723b36735b2ce042e4f0dbb26e166cd59b3c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 10:37:41 -0400 Subject: [PATCH 84/94] Only deal with watcher stopping in the onDidChangeFiles spec --- spec/project-spec.coffee | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/project-spec.coffee b/spec/project-spec.coffee index aa074bbe2..059208cbd 100644 --- a/spec/project-spec.coffee +++ b/spec/project-spec.coffee @@ -14,10 +14,6 @@ describe "Project", -> # Wait for project's service consumers to be asynchronously added waits(1) - afterEach -> - waitsForPromise -> stopAllWatchers() - runs -> temp.cleanupSync() - describe "serialization", -> deserializedProject = null @@ -585,8 +581,10 @@ describe "Project", -> dirTwo = temp.mkdirSync('atom-spec-project-two') fileThree = path.join(dirTwo, 'file-three.txt') - atom.project.setPaths([dirOne]) + # Ensure that all preexisting watchers are stopped + waitsForPromise -> stopAllWatchers() + runs -> atom.project.setPaths([dirOne]) waitsForPromise -> atom.project.watchersByPath[dirOne].getStartPromise() runs -> From 662e2aaf06a170004ebffc7c3d4fa17546b4ae12 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 12:55:25 -0400 Subject: [PATCH 85/94] Revisit a bunch of documentation. --- src/path-watcher.js | 42 +++++++++++++++++++++++++++++++++++------- src/project.coffee | 21 +++++++++++++++++---- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index a12bb9944..bbe412aad 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -326,14 +326,44 @@ class NativeWatcher { } } -// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Construct these by calling -// {watchPath}. +// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Don't instantiate these +// directly. Instead, construct them by calling `watchPath`, or use them indirectly through {Project::onDidChangeFiles} +// instead if you only need to watch for events within active project directories. // -// Maybe PathWatchers may be backed by the same native watcher to conserve operation system resources. Native watchers are -// started when at least one subscription is registered, and stopped when all subscriptions are disposed. +// Multiple PathWatchers may be backed by a single native watcher to conserve operation system resources. Native +// watchers are started when at least one subscription is registered, and stopped when all subscriptions are disposed. // -// Acts as a {Disposable}. +// Compatible with the {Disposable} protocol. When disposed, no more events will be delivered. // +// Arguments accepted by `watchPath`: +// +// * `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. +// * `options` Control the watcher's behavior. Currently a placeholder. +// * `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. +// * `events` {Array} of objects that describe the events that have occurred. +// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, +// or `"renamed"`. +// * `path` {String} containing the absolute path to the filesystem entry that was acted upon. +// * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. +// +// ```js +// const {watchPath} = require('atom') +// +// const disposable = watchPath('/var/log', {}, events => { +// console.log(`Received batch of ${events.length} events.`) +// for (const event of events) { +// console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" +// console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched +// if (event.type === 'renamed') { +// console.log(`.. renamed from: ${event.oldPath}`) +// } +// } +// }) +// +// // Immediately stop receiving filesystem events. If this is the last watcher, asynchronously release any OS +// // resources required to subscribe to these events. +// disposable.dispose() +// ``` class PathWatcher { // Private: Instantiate a new PathWatcher. Call {watchPath} instead. @@ -557,8 +587,6 @@ class PathWatcherManager { // // * `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. // * `options` Control the watcher's behavior. -// * `recursive` If true, passing the path to a directory will recursively watch all changes beneath that -// directory. If false, only the file or directory itself will be watched. // * `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. // * `events` {Array} of objects that describe the events that have occurred. // * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, diff --git a/src/project.coffee b/src/project.coffee index a28fc34a4..cdd09fa1b 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -117,10 +117,24 @@ class Project extends Model callback(buffer) for buffer in @getBuffers() @onDidAddBuffer callback - # Public: Invoke the given callback when any filesystem change occurs within an open + # Public: Invoke a callback when a filesystem change occurs within any open # project path. # - # To watch paths outside of open projects, use the {watchPaths} function. + # ```js + # const disposable = atom.project.onDidChangeFiles(events => { + # for (const event of events) { + # console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" + # console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched + # if (event.type === 'renamed') { + # console.log(`.. renamed from: ${event.oldPath}`) + # } + # } + # } + # + # disposable.dispose() + # ``` + # + # To watch paths outside of open projects, use the [watchPaths]{PathWatcher} function instead. # # * `callback` {Function} to be called with batches of filesystem events reported by # the operating system. @@ -132,8 +146,7 @@ class Project extends Model # * `oldPath` For rename events, {String} containing the filesystem entry's # former absolute path. # - # Returns a {PathWatcher} that may be used like a {Disposable} to manage - # this event subscription. + # Returns a {Disposable} to manage this event subscription. onDidChangeFiles: (callback) -> @emitter.on 'did-change-files', callback From 97ffe46247d906d38ca13016d42f45a980fece9b Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 14:08:29 -0400 Subject: [PATCH 86/94] Consistently use require('temp').track() --- spec/main-process/atom-application.test.js | 3 ++- spec/title-bar-spec.js | 2 +- src/main-process/start.js | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index d4a913859..1e5ae0a86 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -508,7 +508,8 @@ describe('AtomApplication', function () { } function makeTempDir (name) { - return fs.realpathSync(require('temp').mkdirSync(name)) + const temp = require('temp').track() + return fs.realpathSync(temp.mkdirSync(name)) } let channelIdCounter = 0 diff --git a/spec/title-bar-spec.js b/spec/title-bar-spec.js index c0cb806b5..b219a5819 100644 --- a/spec/title-bar-spec.js +++ b/spec/title-bar-spec.js @@ -1,5 +1,5 @@ const TitleBar = require('../src/title-bar') -const temp = require('temp') +const temp = require('temp').track() describe('TitleBar', () => { it('updates its title when document.title changes', () => { diff --git a/src/main-process/start.js b/src/main-process/start.js index fae78a07e..9670e67b6 100644 --- a/src/main-process/start.js +++ b/src/main-process/start.js @@ -1,7 +1,7 @@ const {app} = require('electron') const nslog = require('nslog') const path = require('path') -const temp = require('temp') +const temp = require('temp').track() const parseCommandLine = require('./parse-command-line') const startCrashReporter = require('../crash-reporter-start') const atomPaths = require('../atom-paths') From dc9fe2525514d4fbaadccc7b4e920ff84224a4fd Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 14:18:43 -0400 Subject: [PATCH 87/94] Wrap temp.cleanupSync() calls in try-catch blocks --- spec/atom-environment-spec.coffee | 3 ++- spec/atom-paths-spec.js | 6 +++++- spec/babel-spec.coffee | 3 ++- spec/command-installer-spec.coffee | 3 ++- spec/compile-cache-spec.coffee | 3 ++- spec/default-directory-provider-spec.coffee | 3 ++- spec/git-repository-provider-spec.coffee | 3 ++- spec/grammars-spec.coffee | 3 ++- spec/main-process/file-recovery-service.test.js | 6 +++++- spec/module-cache-spec.coffee | 3 ++- spec/package-manager-spec.coffee | 3 ++- spec/squirrel-update-spec.coffee | 3 ++- spec/style-manager-spec.js | 6 +++++- spec/theme-manager-spec.coffee | 3 ++- spec/update-process-env-spec.js | 6 +++++- spec/workspace-element-spec.js | 8 +++++++- spec/workspace-spec.js | 8 +++++++- 17 files changed, 56 insertions(+), 17 deletions(-) diff --git a/spec/atom-environment-spec.coffee b/spec/atom-environment-spec.coffee index 5d269a3bb..8a3e4e0fb 100644 --- a/spec/atom-environment-spec.coffee +++ b/spec/atom-environment-spec.coffee @@ -6,7 +6,8 @@ StorageFolder = require '../src/storage-folder' describe "AtomEnvironment", -> afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() describe 'window sizing methods', -> describe '::getPosition and ::setPosition', -> diff --git a/spec/atom-paths-spec.js b/spec/atom-paths-spec.js index 3e2da4760..f4bbbf2b7 100644 --- a/spec/atom-paths-spec.js +++ b/spec/atom-paths-spec.js @@ -86,7 +86,11 @@ describe("AtomPaths", () => { afterEach(() => { delete process.env.ATOM_HOME fs.removeSync(electronUserDataPath) - temp.cleanupSync() + try { + temp.cleanupSync() + } catch (e) { + // Ignore + } app.setPath('userData', defaultElectronUserDataPath) }) diff --git a/spec/babel-spec.coffee b/spec/babel-spec.coffee index 070ad7a0b..400e5c03e 100644 --- a/spec/babel-spec.coffee +++ b/spec/babel-spec.coffee @@ -19,7 +19,8 @@ describe "Babel transpiler support", -> afterEach -> CompileCache.setCacheDirectory(originalCacheDir) - temp.cleanupSync() + try + temp.cleanupSync() describe 'when a .js file starts with /** @babel */;', -> it "transpiles it using babel", -> diff --git a/spec/command-installer-spec.coffee b/spec/command-installer-spec.coffee index f0994fc08..dfd25a1df 100644 --- a/spec/command-installer-spec.coffee +++ b/spec/command-installer-spec.coffee @@ -21,7 +21,8 @@ describe "CommandInstaller on #darwin", -> spyOn(CommandInstaller::, 'getInstallDirectory').andReturn(installationPath) afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() it "shows an error dialog when installing commands interactively fails", -> appDelegate = jasmine.createSpyObj("appDelegate", ["confirm"]) diff --git a/spec/compile-cache-spec.coffee b/spec/compile-cache-spec.coffee index 13db6a055..084f87e70 100644 --- a/spec/compile-cache-spec.coffee +++ b/spec/compile-cache-spec.coffee @@ -23,7 +23,8 @@ describe 'CompileCache', -> afterEach -> CompileCache.setAtomHomeDirectory(process.env.ATOM_HOME) CSON.setCacheDir(CompileCache.getCacheDirectory()) - temp.cleanupSync() + try + temp.cleanupSync() describe 'addPathToCache(filePath, atomHome)', -> describe 'when the given file is plain javascript', -> diff --git a/spec/default-directory-provider-spec.coffee b/spec/default-directory-provider-spec.coffee index bf23195cf..35e40618c 100644 --- a/spec/default-directory-provider-spec.coffee +++ b/spec/default-directory-provider-spec.coffee @@ -10,7 +10,8 @@ describe "DefaultDirectoryProvider", -> tmp = temp.mkdirSync('atom-spec-default-dir-provider') afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() describe ".directoryForURISync(uri)", -> it "returns a Directory with a path that matches the uri", -> diff --git a/spec/git-repository-provider-spec.coffee b/spec/git-repository-provider-spec.coffee index 6c6a7b4b9..16ccf8938 100644 --- a/spec/git-repository-provider-spec.coffee +++ b/spec/git-repository-provider-spec.coffee @@ -12,7 +12,8 @@ describe "GitRepositoryProvider", -> provider = new GitRepositoryProvider(atom.project, atom.config, atom.confirm) afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() describe ".repositoryForDirectory(directory)", -> describe "when specified a Directory with a Git repository", -> diff --git a/spec/grammars-spec.coffee b/spec/grammars-spec.coffee index 47198a124..7d4754397 100644 --- a/spec/grammars-spec.coffee +++ b/spec/grammars-spec.coffee @@ -24,7 +24,8 @@ describe "the `grammars` global", -> afterEach -> atom.packages.deactivatePackages() atom.packages.unloadPackages() - temp.cleanupSync() + try + temp.cleanupSync() describe ".selectGrammar(filePath)", -> it "always returns a grammar", -> diff --git a/spec/main-process/file-recovery-service.test.js b/spec/main-process/file-recovery-service.test.js index 862b7f428..618c30ab0 100644 --- a/spec/main-process/file-recovery-service.test.js +++ b/spec/main-process/file-recovery-service.test.js @@ -16,7 +16,11 @@ describe("FileRecoveryService", () => { }) afterEach(() => { - temp.cleanupSync() + try { + temp.cleanupSync() + } catch (e) { + // Ignore + } }) describe("when no crash happens during a save", () => { diff --git a/spec/module-cache-spec.coffee b/spec/module-cache-spec.coffee index 1627ec776..693fd6634 100644 --- a/spec/module-cache-spec.coffee +++ b/spec/module-cache-spec.coffee @@ -9,7 +9,8 @@ describe 'ModuleCache', -> spyOn(Module, '_findPath').andCallThrough() afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() it 'resolves Electron module paths without hitting the filesystem', -> builtins = ModuleCache.cache.builtins diff --git a/spec/package-manager-spec.coffee b/spec/package-manager-spec.coffee index 62cc067e9..2d448ac7c 100644 --- a/spec/package-manager-spec.coffee +++ b/spec/package-manager-spec.coffee @@ -17,7 +17,8 @@ describe "PackageManager", -> spyOn(ModuleCache, 'add') afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() describe "::getApmPath()", -> it "returns the path to the apm command", -> diff --git a/spec/squirrel-update-spec.coffee b/spec/squirrel-update-spec.coffee index 2838be297..fe0fa7479 100644 --- a/spec/squirrel-update-spec.coffee +++ b/spec/squirrel-update-spec.coffee @@ -37,7 +37,8 @@ describe "Windows Squirrel Update", -> WinShell.folderBackgroundContextMenu = new FakeShellOption() afterEach -> - temp.cleanupSync() + try + temp.cleanupSync() it "quits the app on all squirrel events", -> app = quit: jasmine.createSpy('quit') diff --git a/spec/style-manager-spec.js b/spec/style-manager-spec.js index e6b8acae6..641c93709 100644 --- a/spec/style-manager-spec.js +++ b/spec/style-manager-spec.js @@ -15,7 +15,11 @@ describe('StyleManager', () => { }) afterEach(() => { - temp.cleanupSync() + try { + temp.cleanupSync() + } catch (e) { + // Do nothing + } }) describe('::addStyleSheet(source, params)', () => { diff --git a/spec/theme-manager-spec.coffee b/spec/theme-manager-spec.coffee index 42ed9be6d..5d2912f5b 100644 --- a/spec/theme-manager-spec.coffee +++ b/spec/theme-manager-spec.coffee @@ -9,7 +9,8 @@ describe "atom.themes", -> afterEach -> atom.themes.deactivateThemes() - temp.cleanupSync() + try + temp.cleanupSync() describe "theme getters and setters", -> beforeEach -> diff --git a/spec/update-process-env-spec.js b/spec/update-process-env-spec.js index f730ae632..e5a1cfd9c 100644 --- a/spec/update-process-env-spec.js +++ b/spec/update-process-env-spec.js @@ -28,7 +28,11 @@ describe('updateProcessEnv(launchEnv)', function () { } process.env = originalProcessEnv process.platform = originalProcessPlatform - temp.cleanupSync() + try { + temp.cleanupSync() + } catch (e) { + // Do nothing + } }) describe('when the launch environment appears to come from a shell', function () { diff --git a/spec/workspace-element-spec.js b/spec/workspace-element-spec.js index aa5430c88..2e37d9903 100644 --- a/spec/workspace-element-spec.js +++ b/spec/workspace-element-spec.js @@ -9,7 +9,13 @@ const {Disposable} = require('event-kit') const {it, fit, ffit, fffit, beforeEach, afterEach} = require('./async-spec-helpers') describe('WorkspaceElement', () => { - afterEach(() => { temp.cleanupSync() }) + afterEach(() => { + try { + temp.cleanupSync() + } catch (e) { + // Do nothing + } + }) describe('when the workspace element is focused', () => { it('transfers focus to the active pane', () => { diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index 489b45120..bdd5677c8 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -25,7 +25,13 @@ describe('Workspace', () => { waitsForPromise(() => atom.workspace.itemLocationStore.clear()) }) - afterEach(() => temp.cleanupSync()) + afterEach(() => { + try { + temp.cleanupSync() + } catch (e) { + // Do nothing + } + }) function simulateReload() { waitsForPromise(() => { From 23100536378e54b6ebdea907c38e381301adf7fb Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 14:34:06 -0400 Subject: [PATCH 88/94] Reword Project.onDidChangeFiles documentation --- src/project.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/project.coffee b/src/project.coffee index cdd09fa1b..47dc291c7 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -117,7 +117,7 @@ class Project extends Model callback(buffer) for buffer in @getBuffers() @onDidAddBuffer callback - # Public: Invoke a callback when a filesystem change occurs within any open + # Extended: Invoke a callback when a filesystem change occurs within any open # project path. # # ```js @@ -134,11 +134,11 @@ class Project extends Model # disposable.dispose() # ``` # - # To watch paths outside of open projects, use the [watchPaths]{PathWatcher} function instead. + # To watch paths outside of open projects, use the `watchPaths` function instead; see {PathWatcher}. # # * `callback` {Function} to be called with batches of filesystem events reported by # the operating system. - # * `events` An {Array} of objects the describe filesystem events. + # * `events` An {Array} of objects that describe a batch of filesystem events. # * `type` {String} describing the filesystem action that occurred. One of `"created"`, # `"modified"`, `"deleted"`, or `"renamed"`. # * `path` {String} containing the absolute path to the filesystem entry From 1d73f40d20d54b3cb368ea111ffb4c87f5e2733e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 16:23:16 -0400 Subject: [PATCH 89/94] :arrow_up: joanna --- script/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/package.json b/script/package.json index 350fc2b0e..52f0b6d55 100644 --- a/script/package.json +++ b/script/package.json @@ -15,7 +15,7 @@ "electron-winstaller": "2.6.2", "fs-extra": "0.30.0", "glob": "7.0.3", - "joanna": "0.0.8", + "joanna": "0.0.9", "klaw-sync": "^1.1.2", "legal-eagle": "0.14.0", "lodash.template": "4.4.0", From ca28f8ac48de02ee9c1f2cbe7c85ecd9401b1112 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 4 Aug 2017 16:30:43 -0400 Subject: [PATCH 90/94] Fussing with documentation --- src/path-watcher.js | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index bbe412aad..f1da920e1 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -335,25 +335,18 @@ class NativeWatcher { // // Compatible with the {Disposable} protocol. When disposed, no more events will be delivered. // -// Arguments accepted by `watchPath`: -// -// * `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. -// * `options` Control the watcher's behavior. Currently a placeholder. -// * `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. -// * `events` {Array} of objects that describe the events that have occurred. -// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, -// or `"renamed"`. -// * `path` {String} containing the absolute path to the filesystem entry that was acted upon. -// * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. -// // ```js // const {watchPath} = require('atom') // // const disposable = watchPath('/var/log', {}, events => { // console.log(`Received batch of ${events.length} events.`) // for (const event of events) { -// console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" -// console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched +// // "created", "modified", "deleted", "renamed" +// console.log(`Event action: ${event.type}`) +// +// // absolute path to the filesystem entry that was touched +// console.log(`Event path: ${event.path}`) +// // if (event.type === 'renamed') { // console.log(`.. renamed from: ${event.oldPath}`) // } @@ -364,6 +357,18 @@ class NativeWatcher { // // resources required to subscribe to these events. // disposable.dispose() // ``` +// +// `watchPath` accepts the following arguments: +// +// `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. +// `options` Control the watcher's behavior. Currently a placeholder. +// `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. +// * `events` {Array} of objects that describe the events that have occurred. +// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, +// or `"renamed"`. +// * `path` {String} containing the absolute path to the filesystem entry that was acted upon. +// * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. +// class PathWatcher { // Private: Instantiate a new PathWatcher. Call {watchPath} instead. @@ -440,8 +445,9 @@ class PathWatcher { // Private: Attach another {Function} to be called with each batch of filesystem events. See {watchPath} for the // spec of the callback's argument. // - // Returns a {Disposable} that will stop the underlying watcher when all callbacks mapped to it have been disposed. + // * `callback` {Function} to be called with each batch of filesystem events. // + // Returns a {Disposable} that will stop the underlying watcher when all callbacks mapped to it have been disposed. onDidChange (callback) { if (this.native) { const sub = this.native.onDidChange(events => this.onNativeEvents(events, callback)) @@ -464,8 +470,10 @@ class PathWatcher { // Extended: Invoke a {Function} when any errors related to this watcher are reported. // - // Returns a {Disposable}. + // * `callback` {Function} to be called when an error occurs. + // * `err` An {Error} describing the failure condition. // + // Returns a {Disposable}. onDidError (callback) { return this.emitter.on('did-error', callback) } From f623b03157f9fdd0a00e87e33224db2078e472fc Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 7 Aug 2017 09:31:50 -0400 Subject: [PATCH 91/94] Documentation touchups --- src/path-watcher.js | 38 ++++++++++++++++++++------------------ src/project.coffee | 8 ++++++-- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index f1da920e1..0294b6ec3 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -326,14 +326,15 @@ class NativeWatcher { } } -// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Don't instantiate these -// directly. Instead, construct them by calling `watchPath`, or use them indirectly through {Project::onDidChangeFiles} -// instead if you only need to watch for events within active project directories. +// Extended: Manage a subscription to filesystem events that occur beneath a root directory. Construct these by +// calling `watchPath`. To watch for events within active project directories, use {Project::onDidChangeFiles} +// instead. // -// Multiple PathWatchers may be backed by a single native watcher to conserve operation system resources. Native -// watchers are started when at least one subscription is registered, and stopped when all subscriptions are disposed. +// Multiple PathWatchers may be backed by a single native watcher to conserve operation system resources. // -// Compatible with the {Disposable} protocol. When disposed, no more events will be delivered. +// Call {::dispose} to stop receiving events and, if possible, release underlying resources. A PathWatcher may be +// added to a {CompositeDisposable} to manage its lifetime along with other {Disposable} resources like event +// subscriptions. // // ```js // const {watchPath} = require('atom') @@ -353,22 +354,22 @@ class NativeWatcher { // } // }) // -// // Immediately stop receiving filesystem events. If this is the last watcher, asynchronously release any OS -// // resources required to subscribe to these events. +// // Immediately stop receiving filesystem events. If this is the last +// // watcher, asynchronously release any OS resources required to +// // subscribe to these events. // disposable.dispose() // ``` // // `watchPath` accepts the following arguments: // // `rootPath` {String} specifies the absolute path to the root of the filesystem content to watch. -// `options` Control the watcher's behavior. Currently a placeholder. -// `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. -// * `events` {Array} of objects that describe the events that have occurred. -// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, -// or `"renamed"`. -// * `path` {String} containing the absolute path to the filesystem entry that was acted upon. -// * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. // +// `options` Control the watcher's behavior. Currently a placeholder. +// +// `eventCallback` {Function} to be called each time a batch of filesystem events is observed. Each event object has +// the keys: `type`, a {String} describing the filesystem action that occurred, one of `"created"`, `"modified"`, +// `"deleted"`, or `"renamed"`; `path`, a {String} containing the absolute path to the filesystem entry that was acted +// upon; for rename events only, `oldPath`, a {String} containing the filesystem entry's former absolute path. class PathWatcher { // Private: Instantiate a new PathWatcher. Call {watchPath} instead. @@ -433,7 +434,8 @@ class PathWatcher { // const watcher = watchPath(ROOT, {}, events => {}) // await watcher.getStartPromise() // fs.writeFile(FILE, 'contents\n', err => { - // // The watcher is listening and the event should be received asyncronously + // // The watcher is listening and the event should be + // // received asyncronously // } // }) // }) @@ -527,8 +529,8 @@ class PathWatcher { } } - // Extended: Unsubscribe all subscribers from filesystem events. The native watcher resources may take some time to - // be cleaned up, but the watcher will stop broadcasting events immediately. + // Extended: Unsubscribe all subscribers from filesystem events. Native resources will be release asynchronously, + // but this watcher will stop broadcasting events immediately. dispose () { for (const sub of this.changeCallbacks.values()) { sub.dispose() diff --git a/src/project.coffee b/src/project.coffee index 47dc291c7..4564e37bb 100644 --- a/src/project.coffee +++ b/src/project.coffee @@ -123,8 +123,12 @@ class Project extends Model # ```js # const disposable = atom.project.onDidChangeFiles(events => { # for (const event of events) { - # console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" - # console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched + # // "created", "modified", "deleted", or "renamed" + # console.log(`Event action: ${event.type}`) + # + # // absolute path to the filesystem entry that was touched + # console.log(`Event path: ${event.path}`) + # # if (event.type === 'renamed') { # console.log(`.. renamed from: ${event.oldPath}`) # } From f270402c6b4554c4067807e71bc3233a82d7a7e4 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 7 Aug 2017 10:03:36 -0400 Subject: [PATCH 92/94] s/type/action/, s/changed/modified/, s/added/created/ --- src/path-watcher.js | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index 0294b6ec3..add392d7e 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -7,10 +7,10 @@ const {Emitter, Disposable, CompositeDisposable} = require('event-kit') const nsfw = require('nsfw') const {NativeWatcherRegistry} = require('./native-watcher-registry') -// Private: Associate native watcher action type flags with descriptive String equivalents. +// Private: Associate native watcher action action flags with descriptive String equivalents. const ACTION_MAP = new Map([ - [nsfw.actions.MODIFIED, 'changed'], - [nsfw.actions.CREATED, 'added'], + [nsfw.actions.MODIFIED, 'modified'], + [nsfw.actions.CREATED, 'created'], [nsfw.actions.DELETED, 'deleted'], [nsfw.actions.RENAMED, 'renamed'] ]) @@ -43,8 +43,8 @@ class AtomBackend { return } - const announce = (type, oldPath) => { - const payload = {type, path: realPath} + const announce = (action, oldPath) => { + const payload = {action, path: realPath} if (oldPath) payload.oldPath = oldPath eventCallback([payload]) } @@ -93,14 +93,14 @@ class AtomBackend { const realPath = await getRealPath(event.path) if (!realPath) return - eventCallback([{type: 'added', path: realPath}]) + eventCallback([{action: 'added', path: realPath}]) })) this.subs.add(treeView.onEntryDeleted(async event => { const realPath = await getRealPath(event.path) if (!realPath || isOpenInEditor(realPath)) return - eventCallback([{type: 'deleted', path: realPath}]) + eventCallback([{action: 'deleted', path: realPath}]) })) this.subs.add(treeView.onEntryMoved(async event => { @@ -110,7 +110,7 @@ class AtomBackend { ]) if (!realNewPath || !realOldPath || isOpenInEditor(realNewPath) || isOpenInEditor(realOldPath)) return - eventCallback([{type: 'renamed', path: realNewPath, oldPath: realOldPath}]) + eventCallback([{action: 'renamed', path: realNewPath, oldPath: realOldPath}]) })) } @@ -124,8 +124,8 @@ class NSFWBackend { async start (rootPath, eventCallback, errorCallback) { const handler = events => { eventCallback(events.map(event => { - const type = ACTION_MAP.get(event.action) || `unexpected (${event.action})` - const payload = {type} + const action = ACTION_MAP.get(event.action) || `unexpected (${event.action})` + const payload = {action} if (event.file) { payload.path = path.join(event.directory, event.file) @@ -343,12 +343,12 @@ class NativeWatcher { // console.log(`Received batch of ${events.length} events.`) // for (const event of events) { // // "created", "modified", "deleted", "renamed" -// console.log(`Event action: ${event.type}`) +// console.log(`Event action: ${event.action}`) // // // absolute path to the filesystem entry that was touched // console.log(`Event path: ${event.path}`) // -// if (event.type === 'renamed') { +// if (event.action === 'renamed') { // console.log(`.. renamed from: ${event.oldPath}`) // } // } @@ -367,7 +367,7 @@ class NativeWatcher { // `options` Control the watcher's behavior. Currently a placeholder. // // `eventCallback` {Function} to be called each time a batch of filesystem events is observed. Each event object has -// the keys: `type`, a {String} describing the filesystem action that occurred, one of `"created"`, `"modified"`, +// the keys: `action`, a {String} describing the filesystem action that occurred, one of `"created"`, `"modified"`, // `"deleted"`, or `"renamed"`; `path`, a {String} containing the absolute path to the filesystem entry that was acted // upon; for rename events only, `oldPath`, a {String} containing the filesystem entry's former absolute path. class PathWatcher { @@ -599,8 +599,8 @@ class PathWatcherManager { // * `options` Control the watcher's behavior. // * `eventCallback` {Function} or other callable to be called each time a batch of filesystem events is observed. // * `events` {Array} of objects that describe the events that have occurred. -// * `type` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, `"deleted"`, -// or `"renamed"`. +// * `action` {String} describing the filesystem action that occurred. One of `"created"`, `"modified"`, +// `"deleted"`, or `"renamed"`. // * `path` {String} containing the absolute path to the filesystem entry that was acted upon. // * `oldPath` For rename events, {String} containing the filesystem entry's former absolute path. // @@ -613,9 +613,11 @@ class PathWatcherManager { // const disposable = watchPath('/var/log', {}, events => { // console.log(`Received batch of ${events.length} events.`) // for (const event of events) { -// console.log(`Event action: ${event.type}`) // "created", "modified", "deleted", "renamed" -// console.log(`Event path: ${event.path}`) // absolute path to the filesystem entry that was touched -// if (event.type === 'renamed') { +// // "created", "modified", "deleted", "renamed" +// console.log(`Event action: ${event.action}`) +// // absolute path to the filesystem entry that was touched +// console.log(`Event path: ${event.path}`) +// if (event.action === 'renamed') { // console.log(`.. renamed from: ${event.oldPath}`) // } // } From fc7ecb76d1bea7d7516a81fc19682b7a86f35cc2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Mon, 7 Aug 2017 11:13:03 -0400 Subject: [PATCH 93/94] :burn: double word --- src/path-watcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path-watcher.js b/src/path-watcher.js index add392d7e..001b17818 100644 --- a/src/path-watcher.js +++ b/src/path-watcher.js @@ -7,7 +7,7 @@ const {Emitter, Disposable, CompositeDisposable} = require('event-kit') const nsfw = require('nsfw') const {NativeWatcherRegistry} = require('./native-watcher-registry') -// Private: Associate native watcher action action flags with descriptive String equivalents. +// Private: Associate native watcher action flags with descriptive String equivalents. const ACTION_MAP = new Map([ [nsfw.actions.MODIFIED, 'modified'], [nsfw.actions.CREATED, 'created'], From d920d20c2f30bf30c2d12567eb0d9141530a6090 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Tue, 8 Aug 2017 12:42:01 -0400 Subject: [PATCH 94/94] :burn: diagnostic code --- script/lib/generate-startup-snapshot.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/script/lib/generate-startup-snapshot.js b/script/lib/generate-startup-snapshot.js index 6fe7b519c..471bd1201 100644 --- a/script/lib/generate-startup-snapshot.js +++ b/script/lib/generate-startup-snapshot.js @@ -23,7 +23,7 @@ module.exports = function (packagedAppPath) { process.stdout.write(`Generating snapshot script at "${snapshotScriptPath}" (${++processedFiles})`) const relativePath = path.relative(baseDirPath, modulePath) - const result = ( + return ( modulePath.endsWith('.node') || coreModules.has(modulePath) || (relativePath.startsWith(path.join('..', 'src')) && relativePath.endsWith('-element.js')) || @@ -70,8 +70,6 @@ module.exports = function (packagedAppPath) { relativePath === path.join('..', 'node_modules', 'tmp', 'lib', 'tmp.js') || relativePath === path.join('..', 'node_modules', 'tree-view', 'node_modules', 'minimatch', 'minimatch.js') ) - fs.appendFileSync('snapshot-files.txt', `${relativePath} = ${result}\n`) - return result } }).then((snapshotScript) => { fs.writeFileSync(snapshotScriptPath, snapshotScript)