Do not force absolute Paths

[philjb]

fsevents @ 10556809

Which version of macOS are you using?

ProductName:    Mac OS X
ProductVersion: 10.15.7
BuildVersion:   19H1217

Please describe the issue that occurred.

FSEvents api supports two different ways of starting an event stream:

1. Uses a device id and function call FSEventStreamCreateRelativeToDevice which is expecting paths to be relative to the mount point of the device. This is called the per-disk event stream in the docs. The example/main.go uses this approach since it uses a non-zero device id. See the documentation here specifically about param pathsToWatchRelativeToDevice

   pathsToWatchRelativeToDevice
   A CFArray of CFStringRefs, each specifying a relative path to a directory on the device identified by the dev parameter. The paths should be relative to the root of the device. For example, if a volume "MyData" is mounted at "/Volumes/MyData" and you want to watch "/Volumes/MyData/Pictures/July", specify a path string of "Pictures/July". To watch the root of a volume pass a path of "" (the empty string).

2. The alternative is with function FSEventStreamCreate which is expecting absolute paths. This is the "per-host" event stream constructor. fsevents uses this approach if the Device in the EventStream struct is zero. While the docs are ambiguous, on my system, the "per-host" approach only works with absolute paths.

   pathsToWatch
   A CFArray of CFStringRefs, each specifying a path to a directory, signifying the root of a filesystem hierarchy to be watched for modifications.

However, wrap.go/createPaths() transforms all paths in an EventStream struct to absolute paths at https://github.com/fsnotify/fsevents/blob/f721bd2b045774a566e8f7f5fa2a9985e04c875d/wrap.go#L154-L158

This prevents using the "per-disk" mode for all volumes except the root volume at /.

Are you able to reproduce the issue? Please provide steps to reproduce and a code sample if possible.

Yes.

1. Create a new Volume on OSX (I use APFS) mounted at /Volumes/fsevents-repo
2. Use the example/main.go program with path /Volumes/fsevents-repo (the only change)
3. go run ./example/main.go
4. touch /Volumes/fsevents-repo/testfile and observe there is no event reports.
5. Now, change wrap.go/createPaths to using

   for _, path := range paths {
       p := path
       cpath := C.CString(p)

   and repeat the steps above.

PR coming shortly.

[nathany]

Thanks for reporting the issue. A PR would be appreciated.

[marklr]

I have a tentative PR that works with the provided test case, by stripping the mount points from the paths before adding them to the monitoring list: https://github.com/fsnotify/fsevents/pull/58

[pbnjay]

So the problem identified here is that EventStream is mixing FSEventStreamCreate and FSEventStreamCreateRelativeToDevice which each have different input requirements, and each provide different sources for event IDs under the hood. The selection of which stream type is done magically depending on the value of EventStream.Device

For the typical users of this package, is there any reason to prefer FSEventStreamCreateRelativeToDevice and add all the complexity of PR #58 ? Would it make more sense to split the advanced functionality into a DeviceEventStream interface that makes the behavior explicit? This would still allow for persistent notifications but wouldn't introduce more surface area for bugs in the most common use-case.

If the goal of this package is future adoption in fsnotify/fsnotify then simplifying the primary codepath would be beneficial, right?

[marklr]

absolutely agree that there's probably a need to revisit the public API. I'm not familiar enough with the project to offer significant input right now but am happy to. help with whatever implementation is decided upon.

On Sat, Jan 22, 2022, 23:27 Jeremy Jay @.***> wrote:

So the problem identified here is that EventStream is mixing FSEventStreamCreate and FSEventStreamCreateRelativeToDevice which each have different input requirements, and each provide different sources for event IDs under the hood. The selection of which stream type is done magically depending on the value of EventStream.Device

For the typical users of this package, is there any reason to prefer FSEventStreamCreateRelativeToDevice and add all the complexity of PR #58 https://github.com/fsnotify/fsevents/pull/58 ? Would it make more sense to split the advanced functionality into a DeviceEventStream interface that makes the behavior explicit? This would still allow for persistent notifications but wouldn't introduce more surface area for bugs in the most common use-case.

If the goal of this package is future adoption in fsnotify/fsnotify then simplifying the primary codepath would be beneficial, right?

— Reply to this email directly, view it on GitHub https://github.com/fsnotify/fsevents/issues/49#issuecomment-1019360759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHCVHSAF2RJQKQYXD2JJRDUXMOKLANCNFSM5AQMFFAQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

[nathany]

Simple is good.