Support exclusions in spin watch configuration
In some circumstances, it might be nice to be able to "include everything, but" for the Spin watch configuration. Rather than having to explicitly include everything.
In this example: https://github.com/mikkelhegn/docusaurus-example (generated from a standard Docusaurus template), the build output is a subfolder to the my-websites folder, where all the source is. Which means if the whole directory is watched, it will loop at every build. To avoid the spin watch config would have to include all subdirectories and files in the root, for it to work well.
In this example, being able to tell Spin to watch everything but the build folder, would be a mush easier config.
Similar to the syntax of .gitignore, I'd imaging us supporting "!build".
Looks like we have ignore_patterns in our watch Config. @calebschoepp do we already have support for ignoring certain paths?
@kate-goldenring currently the ignored paths are hardcoded and we don't support any way for users to pass in paths that they want to ignore. I do think that supporting some sort of ignore path is a good idea. Perhaps the simplest implementation is just treating watch paths with a ! prefix as something to be ignored like Kate mentioned.
Hi!
The code has gone through some refactor from the time this issue was raised. After some initial testing, I think the existing code implements @kate-goldenring and @calebschoepp suggestion of using ! in front of the file/folder in watch files.
For example, when I included everything in the file and used !filename.rs, it is excluded from spin watch, similarly can exclude a folder with !src\*
Would like to know your thoughts on if covers the issue or anything else/edge-cases to look out
Appreciate your input! :D
cc: @itowlson
I think none of your patterns cover spin.toml - your wildcard is */* which I would expect to cover subdirectories only.
But spin.toml should be watched anyway. A component shouldn't be able to ignore away manifest changes.
I don't see anything checking for ! patterns unless it's already handled by the watchexec glob filter. But there's a separate checker for spin.toml which should be reloading when that file changes regardless of anything in the build.watch lists. I'll try to reproduce - thanks for the heads up.
Huh. Seems like the ! prefix does work. I better claim this as an achievement quick.
spin.toml was a bad example to show my intention, but I am glad it wasn't supposed to happen :D
In general, it was interesting to see files/folders getting excluded as I could not see anything in the existing code explicitly handling this case. Maybe it is already handled by the watchexec glob filter as you mentioned.
Okay something weird is going on with spin.toml.
Also, the magic exclude is actually a problem, because we use a single watcher for globs, meaning the lists get combined. Consider two components, one of which watches ["utils/*.rs"] and the other one of which watches ["!utils/useful.rs"]. Because we combine the lists, changing utils/useful.rs does not trigger a rebuild, even though the first component does care about it.
I guess we will need to split out a watcher per component but man that's going to cause a load of pain for synchronising notifications.
The exclamation mark syntax isn't mentioned in the docs for watchexec (https://watchexec.github.io/docs/glob-patterns.html) or the underlying glob library (https://docs.rs/globset/latest/globset/). And the globset filterer docs have a separate parameter for providing exclusions (https://docs.rs/watchexec-filterer-globset/latest/watchexec_filterer_globset/struct.GlobsetFilterer.html#method.new). I'm not sure I trust my understanding of what's going on here.
watchexec --filter-file also allows (undocumented?) for the ! syntax and passes it unmodified into GlobsetFilterer so here we are I guess.
Okay the situation with spin.toml is a bit more nuanced than it appeared. We do still monitor for changes in spin.toml so that we can reconfigure the watchers on a manifest change. All that is working as intended. However, we also implicitly watch spin.toml as part of each component's build and artifact filters, so that we can rebuild/reload if anything interesting happens (e.g. the build.command changes or the files section changes). And adding !spin.toml overrides that, meaning that changes to spin.toml no longer cause a rebuild, making it look like we are no longer responding to manifest changes. But we are - just not to trigger rebuilds for that component.
I'm not too concerned about this. Now I understand that it's limited to "this component will no longer rebuild/reload on a spin.toml change," it seems like an edge case where the user gets what they asked for even if they regret asking for it. Let me know if you feel differently about it.
What I am concerned about is that exclusions on one component override inclusions on other components. That's a nasty little surprise for anyone who uses the exclusion syntax.
Okay the situation with
spin.tomlis a bit more nuanced than it appeared. We do still monitor for changes inspin.tomlso that we can reconfigure the watchers on a manifest change. All that is working as intended. However, we also implicitly watchspin.tomlas part of each component'sbuildandartifactfilters, so that we can rebuild/reload if anything interesting happens (e.g. thebuild.commandchanges or thefilessection changes). And adding!spin.tomloverrides that, meaning that changes tospin.tomlno longer cause a rebuild, making it look like we are no longer responding to manifest changes. But we are - just not to trigger rebuilds for that component.I'm not too concerned about this. Now I understand that it's limited to "this component will no longer rebuild/reload on a spin.toml change," it seems like an edge case where the user gets what they asked for even if they regret asking for it. Let me know if you feel differently about it.
I think that behaviour is reasonable.
@mikkelhegn Please give the latest Spin build a test and let me know if you run into any weirdness with the ! syntax. We have not done anything to enable it but seem to be inheriting it from the library we use, but it's not very documented and I am not sure whether to trust it enough to close this issue. If it's working well for you with the fixes in #2554 then we can close this.
okay nobody has shrieked at me about #2554 not working so gonna MASH THAT CLOSE BUTTON