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.

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

feat: fs events #3452

Merged
merged 37 commits into from
Feb 21, 2020
Merged

feat: fs events #3452

merged 37 commits into from
Feb 21, 2020

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Dec 6, 2019

Prototype for file watching.

Uses https://docs.rs/notify/5.0.0-pre.2/notify/index.html

I used some code from @jcao219

cli/ops/fs.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju changed the title feat: fs events [WIP] feat: fs events Dec 6, 2019
@bartlomieju
Copy link
Member Author

After more digging I'm not sure fs events can solved elegantly at the moment. There's no way to turn this sync API into async one.

Once we land Tokio 0.2 there's spawn_blocking function that can be coupled with recv_timeout. So postponing this PR until #3387 is done

@bartlomieju bartlomieju reopened this Jan 23, 2020
@bartlomieju bartlomieju changed the title [WIP] feat: fs events feat: fs events Jan 23, 2020
@bartlomieju
Copy link
Member Author

Not sure how to test it yet... Docs state that contents of event depend on the platform.

Example output for fs_watch_test.ts on macOS:

{"type":{"create":{"kind":"folder"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"content"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file1.txt"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"content"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file2.txt"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file1.txt"],"attrs":{}}
{"type":{"remove":{"kind":"file"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"content"}},"paths":["/private/var/folders/4k/qds_txm54r5ffc909vqvgdkw0000gn/T/715c24c7/file1.txt"],"attrs":{}}

Also serialization output is really questionable in my opinion...

@ry
Copy link
Member

ry commented Jan 23, 2020

I think a platform dependent test would be fine. Probably a custom integration test would be the best way to test this. Create a temp dir, add file, touch it, etc - check events are issued in JS.

@bartlomieju
Copy link
Member Author

Test case:

  const testDir = await Deno.makeTempDir();
  const file1 = testDir + "/file1.txt";
  const file2 = testDir + "/file2.txt";

  const watcher = Deno.watch(testDir, { recursive: true });
  // start capturing events in the background
  await Deno.writeFile(file1, new Uint8Array([0, 1, 2]));
  await Deno.writeFile(file2, new Uint8Array([0, 1, 2]));
  await Deno.remove(file1);
  await Deno.rename(file2, file1);
  await Deno.chmod(file1, 0o666);
  const f = await Deno.open(file1);
  f.close();

  setTimeout(() => {
    watcher.close();
  }, 750);

ubuntu:

{"type":{"create":{"kind":"file"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"any"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{}}
{"type":{"access":{"kind":"close","mode":"write"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/tmp/392eadc2/file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"any"}},"paths":["/tmp/392eadc2/file2.txt"],"attrs":{}}
{"type":{"access":{"kind":"close","mode":"write"}},"paths":["/tmp/392eadc2/file2.txt"],"attrs":{}}
{"type":{"remove":{"kind":"file"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"rename","mode":"from"}},"paths":["/tmp/392eadc2/file2.txt"],"attrs":{"tracker":2133}}
{"type":{"modify":{"kind":"rename","mode":"to"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{"tracker":2133}}
{"type":{"modify":{"kind":"rename","mode":"both"}},"paths":["/tmp/392eadc2/file2.txt","/tmp/392eadc2/file1.txt"],"attrs":{"tracker":2133}}
{"type":{"modify":{"kind":"metadata","mode":"any"}},"paths":["/tmp/392eadc2/file1.txt"],"attrs":{}}

macOS

{"type":{"create":{"kind":"folder"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file1.txt"],"attrs":{}}
{"type":{"remove":{"kind":"file"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"content"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file1.txt"],"attrs":{}}
{"type":{"create":{"kind":"file"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"rename","mode":"from"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"data","mode":"content"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"rename","mode":"from"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"metadata","mode":"ownership"}},"paths":["/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/679f3d6f/file1.txt"],"attrs":{}}

Windows

{"type":{"create":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file1.txt"],"attrs":{}}
{"type":{"create":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file2.txt"],"attrs":{}}
{"type":{"remove":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"rename","mode":"from"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file2.txt"],"attrs":{}}
{"type":{"modify":{"kind":"rename","mode":"to"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file1.txt"],"attrs":{}}
{"type":{"modify":{"kind":"any"}},"paths":["C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\9d0f8c00\\file1.txt"],"attrs":{}}

@ry
Copy link
Member

ry commented Jan 23, 2020

feel free to make the test sloppy - i don't think nailing down the exact order is important at this stage

@bartlomieju
Copy link
Member Author

feel free to make the test sloppy - i don't think nailing down the exact order is important at this stage

Sure, what about return type from watcher? What you see above is what notify serializes to

@bartlomieju bartlomieju mentioned this pull request Jan 26, 2020
@bartlomieju
Copy link
Member Author

This is getting stale; the only thing left to do in it is providing some unified interface between different OSes (see #3452 (comment)).

cli/ops/fs.rs Outdated Show resolved Hide resolved
cli/ops/fs.rs Outdated Show resolved Hide resolved
cli/ops/fs.rs Outdated Show resolved Hide resolved
@ry ry removed the help wanted label Feb 21, 2020
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally working!

@ry ry merged commit bd640bc into denoland:master Feb 21, 2020
@bartlomieju bartlomieju deleted the feat-fs_events branch February 21, 2020 18:25
@ry ry mentioned this pull request Feb 21, 2020
43 tasks

export function fsEvents(
paths: string | string[],
options = { recursive: true }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be written as

{
  recursive = true
}: FsEventsOptions = {}

for full robustness.

@magichim
Copy link

magichim commented Mar 1, 2020

Deno.watch feature is added at this time in 0.35 version?
@bartlomieju

@bartlomieju
Copy link
Member Author

Yes https://github.com/denoland/deno/releases/tag/v0.35.0

@ry
Copy link
Member

ry commented Mar 1, 2020

https://deno.land/typedoc/index.html#fsevents

caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 21, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 24, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Jan 31, 2021
caspervonb pushed a commit to caspervonb/deno_std that referenced this pull request Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants