Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix renames of files that existed before we started watching on macOS #48

Merged
merged 1 commit into from
Feb 14, 2021
Merged

Fix renames of files that existed before we started watching on macOS #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 8 additions & 13 deletions src/macos/FSEventsBackend.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,29 +71,24 @@ void FSEventsCallback(
list->update(paths[i]);
} else {
// If multiple flags were set, then we need to call `stat` to determine if the file really exists.
// We also check our local cache of recently modified files to see if we knew about it before.
// This helps disambiguate creates, updates, and deletes.
auto existed = !since && state->tree->find(paths[i]);
struct stat file;
if (stat(paths[i], &file)) {
// File does not exist now. If it existed before in our local cache,
// or was removed/renamed and we don't have a cache (querying since a snapshot)
// then the file was probably removed. This is not exact since the flags set by
// fsevents get coalesced together (e.g. created & deleted), so there is no way to
// File does not exist, so we have to assume it was removed. This is not exact since the
// flags set by fsevents get coalesced together (e.g. created & deleted), so there is no way to
// know whether the create and delete both happened since our snapshot (in which case
// we'd rather ignore this event completely). This will result in some extra delete events
// being emitted for files we don't know about, but that is the best we can do.
if (existed || (since && (isRemoved || isRenamed))) {
state->tree->remove(paths[i]);
list->remove(paths[i]);
}

state->tree->remove(paths[i]);
list->remove(paths[i]);
continue;
}

// If the file was modified, and existed before, then this is an update, otherwise a create.
uint64_t mtime = CONVERT_TIME(file.st_birthtimespec);
if (isModified && (existed || mtime <= since)) {
uint64_t ctime = CONVERT_TIME(file.st_birthtimespec);
uint64_t mtime = CONVERT_TIME(file.st_mtimespec);
auto existed = !since && state->tree->find(paths[i]);
if (isModified && (existed || ctime <= since)) {
state->tree->update(paths[i], mtime);
list->update(paths[i]);
} else {
Expand Down
9 changes: 7 additions & 2 deletions test/since.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,20 @@ function testPrecision() {
const isSecondPrecision = testPrecision();

describe('since', () => {
const sleep = (ms = 20) => new Promise(resolve => setTimeout(resolve, ms));

before(async () => {
// wait for tmp dir to be created.
await sleep();
});

after(async () => {
try {
await fs.unlink(snapshotPath);
} catch (err) {}
});

backends.forEach(backend => {
const sleep = (ms = 10) => new Promise(resolve => setTimeout(resolve, ms));

describe(backend, () => {
describe('files', () => {
it('should emit when a file is created', async () => {
Expand Down
96 changes: 61 additions & 35 deletions test/watcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('watcher', () => {
.toString(31)
.slice(2)}`,
);
let ignoreDir, ignoreFile, sub;
let ignoreDir, ignoreFile, fileToRename, dirToRename, sub;

before(async () => {
tmpDir = path.join(
Expand All @@ -60,6 +60,10 @@ describe('watcher', () => {
fs.mkdirpSync(tmpDir);
ignoreDir = getFilename();
ignoreFile = getFilename();
fileToRename = getFilename();
dirToRename = getFilename();
fs.writeFileSync(fileToRename, 'hi');
fs.mkdirpSync(dirToRename);
await new Promise(resolve => setTimeout(resolve, 100));
sub = await watcher.subscribe(tmpDir, fn, {
backend,
Expand Down Expand Up @@ -103,6 +107,16 @@ describe('watcher', () => {
]);
});

it('should emit when an existing file is renamed', async () => {
let f2 = getFilename();
fs.rename(fileToRename, f2);
let res = await nextEvent();
assert.deepEqual(res, [
{type: 'delete', path: fileToRename},
{type: 'create', path: f2},
]);
});

it('should emit when a file is deleted', async () => {
let f = getFilename();
fs.writeFile(f, 'hello world');
Expand Down Expand Up @@ -137,6 +151,16 @@ describe('watcher', () => {
]);
});

it('should emit when an existing directory is renamed', async () => {
let f2 = getFilename();
fs.rename(dirToRename, f2);
let res = await nextEvent();
assert.deepEqual(res, [
{type: 'create', path: f2},
{type: 'delete', path: dirToRename},
]);
});

it('should emit when a directory is deleted', async () => {
let f = getFilename();
fs.mkdir(f);
Expand Down Expand Up @@ -321,17 +345,6 @@ describe('watcher', () => {
});

describe('rapid changes', () => {
it('should ignore files that are created and deleted rapidly', async () => {
let f1 = getFilename();
let f2 = getFilename();
await fs.writeFile(f1, 'hello world');
await fs.writeFile(f2, 'hello world');
fs.unlink(f2);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f1}]);
});

it('should coalese create and update events', async () => {
let f1 = getFilename();
await fs.writeFile(f1, 'hello world');
Expand All @@ -341,29 +354,42 @@ describe('watcher', () => {
assert.deepEqual(res, [{type: 'create', path: f1}]);
});

it('should coalese create and rename events', async () => {
let f1 = getFilename();
let f2 = getFilename();
await fs.writeFile(f1, 'hello world');
fs.rename(f1, f2);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f2}]);
});

it('should coalese multiple rename events', async () => {
let f1 = getFilename();
let f2 = getFilename();
let f3 = getFilename();
let f4 = getFilename();
await fs.writeFile(f1, 'hello world');
await fs.rename(f1, f2);
await fs.rename(f2, f3);
fs.rename(f3, f4);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f4}]);
});
if (backend !== 'fs-events') {
it('should ignore files that are created and deleted rapidly', async () => {
let f1 = getFilename();
let f2 = getFilename();
await fs.writeFile(f1, 'hello world');
await fs.writeFile(f2, 'hello world');
fs.unlink(f2);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f1}]);
});

it('should coalese create and rename events', async () => {
let f1 = getFilename();
let f2 = getFilename();
await fs.writeFile(f1, 'hello world');
fs.rename(f1, f2);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f2}]);
});

it('should coalese multiple rename events', async () => {
let f1 = getFilename();
let f2 = getFilename();
let f3 = getFilename();
let f4 = getFilename();
await fs.writeFile(f1, 'hello world');
await fs.rename(f1, f2);
await fs.rename(f2, f3);
fs.rename(f3, f4);

let res = await nextEvent();
assert.deepEqual(res, [{type: 'create', path: f4}]);
});
}

it('should coalese multiple update events', async () => {
let f1 = getFilename();
Expand Down