Skip to content

Watcher does not ignore created files that do not match sources.#2743

Draft
trulede wants to merge 1 commit intogo-task:mainfrom
trulede:PR/watch-new-files
Draft

Watcher does not ignore created files that do not match sources.#2743
trulede wants to merge 1 commit intogo-task:mainfrom
trulede:PR/watch-new-files

Conversation

@trulede
Copy link
Contributor

@trulede trulede commented Mar 15, 2026

@mkeeler I wanted to split a part of your PR out, and combine with PR from @butuzov.

I've streamlined it a little, but the principle is the same. If you @mkeeler are happy with that either open a PR with only this part and the tests, or indicate that I should bring the tests into this PR. Either way I would put your name in the commit as author.

Related #2740 and #2742

@trulede
Copy link
Contributor Author

trulede commented Mar 15, 2026

We need to consider the change in PR #2493 which might be a variation of the same problem.

e.Logger.VerboseErrf(logger.Magenta, "task: event skipped for being an ignored dir: %s\n", event.Name)
continue
}
if event.Has(fsnotify.Remove) || event.Has(fsnotify.Rename) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if event.Has(fsnotify.Remove) || event.Has(fsnotify.Rename) {
if event.Has(fsnotify.Remove) || event.Has(fsnotify.Rename) || event.Has(fsnotify.Write) {

Without this I think writes to existing files that are not in the sources would still trigger cancellation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks. I've added that.

@mkeeler
Copy link

mkeeler commented Mar 16, 2026

@trulede Feel free to bring the tests over here if you like. What about the second half to my original PR to fix the task fingerprinting uniqueness issue? Should I refactor my PR to only have that fix within it.

@trulede
Copy link
Contributor Author

trulede commented Mar 16, 2026

@mkeeler I've copied over the watch_tests and will get them running and then update this PR.

Yes, refactor your PR to have the fix for the checksums.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants