Skip to content

Commit 067688f

Browse files
committed
scope included vars to their namespace
1 parent 323bfc6 commit 067688f

7 files changed

Lines changed: 471 additions & 41 deletions

File tree

AGENTS.md

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ The Go toolchain version comes from `go.mod` (`go 1.26.3`). Tests run with
122122
env → preconditions → up-to-date → cmds), and cover it with a literal
123123
`taskfile.Config` test in `run_test.go` plus a `Parse`-based test in
124124
`parse_test.go` if YAML shape matters.
125-
- **Touching env/var resolution**: respect the existing precedence
126125
- **Touching env/var resolution**: respect the existing precedence
127126
(`BaseEnv` < parent task env < task dotenv < task vars < task env) and the
128127
rule that *task dotenv never overrides global dotenv or OS env* (see
@@ -135,13 +134,23 @@ The Go toolchain version comes from `go.mod` (`go 1.26.3`). Tests run with
135134
resolve lazily through a recursive lookup in `resolveAllVars` (see
136135
`taskfile/vars.go`); they may reference each other and the built-in
137136
`GIT_*` family transitively, declaration order is irrelevant, and
138-
cycles short-circuit to the empty string. The built-in `GIT_*`
139-
resolver in `taskfile/gitvars.go` is wired through
140-
`Runner.builtinLookup` and is consulted *after* user vars / CLI_ARGS
141-
but *before* the process environment by `expandVars`,
142-
`resolveEnvValue`, and `checkRequires`. Watch mode must call
143-
`ResetRan` between iterations — it also clears the gitVars cache so
144-
`{{.GIT_DIRTY}}` re-evaluates after each edit.
137+
cycles short-circuit to the empty string. **Vars are namespace-scoped**:
138+
a var declared in an included `gogo.yaml` lives in
139+
`Config.NamespaceVars[namespace]` (with its own working dir in
140+
`Config.NamespaceDirs[namespace]` for `sh:` resolution) and is visible
141+
only to tasks at or below that namespace — sibling includes never see
142+
each other's vars (see `TestNamespacedVarsIsolateSiblings`). Root vars
143+
in `Config.Vars` stay visible everywhere. The most-specific namespace
144+
wins on collisions, so `proxy:` and `metrics:` can each declare their
145+
own `LDFLAGS` without clobbering. The built-in `GIT_*` resolver in
146+
`taskfile/gitvars.go` is wired through `Runner.builtinLookup` and is
147+
consulted *after* user vars / CLI_ARGS but *before* the process
148+
environment by `expandVars`, `resolveEnvValue`, and `checkRequires`.
149+
Unresolved `${VAR}` references survive verbatim through
150+
`unknownShellVarSpan`; in particular `$2` (awk positional) is **not**
151+
rewritten to `${2}` (see `TestShVarPreservesAwkPositional`). Watch mode
152+
must call `ResetRan` between iterations — it also clears the gitVars
153+
cache so `{{.GIT_DIRTY}}` re-evaluates after each edit.
145154
- **Touching include logic**: cycles must be detected by absolute dir
146155
(`includeStack`), nested namespaces are colon-joined
147156
(`parent:child:grandchild`), and dotenv files dedupe globally via

taskfile/includes.go

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ func newIncludeLoader(root *Config) (*includeLoader, error) {
7171

7272
func (l *includeLoader) load() (*Config, error) {
7373
l.root.Namespaces = make(map[string]string)
74+
l.root.NamespaceDirs = make(map[string]string)
75+
l.root.NamespaceVars = make(map[string]map[string]Var)
7476

7577
// Process flatten files first (their tasks merge into the root namespace).
7678
// Order vs. includes doesn't matter for correctness because both honor
@@ -167,7 +169,7 @@ func (l *includeLoader) loadInclude(req includeRequest) error {
167169
}
168170
}
169171

170-
l.mergeVars(included.Vars)
172+
l.mergeVars(included.Namespace, included.Dir, included.Vars)
171173
l.mergeSourcePresets(included.Sources)
172174
l.mergeSecrets(included.Secrets)
173175
return l.mergeTasks(included)
@@ -199,6 +201,7 @@ func (l *includeLoader) parseInclude(req includeRequest) (*includedConfig, error
199201
Parent: req,
200202
}
201203
l.root.Namespaces[childDir] = req.namespace
204+
l.root.NamespaceDirs[req.namespace] = childDir
202205
return included, nil
203206
}
204207

@@ -303,7 +306,7 @@ func (l *includeLoader) loadFlatten(req flattenRequest) error {
303306
}
304307
}
305308

306-
l.mergeVars(flattened.Vars)
309+
l.mergeVars(req.namespace, req.ancestorDir, flattened.Vars)
307310
l.mergeSourcePresets(flattened.Sources)
308311
return l.mergeFlattenedTasks(flattened, req.namespace, req.ancestorDir)
309312
}
@@ -315,17 +318,40 @@ func namespaceJoin(prefix, name string) string {
315318
return prefix + ":" + name
316319
}
317320

318-
// mergeVars merges child global vars into the root. Root vars win conflicts.
319-
func (l *includeLoader) mergeVars(vars map[string]Var) {
321+
// mergeVars merges vars declared by an included or flattened file into the
322+
// scope that owns them. Vars from the root or root-level flatten files
323+
// (namespace "") land in Config.Vars and are visible everywhere; vars from
324+
// an include land in Config.NamespaceVars[namespace] and are visible only
325+
// to tasks that live at or below that namespace — so two sibling includes
326+
// can each declare their own LDFLAGS/IMAGE_NAME without leaking values
327+
// into each other. "First defined wins" still holds within each scope,
328+
// matching the prior root-merge precedence.
329+
func (l *includeLoader) mergeVars(namespace, dir string, vars map[string]Var) {
320330
if len(vars) == 0 {
321331
return
322332
}
323-
if l.root.Vars == nil {
324-
l.root.Vars = make(map[string]Var)
333+
if namespace == "" {
334+
if l.root.Vars == nil {
335+
l.root.Vars = make(map[string]Var)
336+
}
337+
for k, v := range vars {
338+
if _, exists := l.root.Vars[k]; !exists {
339+
l.root.Vars[k] = v
340+
}
341+
}
342+
return
343+
}
344+
if _, ok := l.root.NamespaceDirs[namespace]; !ok {
345+
l.root.NamespaceDirs[namespace] = dir
346+
}
347+
scoped := l.root.NamespaceVars[namespace]
348+
if scoped == nil {
349+
scoped = make(map[string]Var)
350+
l.root.NamespaceVars[namespace] = scoped
325351
}
326352
for k, v := range vars {
327-
if _, exists := l.root.Vars[k]; !exists {
328-
l.root.Vars[k] = v
353+
if _, exists := scoped[k]; !exists {
354+
scoped[k] = v
329355
}
330356
}
331357
}

taskfile/parse_test.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,13 @@ tasks:
184184
tf, err := LoadWithIncludes(dir)
185185
require.NoError(t, err)
186186

187-
assert.Contains(t, tf.Vars, "VERSION")
188-
assert.Equal(t, "1.2.3", tf.Vars["VERSION"].Value)
187+
// Child vars are scoped to their include's namespace, not leaked into the
188+
// root — a sibling include can declare its own VERSION without clashing.
189+
assert.NotContains(t, tf.Vars, "VERSION", "included vars must not leak into the root namespace")
190+
require.Contains(t, tf.NamespaceVars, "child")
191+
assert.Contains(t, tf.NamespaceVars["child"], "VERSION")
192+
assert.Equal(t, "1.2.3", tf.NamespaceVars["child"]["VERSION"].Value)
193+
assert.Equal(t, filepath.Join(dir, "child"), tf.NamespaceDirs["child"])
189194
}
190195

191196
func TestLoadWithIncludesNestedNamespaceCollision(t *testing.T) {

0 commit comments

Comments
 (0)