Closed Bug 958280 Opened 10 years ago Closed 2 years ago

[OS.File] Add a watch() function to OS.File

Categories

(Toolkit Graveyard :: OS.File, defect)

Product:
Toolkit Graveyard
Component:
OS.File
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: harth, Unassigned)

References

(Blocks 3 open bugs)

Details

Attachments

(1 file)

jscfilewatcher-demo xpi
120.66 KB, application/x-xpinstall
Details 
DevTools needs a way to watch for changes to a file on disk.
Blocks: 950915
Summary: Add a watch() function to OS.File → [OS.File] Add a watch() function to OS.File
This one is quite non-trivial.

For Linux/Android/B2G: http://linux.die.net/man/7/inotify
For Windows: http://msdn.microsoft.com/en-us/library/aa364417%28VS.85%29.aspx
For MacOS X: https://developer.apple.com/library/mac/documentation/Darwin/Conceptual/FSEvents_ProgGuide/Introduction/Introduction.html

Given the scope, each of these deserves a bug in its own right.

We will probably need a worker just for watching files.

Here's a possible API:

let watcher = new OS.File.Watcher(callback)
watcher.addPath(...);
watcher.removePath(...);
watcher.close(); // Don't forget to do this, otherwise resource leak

I'm not certain whether we can do it in JS at all, we might wish to go native.
Windows has multiple way to watch changes on the filesystem.
ReadDirectoryChangesW: http://msdn.microsoft.com/en-us/library/windows/desktop/aa365465%28v=vs.85%29.aspx
Change journals (NTFS and ReFS only): http://msdn.microsoft.com/en-us/library/windows/desktop/aa363798%28v=vs.85%29.aspx
I would be interested in working on this feature, does it need to be delivered ASAP or can I take my time?
Flags: needinfo?(dteller)
Flags: needinfo?(dteller)
I'll let the devtools team answer that.
Flags: needinfo?(paul)
I'm willing to mentor that bug, mind you.
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #5)
> I'm willing to mentor that bug, mind you.

Awesome :)
(In reply to Alessio Placitelli from comment #3)
> I would be interested in working on this feature, does it need to be
> delivered ASAP or can I take my time?

It's a pretty important piece of a feature we'd like to land mid May.
Flags: needinfo?(paul)
Gaia build system would also need that for the current refactoring.

Yuren, just to be sure, noone on your side already worked on this?
Flags: needinfo?(yurenju.mozilla)
(In reply to Paul Rouget [:paul] from comment #7)
> (In reply to Alessio Placitelli from comment #3)
> > I would be interested in working on this feature, does it need to be
> > delivered ASAP or can I take my time?
> 
> It's a pretty important piece of a feature we'd like to land mid May.

Sounds reasonable: I'll wait for Yuren to reply then.
for now we just use nodejs to watch files and execute |make|. it would be great if we can use this feature to watch files change and automatic build gaia in browser with an extension :-)
Flags: needinfo?(yurenju.mozilla)
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #5)
> I'm willing to mentor that bug, mind you.

Given the answers from :yurenju, :paul and :ochameau, is this something I can work on? I'm interested! :)
Flags: needinfo?(dteller)
Blocks: 971719
(In reply to Alessio Placitelli from comment #11)
> (In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from
> comment #5)
> > I'm willing to mentor that bug, mind you.
> 
> Given the answers from :yurenju, :paul and :ochameau, is this something I
> can work on? I'm interested! :)

I have just divided this bug in three. You can give a try to one of bug 992894, bug 992895 or bug 992896. We'll see how it goes. And thanks for offering :)
Flags: needinfo?(dteller)
Blocks: 995309
Any update on this?
Lots of activity on blocker bug 	992894. Once this one is done, I believe that things will be easier for Mac OS X. I don't know about Linux, Android or B2G yet.
Eagerly awaiting this. :) Thanks for everyones hard work on it :)

Already I have two applications for it, for now im resorting to js-ctypes.

One application: http://stackoverflow.com/questions/24336744/with-js-ctypes-detect-other-profile-windows-windows
Another use file watcher can be used for: watch when protocol handlers are changed:
http://stackoverflow.com/questions/25132170/how-to-watch-nsihandlerservice-storehandlerinfo/25133485#25133485
Hi all,
How does this file watcher differ from RDF observer?
http://mxr.mozilla.org/mozilla-release/source/toolkit/mozapps/downloads/content/helperApps.js#87
Nevermind rdf observer only works from within one profile, not notified in other profiles which makes sense.
I'm curious, can these OS.File.watch functions be used to watch for non-file content changes? Like if a file was locked or if a lock was released on the file?
(In reply to noitidart from comment #20)
> I'm curious, can these OS.File.watch functions be used to watch for non-file
> content changes? Like if a file was locked or if a lock was released on the
> file?

No, as the OS dependent functions used to detect this kind of events do not behave consistently (i.e. detect the same things) across the supported platforms.

This API can be used to detect file/directory creation, deletion and modification.
(In reply to Alessio Placitelli [:Dexter] from comment #21)
> (In reply to noitidart from comment #20)
> > I'm curious, can these OS.File.watch functions be used to watch for non-file
> > content changes? Like if a file was locked or if a lock was released on the
> > file?
> 
> No, as the OS dependent functions used to detect this kind of events do not
> behave consistently (i.e. detect the same things) across the supported
> platforms.
> 
> This API can be used to detect file/directory creation, deletion and
> modification.

oh cool thanks alessio for that, forgive me the masssssive delay haha
An idea/question on the API, we may want to make it OS.File.Directory.Watcher and move the Iterator from OS.FileDirectoryIterator to OS.File.Directory.Iterator (No deprecation to, OS.File.DirectoryIterator, it should still exist for legacy purposes (all those that using it right now) but just redirection from OS.File.Directory.Iterator to OS.File.DirectoryItrator) what do you guys think?
Or maybe OS.File.Watcher is good enough as we allow in options devuser to pass in flags. So even though the default flags are for directory watching, devusers can modify the flag set to watch files.
I found this awesome document someone wrote up Jan 2014 when trying to make a cross-os API: https://onedrive.live.com/view.aspx?cid=C2C2E10FEF4F47F9&resid=c2c2e10fef4f47f9!397&app=Word

They have this nice table:
+--------------------------------+-------------------+
| API                            |         OS        |
+--------------------------------+-------------------+
| inotify                        | Linux, Android    |
+--------------------------------+-------------------+
| kqueue                         | BSD, OS X, iOS    |
+--------------------------------+-------------------+
| ReadDirectoryChangesW          | Windows           |
+--------------------------------+-------------------+
| File Events Notification (FEN) | Solaris 11        |
+--------------------------------+-------------------+
| FSEvents                       | OS X / OS X 10.7+ |
+--------------------------------+-------------------+
| N/A                            | Plan 9            |
+--------------------------------+-------------------+
| fanotify                       | Linux 2.6.37+     |
+--------------------------------+-------------------+
Interesting repo they have techniques for all the different OS's. this is node.js's file watcher thingy:

unix: https://github.com/joyent/node/tree/master/deps/uv/src/unix
win: https://github.com/joyent/node/tree/master/deps/uv/src/win
So based on the original plans of:
```
let watcher = new OS.File.Watcher(callback)
...
```

I propose changing it to

```
let watcher = new OS.File.DirectoryWatcher(callback)
...
```

This matches `OS.File.DirectoryIterator` style.
This sounds like a good idea. There are many dependencies that we need to land before that, though.
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #29)
> This sounds like a good idea. There are many dependencies that we need to
> land before that, though.

If its js and ctypes I would love to go for it i think i can do it. Can you please mentor me to land a jsctypes version.

I know the jsctypes is probably not as performant as Dexter's but maybe we can land this to just nightly for research? I'll definitely learn the internals of OS.File and will be able to help a lot more on the OS.File bugs.

The jsctypes file watcher is all done except for GioFileMonitor, i started it off then handed it off to p0lip, he should be almost done.
Copying comment originally posted to bug 1182254:

(In reply to noitidart from comment #1)
> If you guys will accept a js-ctypes version of file watcher (and have the
> mentor resources to provide me with) I could really focus on Bug 958280 and
> knock it out in a month.

The DevTools team is now even more interested in this ability than before, as we'd like to have a way to reload the DevTools toolbox when source files are changed for a better contributor experience.

I myself don't have experience with js-ctypes, so I am not a good mentor for that approach, and I don't believe anyone else on DevTools team is either.  

It seems the Windows version in bug 992894 went with a true native approach, so perhaps that means js-ctypes is not the best way to go here for other platforms?

Yoric, are you able to mentor this work for other platforms?  Do you care whether native vs. js-ctypes is used?
Flags: needinfo?(dteller)
Deflecting the Linux part to Dexter.
Also, André, you mentioned that you would be interested in mentoring for MacOS, right?
Flags: needinfo?(dteller) → needinfo?(areinald.bug)
Flags: needinfo?(alessio.placitelli)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #31)
> Copying comment originally posted to bug 1182254:
> 
> (In reply to noitidart from comment #1)
> > If you guys will accept a js-ctypes version of file watcher (and have the
> > mentor resources to provide me with) I could really focus on Bug 958280 and
> > knock it out in a month.
> 
> The DevTools team is now even more interested in this ability than before,
> as we'd like to have a way to reload the DevTools toolbox when source files
> are changed for a better contributor experience.
> 
> I myself don't have experience with js-ctypes, so I am not a good mentor for
> that approach, and I don't believe anyone else on DevTools team is either.  
> 
> It seems the Windows version in bug 992894 went with a true native approach,
> so perhaps that means js-ctypes is not the best way to go here for other
> platforms?
> 
> Yoric, are you able to mentor this work for other platforms?  Do you care
> whether native vs. js-ctypes is used?

I understand its no prob. The js-ctypes can be just for reasearch. Perf comparison to the native so we can imporve jsctypes is my hope. Its not using pipes yet its on a timeout. Would need to change that if you guys wanted to consider it for what was not completed natively already.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #31)
> It seems the Windows version in bug 992894 went with a true native approach,
> so perhaps that means js-ctypes is not the best way to go here for other
> platforms?

Bug 992896 (the Linux version), using GIOFileMonitor is almost there, there are a few points left to address from the previous review pass. Unfortunately, as I'm now working on Telemetry, I don't have much time left to complete this patch.

The open issues are just really language specific issues, nothing related to the watcher logic itself: maybe someone with C++ knowledge could pick it up. I'd be open to help and discuss it further.

As for the native vs js-ctypes, we went along the native path for mainly two reasons: we were concerned about the complexity of the code and the performance. But a long time passed since then, so the concerns about performance may not hold anymore.
Flags: needinfo?(alessio.placitelli)
(In reply to David Rajchenbach-Teller [:Yoric] (use "needinfo") from comment #32)
> Deflecting the Linux part to Dexter.
> Also, André, you mentioned that you would be interested in mentoring for
> MacOS, right?

FSEvents are indeed the way to go for Mac OS X.

Plus I've been involved in a related issue on Linux: it was file access interception, not change notification though.

So I may have some piece of advice on both platforms.

Like Dexter, I'm currently busy with Telemetry, but if I can help here I definitely will.
Flags: needinfo?(areinald.bug)
One drawback of FSEvents is they have file level granularity only since 10.7.

A lower level alternative supported for all OS versions are Kernel Queues, but they may be more tricky to fit into our design (they don't rely on a CFRunLoop, so we may need a separate thread just for watching file changes).
We are already spawning a thread for the other versions.
(In reply to André Reinald from comment #37)
> One drawback of FSEvents is they have file level granularity only since 10.7.
> 
> A lower level alternative supported for all OS versions are Kernel Queues,
> but they may be more tricky to fit into our design (they don't rely on a
> CFRunLoop, so we may need a separate thread just for watching file changes).

For 10.7+ we used FSEvents with file level granularity.

Initially for <10.7 we used kq and its still there (as we think iOS only has kq and we will have fx on ios soon). But one issue we found with kq was due to the directory watched granularity: Contents modified fires on the directory only if an existing file is modified atomically (file removed and new file replaces it): https://github.com/Noitidart/jscFileWatcher/issues/19 http://stackoverflow.com/questions/29999204/kqueue-on-directory-not-trigger-when-file-within-is-modified

So we switched to FSEvents for <10.7 but we didnt have a 10.6 system to test on. We were hoping that FSEvents even though directory granularity only, would trigger on the watched directory even when existing files within were not modified atomically. If this <10.7 FSEvents also is similar to kq in that it wont fire contents-modified on directory when subfile is modified non-atomically we were out of ideas and decided to tell the devuser using the api that, if they want to watch when a certain file changes on <10.7 or iOS (as we would continue kq support on iOS) they would have to watch the file directly.

We disallowed watching file directly on inotify, even though it supports it, because we wanted to match the behavior of 10.7+ FSEvents and Windows ReadDirectoryChangesW.
Here are some other quirks from my memory and basic notes but something like this was quirky:

inotify (needed for android support) no notification when do anything in a subdir of watched dir so to make other OS match behavior we discard events for a subdir when triggered my contents-modified within subdir, so for FSEvents 10.7+ and ReadDirectoryChangesW. For ReadDirChW we set subtree to false but still we would get notification when a file was created in a subdir, so if we got file-add on subdir of watched dir, we would discard that event (not present it to devuser) if they were not watching that subdir path as well.

https://github.com/Noitidart/jscFileWatcher/issues/8
And our goal was to support 10.6+ as Firefox 29 is guaranteed to work on mac 10.6+ in the specs: https://www.mozilla.org/en-US/firefox/29.0/system-requirements/
In the the jscfilewatcher we create a thread per new OS.File.DirectoryWatcher() and we limit watching to 64 paths as thats the max ReadDirectoryChangesW can do per thread so p0lip and i thought to keep things same across as much as possible.
I finally made a droppable git submodule out of the js-ctypes watcher. Redesigned it too, much much perfromant then initial version.

https://github.com/Noitidart/jscFileWatcher/

Here is the demo:

https://github.com/Noitidart/CommPlayground/tree/jscfilewatcher-demo

Download the XPI and then make file changes on your desktop and you will see it logged in your console. On Android it watches the OS.Constants.Path.profileDir as there is no desktop path there.

There is a readme.md on both repos.
Doing need info to @Yoric to see if we should move head with part of, or all of, or none of, the ctypes version.
Flags: needinfo?(dteller)
js-ctypes is out of fashion these days, so I'd say let's not do it.
Flags: needinfo?(dteller)
No problem, understood. :)

Mass closure: OSFIle is being replaced with IOUtils and PathUtils. If you think this bug was closed in error, please re-open.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: