spin icon indicating copy to clipboard operation
spin copied to clipboard

Support exclusions in spin watch configuration

Open mikkelhegn opened this issue 2 years ago • 3 comments

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.

mikkelhegn avatar Aug 14 '23 19:08 mikkelhegn

Similar to the syntax of .gitignore, I'd imaging us supporting "!build".

kate-goldenring avatar Aug 24 '23 02:08 kate-goldenring

Looks like we have ignore_patterns in our watch Config. @calebschoepp do we already have support for ignoring certain paths?

kate-goldenring avatar Aug 24 '23 02:08 kate-goldenring

@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.

calebschoepp avatar Aug 24 '23 16:08 calebschoepp

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

image

Appreciate your input! :D

cc: @itowlson

me-diru avatar Jun 12 '24 23:06 me-diru

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.

itowlson avatar Jun 12 '24 23:06 itowlson

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.

itowlson avatar Jun 13 '24 00:06 itowlson

Huh. Seems like the ! prefix does work. I better claim this as an achievement quick.

itowlson avatar Jun 13 '24 00:06 itowlson

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.

me-diru avatar Jun 13 '24 00:06 me-diru

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.

itowlson avatar Jun 13 '24 00:06 itowlson

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.

itowlson avatar Jun 13 '24 00:06 itowlson

watchexec --filter-file also allows (undocumented?) for the ! syntax and passes it unmodified into GlobsetFilterer so here we are I guess.

itowlson avatar Jun 13 '24 00:06 itowlson

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.

itowlson avatar Jun 13 '24 01:06 itowlson

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.

I think that behaviour is reasonable.

calebschoepp avatar Jun 13 '24 17:06 calebschoepp

@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.

itowlson avatar Jun 13 '24 22:06 itowlson

okay nobody has shrieked at me about #2554 not working so gonna MASH THAT CLOSE BUTTON

itowlson avatar Jun 25 '24 02:06 itowlson