Introduce an alternative Linux implementation using inotify…

[savetheclocktower]

…that fits our feature set better than EFSW’s does and doesn’t have the complexity of recursive watching.

In pulsar-edit/pulsar#1592 we're dealing with a crash whose origin seems to be a memory allocation issue somewhere within pathwatcher — specifically EFSW’s inotify implementation. We could chase it down further, but we'd basically be debugging vendor code at that point, and I'd much rather dedicate the same level of effort toward a replacement.

A while back, when EFSW’s macOS implementation didn't fit our needs, I wrote an API-compatible FSEvents adapter that we could use in its place. (Then, much more recently, when FSEvents seemed to mean regressions in certain cases, I wrote a separate kqueue-based adapter to use instead.)

This PR extends this idea further by adding a new inotify-based adapter for Linux that can be used instead of EFSW's inotify implementation. Our adapter has the advantage of being much simpler because pathwatcher doesn't do recursive filesystem watching; it cares only about directories and their direct children.

I cannot promise that this itself will prevent the crashes reported in pulsar-edit/pulsar#1592, but it should at the very least move them toward code that we maintain and can understand much better.

The tests pass on my Linux VM; if they pass in CI, I'll make a draft PR in the Pulsar repo that points to this pathwatcher branch simply for the purpose of generating binaries that affected users can download to see whether this fixes the issue.

[savetheclocktower]

Tentatively good news — one user affected by pulsar-edit/pulsar#1592 says that this fixes it for them. So after this lands, I'll be motivated to write a Windows-specific adapter as well so that we can finally retire EFSW.

I'm grateful for EFSW’s existence (I tried rewriting pathwatcher back in 2024 and it didn't go well until I brought EFSW into the mix!) but we probably don't need it anymore now that we can write simpler adapters for our narrow use cases. And it's looking like pathwatcher will stay with us (in one form or another) instead of being retired, so we might as well make it light and robust.

[savetheclocktower]

Paging @Daeraxa as an “unofficial” reviewer. (We should get you added to the appropriate list so you can do real reviews!)

[Daeraxa]

Paging @Daeraxa as an “unofficial” reviewer. (We should get you added to the appropriate list so you can do real reviews!)

I will be happy to review as soon as I can, sorry I've been absurdly busy recently and won't be able to look properly until Monday, but I will look!

[Daeraxa]

So the binary produced definitely seems to work, I haven't managed to get it all working directly via yarn start but that's because I clearly haven't updated it in other packages used in Pulsar - specifically tree-view.

[savetheclocktower]

I haven't managed to get it all working directly via yarn start but that's because I clearly haven't updated it in other packages used in Pulsar - specifically tree-view.

If you're building from pulsar-edit/pulsar#1597 that's supposed to be taken care of. As long as you run yarn install && yarn build after checking out, it should update all usages of @pulsar-edit/node-pathwatcher to use the same copy. If it's still giving you trouble after that, let me know.

[Daeraxa]

aah hadnt realised it was in a pr, still working through things... lots to catch up on.

Huh, dunno what went wrong, I did the same as in that PR but it didn't resolve correctly - no idea what the issue was there. Either way it seems to work just fine.

Do you know of any other place where we might have seen this issue other than tree-view?

[Daeraxa]

Humour my ignorance here, if this is an API compatible replacement then why do we still need to include this? Or is it only a particular part being replaced?

[savetheclocktower]

The .hpp file being referenced includes type definitions. So we need it in order to be able to refer to things below like efsw::FileWatchListener and efsw::WatchID.

But, yes, in another sense only a particular part is being replaced. In core.h we conditionally define the FileWatcher type as referring to either an efsw::FileWatcher (on Windows) or an API-compatible replacement (on other platforms). In order to prove it's an API-compatible replacement, we have to reference efsw‘s types.

I imagine that we'll eventually use our own implementation on Windows, at which point we can remove efsw from the repo entirely and just use types entirely of our own definition.