Skip to content

Watch: restarts tasks only when at least one watched file changes. #2493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trulede wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −34
Open

Watch: restarts tasks only when at least one watched file changes. #2493

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 39 additions & 34 deletions watch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,41 +78,46 @@ func (e *Executor) watchTasks(calls ...*Call) error {
}
e.Logger.VerboseErrf(logger.Magenta, "task: received watch event: %v\n", event)

cancel()
ctx, cancel = context.WithCancel(context.Background())

e.Compiler.ResetCache()

// Count the tasks that would be restarted by the watch (_before_ cancelling them).
watchCount := 0
for _, c := range calls {
go func() {
if ShouldIgnore(event.Name) {
e.Logger.VerboseErrf(logger.Magenta, "task: event skipped for being an ignored dir: %s\n", event.Name)
return
}
t, err := e.GetTask(c)
if err != nil {
e.Logger.Errf(logger.Red, "%v\n", err)
return
}
baseDir := filepathext.SmartJoin(e.Dir, t.Dir)
files, err := e.collectSources(calls)
if err != nil {
e.Logger.Errf(logger.Red, "%v\n", err)
return
}

if !event.Has(fsnotify.Remove) && !slices.Contains(files, event.Name) {
relPath, _ := filepath.Rel(baseDir, event.Name)
e.Logger.VerboseErrf(logger.Magenta, "task: skipped for file not in sources: %s\n", relPath)
return
}
err = e.RunTask(ctx, c)
if err == nil {
e.Logger.Errf(logger.Green, "task: task \"%s\" finished running\n", c.Task)
} else if !isContextError(err) {
e.Logger.Errf(logger.Red, "%v\n", err)
}
}()
if ShouldIgnore(event.Name) {
e.Logger.VerboseErrf(logger.Magenta, "task: event skipped for being an ignored dir: %s\n", event.Name)
continue
}
Comment on lines 83 to +87
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

ShouldIgnore(event.Name) does not depend on the task call, but it's checked (and logged) once per calls entry, which can spam duplicate "ignored dir" messages on a single event. Consider moving the ignore check outside the for _, c := range calls loop and returning early when ignored.

Copilot uses AI. Check for mistakes.
t, err := e.GetTask(c)
if err != nil {
e.Logger.Errf(logger.Red, "%v\n", err)
continue
}
baseDir := filepathext.SmartJoin(e.Dir, t.Dir)
files, err := e.collectSources(calls)
if err != nil {
e.Logger.Errf(logger.Red, "%v\n", err)
continue
}
Comment on lines +88 to +98
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

e.collectSources(calls) is recomputed for every task in the loop, but it returns the same combined source list each time for this event. This adds unnecessary traversal/globbing work on every fsnotify event. Compute files once before iterating over calls (or short-circuit after the first match) to reduce per-event overhead.

Copilot uses AI. Check for mistakes.
if !event.Has(fsnotify.Remove) && !slices.Contains(files, event.Name) {
relPath, _ := filepath.Rel(baseDir, event.Name)
e.Logger.VerboseErrf(logger.Magenta, "task: skipped for file not in sources: %s\n", relPath)
continue
}
watchCount++
}
// Cancel _all_ watched tasks, get new context, then restart the tasks.
if watchCount > 0 {
cancel()
ctx, cancel = context.WithCancel(context.Background())
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

e.Compiler.ResetCache() was previously called on watch events before restarting tasks, which clears the dynamic variable cache. With the current change, watched tasks restart without resetting this cache, so dynamic vars may remain stale across restarts. Consider resetting the compiler cache inside the watchCount > 0 block before starting the new runs.

Suggested change
ctx, cancel = context.WithCancel(context.Background())
ctx, cancel = context.WithCancel(context.Background())
e.Compiler.ResetCache()

Copilot uses AI. Check for mistakes.
for _, c := range calls {
c := c
go func() {
err = e.RunTask(ctx, c)
if err == nil {
e.Logger.Errf(logger.Green, "task: task \"%s\" finished running\n", c.Task)
} else if !isContextError(err) {
e.Logger.Errf(logger.Red, "%v\n", err)
}
Comment on lines +112 to +118
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The goroutine assigns to the outer err variable (err = e.RunTask(...)). This introduces a data race between goroutines and also clobbers the function-scope err (used for watcher creation / error handling). Make err a new local inside the goroutine (e.g., err := ...) so each task run has its own error value.

Copilot uses AI. Check for mistakes.
}()
}
}
Comment on lines +81 to 121
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

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

The new behavior (only cancel/restart when the event matches at least one watched source) is not covered by the existing watch tests. Consider adding a watch_test.go case that triggers a fsnotify event on a file outside sources and asserts that no additional task run happens (and the current run isn't canceled).

Copilot uses AI. Check for mistakes.
case err, ok := <-w.Errors:
switch {
Expand Down
Loading