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

[Filesystem] Add watch method to watch filesystem for changes #31462

Open
wants to merge 2 commits into
base: 4.4
from

Conversation

@pierredup
Copy link
Contributor

pierredup commented May 10, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR TBD

Add a watch method to the Filesystem component to monitor the filesystem for changes, and executing a callback function whenever a change is detected.

The watch method can take any parameter and uses a resource locator to transform a path to a resource. The resource can then be monitored for any changes. When a directory is monitored, you can get changed events when a file in the directory changes, when a new file is created or when an existing file is deleted. It also tries to make usage of Inotify is the extension is installed.

Example usage:

$fs = new Filesystem();

$fs->watch('/path/to/some/directory', function ($file, $event) {
    if ($event === FileChangeEvent::FILE_CREATED) {
        echo $file.' was created!';
    }
});

If you want to bail out of the watch events, the callback can return false to stop the process.

$fs = new Filesystem();

$fs->watch('/path/to/some/directory', function ($file, $event) {
    if ($event === FileChangeEvent::FILE_DELETED) {
        echo $file.' was deleted!';
        return false; // <- Will stop the watch process and continue with execution of the the rest of the application
    }
});
@pierredup pierredup changed the title Add watch method to watch filesystem for changes [Filesystem] Add watch method to watch filesystem for changes May 10, 2019
@pierredup pierredup force-pushed the pierredup:filesystem-watch branch from 0fdfa5a to 7ef4b10 May 10, 2019
@ro0NL

This comment has been minimized.

Copy link
Contributor

ro0NL commented May 10, 2019

ref #4605, perhaps some relevant info is there still.

@nicolas-grekas nicolas-grekas modified the milestones: 4.3, next May 11, 2019
}
}
sleep(1);

This comment has been minimized.

Copy link
@miniyarov

miniyarov May 12, 2019

Contributor

Could be useful to use usleep and define by microseconds by an optional parameter?

This comment has been minimized.

Copy link
@pierredup

pierredup Sep 30, 2019

Author Contributor

I'm not sure if it will be worth it, as the sleep is only applicable in this watcher, so I don't think it should be defined in every watcher as well as the interface.
And if we use usleep and the parameter is passed with a very low number, then it can be quite resource intensive to continuously check the filesystem and not have a long enough wait time between checks

This comment has been minimized.

Copy link
@pierredup

pierredup Oct 1, 2019

Author Contributor

A parameter has been added to control the waiting time

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Sep 27, 2019

@pierredup up to finish this one?

@pierredup pierredup force-pushed the pierredup:filesystem-watch branch from 7ef4b10 to 4d7dae5 Sep 30, 2019
@pierredup pierredup changed the base branch from master to 4.4 Sep 30, 2019
@pierredup pierredup force-pushed the pierredup:filesystem-watch branch 3 times, most recently from cce66d2 to 1cbfd18 Sep 30, 2019
@pierredup

This comment has been minimized.

Copy link
Contributor Author

pierredup commented Sep 30, 2019

Appveyor failure is unrelated, this is ready for review

@pierredup pierredup force-pushed the pierredup:filesystem-watch branch from 1cbfd18 to 2a556d3 Sep 30, 2019
*/
class INotifyWatcher implements WatcherInterface
{
public function watch($path, callable $callback)

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Sep 30, 2019

Member

Idea: add a float $timeout = null argument to control the max idle time.
The implementation would use stream_select() to pause instead of inotify_read()

This comment has been minimized.

Copy link
@pierredup

pierredup Oct 1, 2019

Author Contributor

Timeout added to both the filesystem watcher and the inotify watcher, which takes a value in milliseconds

@pierredup pierredup force-pushed the pierredup:filesystem-watch branch from 418d1d6 to 5d6ada4 Oct 1, 2019
@michaljusiega

This comment has been minimized.

Copy link
Contributor

michaljusiega commented Oct 10, 2019

It is possible to merge this PR in near feature ?

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
$isDir = is_dir($path);
if ($isDir) {
$watchId = inotify_add_watch($inotifyInit, $path, IN_CREATE | IN_DELETE | IN_MODIFY);

This comment has been minimized.

Copy link
@maidmaid

maidmaid Nov 1, 2019

Contributor

The files moves (IN_MOVE | IN_MOVE_SELF) and attributes changes (IN_ATTRIB) should also be watched imo (and also IN_DELETE_SELF).

*/
class INotifyWatcher implements WatcherInterface
{
public function watch($path, callable $callback, float $timeout = null)

This comment has been minimized.

Copy link
@maidmaid

maidmaid Nov 1, 2019

Contributor

What about if I would like to watch path recursively (like in the #32764 use case) ?

*
* @param mixed $path The path to watch for changes. Can be a path to a file or directory, iterator or array with paths
* @param callable $callback The callback to execute when a change is detected
* @param float $timeout The time in milliseconds to wait between checking for changes (defaults to 1000 when inotify is not available)

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Nov 5, 2019

Member

Sorry I wasn't clear enough in my suggestion here: I meant an idle timeout - ie if nothing happens for that duration, we should quit the while loop in implementations.
But as long as something happens during the interval, we should continue looping.
By default or null should mean : forever (as is does now already - configuring the pause timeout doesn't provide any useful behavior from the outside)

@nicolas-grekas nicolas-grekas modified the milestones: 4.4, next Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.