fix: honour the global run setting from an included Taskfile - #3005
Open
no-hup wants to merge 1 commit into
Open
fix: honour the global run setting from an included Taskfile#3005no-hup wants to merge 1 commit into
no-hup wants to merge 1 commit into
Conversation
Taskfile.Merge already pushes the included file's global silent and use_gitignore settings down onto its tasks, but never did the same for run. Because GetHash falls back to the root Taskfile's run value, an included Taskfile's top-level 'run: once' was silently ignored and its tasks executed once per call instead of once per run. Tasks that set run explicitly are unaffected. Fixes go-task#1738
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1738
The problem
If an included Taskfile sets
runat the top level rather than per-task, the setting is dropped when it's included. Its tasks run once per call instead of once per run — no warning, no error. Running that same Taskfile directly works fine, which makes it a confusing one to track down.Why it happens
Taskfile.Mergealready pushes two of the included file's global settings down onto its tasks —silentanduse_gitignore— but never looks atRun:https://github.com/go-task/task/blob/main/taskfile/ast/taskfile.go#L63-L79
Downstream,
GetHashfalls back to the root Taskfile:After
graph.Merge(),e.Taskfileis the root file, so an included file'sRunis unreachable from there.The change
One block in
Merge, mirroring thesilentblock directly above it: when the included Taskfile setsrun, apply it to its own tasks that don't setrunthemselves. Tasks with an explicitrun:are untouched.@trulede proposed this same fix in #2548 and described the cause in the issue thread — that PR was closed without tests, so I've added the coverage it was missing.
Tests
New
testdata/run_once_included/fixture: a root Taskfile including a library Taskfile that setsrun: onceglobally, withservice-aandservice-bboth depending onlibrary:build. The test sits next toTestRunOnceSharedDepsand follows the same style.go test ./...→ 817 passed across 33 packages;go vet ./...clean.Behaviour change worth calling out
This does change behaviour for anyone currently affected by the bug, and I'd rather flag it than have it surface in review:
run: oncewill now actually run its tasks once. If someone was calling such a task repeatedly with differentvarsand relying on it re-running, they'll now get one execution —run: oncehashes on task name only, not vars.runand an included file sets a different one, the included file's value now wins for its own tasks. That's the same precedencesilentanduse_gitignorealready have, so this makesrunconsistent with its siblings rather than introducing a new rule.Both follow from honouring the setting at all, but if you'd prefer the root's global to keep winning, that's a one-line change and I'm happy to make it.
Disclosure: I used an AI assistant while writing and testing this change. I've reviewed it and verified the behaviour myself, and I'm happy to explain or revise anything here.