feat: Watchman integration #188
Conversation
|
i know it's not working yet, i also want to clean up the watchman library i forked and publish a proper release. figured i'd open this to see if there was interest in the first place for such a feature |
What do you mean by this? Could you explain how adding this functionality to PC directly, vs wrapping individual programs which need to restart on file changes with tools like https://github.com/watchexec/watchexec, would help with that? Thanks! |
watchman follows a server-client model. the user's filesystem is split into a collection of "projects" (user-configurable, usually means a directory with one of
what this means is that the inotify allotments are only ever held by the watchman server. when the server receives an inotify notification, it filters the paths that are irrelevant to a given subscription and sends any relevant paths in a subscription ipc message. effectively, this is inotify in userspace :) ETA: git and jujutsu both integrate with watchman as well (i've had both options enabled for a while, and rarely encounter issues) |
|
after saying all of that, i think the way i set up subscriptions for each process wrong. instead of using a single watchman subscription for all processes, i should use a single subscription per process that needs it. then i don't need to do further filtering in the client (like i do in 6653b14#diff-defe37158035c547155c5ae19d82fc87e70e6ccbe0d61b261c201872f92b2d00R125-R185) |
| github.com/bytedance/sonic v1.11.6 // indirect | ||
| github.com/bytedance/sonic/loader v0.1.1 // indirect | ||
| github.com/cdmistman/watchman v0.2.1-0.20240521013409-16b1ae353832 // indirect |
There was a problem hiding this comment.
this won't be pinned to a revision by the time i mark this pr as ready. once i finish this integration and clean up the library's api and do some more tests i'll cut a 0.3 release
| //Wait should wait for I/O consumption, but if the execution is too fast | ||
| //e.g. echo 'hello world' the output will not reach the pipe | ||
| //TODO Fix this | ||
| // Wait should wait for I/O consumption, but if the execution is too fast |
There was a problem hiding this comment.
sorry i have gofumpt enabled with gopls 😅
| } | ||
|
|
||
| func (p *Process) setStartTime(startTime time.Time) { |
There was a problem hiding this comment.
i should use this value to reset the watchman subscription clock
| @@ -254,6 +263,11 @@ func (p *Process) isRestartable() bool { | |||
| p.Lock() | |||
| exitCode := p.getExitCode() | |||
| p.Unlock() | |||
|
|
|||
| if p.isDevRestart.Swap(false) { | |||
There was a problem hiding this comment.
i needed a new signal for when the restart is associated with a watch. i thought about changing isStopped to some stopKind enum ie:
type stopKind u8
const (
notStopped stopKind = iota
isStopped
isDevRestart
)
this is probably still the better solution (especially since p.isStopped.Swap(false) is missing in this if)
| } | ||
|
|
||
| if notif.IsFreshInstance() { | ||
| log.Debug().Msg("fresh instance notification, ignoring") |
There was a problem hiding this comment.
oops forgot continues
| files := []string{} | ||
| for _, rawFile := range notif.Files() { | ||
| // if the query fields change, this will need to be a map[string]any | ||
| file := rawFile.(string) | ||
| files = append(files, file) | ||
| } | ||
|
|
||
| for _, sub := range subs { | ||
| sub.updates <- files | ||
| } |
There was a problem hiding this comment.
should change this to send to the channel associated with a subscription name. the name should be something like process-compose-dev-{process config name}
| config *types.Watch | ||
| } | ||
|
|
||
| func (s *WatchmanSub) Recv() ([]string, bool) { |
There was a problem hiding this comment.
should change this to only receive on the channel. data associated with a subscription should be tracked by watchman, not in the layer adapting the client
| Glob string `yaml:"path"` | ||
| Ignore []string `yaml:"ignore"` |
There was a problem hiding this comment.
not quite the same as docker-compose's. docker-compose uses just a Path where i'm using a Glob - i was thinking it might be more convenient to do a glob, but a path might make more sense?
|
been putting this together, i forked https://github.com/sjansen/watchman for this lmao
primary motivation is as an analog to the Develop spec in the docker-compose spec.
at work, we use process-compose for our dev workflow, and the only part of my workflow that's missing is filesystem watching for restarting services. while this could be implemented as a shell script, i think it would be great if process-compose directly integrated with watchman specifically for this feature so as not to eat up precious inotify allotments (we do a lot of virtualization).
happy to accept feedback - i know that no issue was opened, nor has there been any discussion of such a feature. it's a little project i gave myself for my own sake as i'd like to use process-compose in a personal project :)