Watcher Refactor

[Byron]

This issue keeps track of initial research to understand watcher.rs better, with the intend to formulate a plan to revise it, and make it more maintainable in the process.

PRs

• watcher refactor #3471
• split tauri/watcher into crate #3541

Refactor opportunities fixed In PRs

• Excessive use of try_from() which should rather be new() for better readability of what's going on. try_from is still called by hand each time, so new() is really the intend here.
• Allow handlers to be stopped using a message/event through a channel. They are already using this, and all it takes is to expose it. That way Watcher doesn't have to be Clone, which makes WatcherInner obsolete, removing a level of indirection.
  • This turned out not to be possible yet, Clone is still required. All abstractions were removed though.
• Handler should gain a method that has all the code currently distributed in various handlers modules, then can also reuse more state, like opened git repositories. This would also do away with Arc<Mutex>>, which I doubt are necessary everywhere. It doesn't need to be clone anymore either.
  • Mostly addressed, even though there still is a handle() method.
  • The internal message queue… it probably resolves itself then. - a channel is still used to communicate with this long-running process.
• Only keep one active Watcher, depending on the project.

Issues related to watcher

• Deletions of the project itself should be recognized as event and cause the UI to act accordingly (Related to UI freeze when watcher cannot find folder #3001). I'd find it unsurprising if the UI would at least go back to the project selector, while showing a message of some kind that indicates what happened.

Discovered Issues

Issues (re)discovered during the watcher refactoring, but not related to the watcher alone.

• Creating and removing empty files often doesn't register the removal (MacOS) #3511
• "Add new project" in a worktree fails due to not being able to read the objects directory. #3062 (lack of worktree support)
• Adding the Linux Kernel as project takes more than an hour to complete #3512
• git::credential::Helpers::from_path() is used in tests, but depends on the actual HOME environment variables, isn't isolated
  • Maybe check other from_path() implementations as well.

Notes

The notes here are partially derived from my notes branch.

• Application keeps watchers for all registered projects, even though the user can only work in one project at a time right now.
  • It's probably to allow quick-switching of projects. However, while the application is closed, it still has a way to catch up with changes, so it should be possible to do the same when switching a project, and watch only when needed. Maybe it's an optimization though that nobody needs. But in a way, the question is what the app should do if a project triggers filesystem events that aren't relevant to what's currently displayed - seems like unnecessary overhead.
• Watcher is Clone to make it possible to keep track of it, and to supervise it to deal with its error code. These run forever though, and are never removed, so it seems OK to not track them in the first place. And if communication is needed, this should be done with a channel of some sort. The tracing can then happen as part of instrumentation or dedicated tracing in the watcher impl itself.
  • Without Clone, WatcherInner can become Watcher (CancellationToken isn't Clone)
• CancellationToken is used to stop() watchers, which happens only on project-deregistration
• Handler is abstracting multiple different 'logic' implementations, and I have a feeling that maybe these can be combined into one handler method, that calls other methods, which do the job. That would also allow these to reuse state and not open a repository each time. It's clearly an 'object oriented' approach which actively hurts usability as state is hard and is actively worked around with Arc<Mutex>> everywhere.
• It calls itself through events, and a single processed event can create

[mtsgrd]

About watching all projects, I'd be very happy if we could limit it to the currently active project. Originally it served a more important purpose, and I can picture a future where we might want it again, but even so it would need to do so selectively. Let's simplify for now and re-introduce later if needed?

[Byron]

Thanks for sharing!
I also prefer to have only as much complexity (and capability) as needed, while keeping the code easy to refactor. Thus it's probably beneficial to only keep a single watcher for the currently active project. The issue was updated with that refactor opportunity.

[Byron]

I think with the two PRs merged, this tracking issue can be considered resolved. The one remaining issue related to the watcher (#3447) needs feedback, as it couldn't be reproduced.