Closed
Bug 958280
Opened 11 years ago
Closed 3 years ago
[OS.File] Add a watch() function to OS.File
Categories
(Toolkit Graveyard :: OS.File, defect)
Toolkit Graveyard
OS.File
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: harth, Unassigned)
References
(Blocks 3 open bugs)
Details
Attachments
(1 file)
(deleted),
application/x-xpinstall
|
Details |
DevTools needs a way to watch for changes to a file on disk.
Updated•11 years ago
|
Summary: Add a watch() function to OS.File → [OS.File] Add a watch() function to OS.File
Comment 1•11 years ago
|
||
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.
Comment 2•11 years ago
|
||
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
Comment 3•11 years ago
|
||
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)
Updated•11 years ago
|
Flags: needinfo?(dteller)
Comment 5•11 years ago
|
||
I'm willing to mentor that bug, mind you.
Comment 6•11 years ago
|
||
(In reply to David Rajchenbach Teller [:Yoric] (please use "needinfo?") from comment #5)
> I'm willing to mentor that bug, mind you.
Awesome :)
Comment 7•11 years ago
|
||
(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)
Comment 8•11 years ago
|
||
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)
Comment 9•11 years ago
|
||
(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.
Comment 10•11 years ago
|
||
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)
Comment 11•11 years ago
|
||
(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)
(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)
Comment 13•10 years ago
|
||
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.
Comment 16•10 years ago
|
||
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
Comment 17•10 years ago
|
||
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
Comment 18•10 years ago
|
||
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
Comment 19•10 years ago
|
||
Nevermind rdf observer only works from within one profile, not notified in other profiles which makes sense.
Comment 20•10 years ago
|
||
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?
Comment 21•10 years ago
|
||
(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.
Comment 22•10 years ago
|
||
(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
Comment 23•10 years ago
|
||
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?
Comment 24•10 years ago
|
||
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.
Comment 25•10 years ago
|
||
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+ |
+--------------------------------+-------------------+
Comment 26•10 years ago
|
||
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
Comment 27•10 years ago
|
||
Nice writeup here about the node.js filewatcher techqniues: https://github.com/joyent/node/blob/ae58fc407f916b2abb164453a5b09273c543dbc3/doc/api/fs.markdown#availability
Comment 28•10 years ago
|
||
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.
Comment 30•10 years ago
|
||
(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.
Blocks: dt-contribute
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)
Comment 33•9 years ago
|
||
(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.
Comment 34•9 years ago
|
||
GioFileMonitor in js-ctypes already written here: https://github.com/max-linux/max-desktop/blob/ba002b6de880af9ec02e79fa9b7f194cc7cb5e91/max-ubufox/res/libs/gio.jsm#L202
Comment 35•9 years ago
|
||
(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)
Comment 36•9 years ago
|
||
(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)
Comment 37•9 years ago
|
||
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.
Comment 39•9 years ago
|
||
(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.
Comment 40•9 years ago
|
||
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
Comment 41•9 years ago
|
||
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/
Comment 42•9 years ago
|
||
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.
Comment 43•8 years ago
|
||
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.
Comment 44•8 years ago
|
||
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)
Comment 46•8 years ago
|
||
No problem, understood. :)
Comment 47•3 years ago
|
||
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: 3 years ago
Resolution: --- → WONTFIX
Updated•2 years ago
|
Product: Toolkit → Toolkit Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•