Revert "simplify getVariables() and improve nested variables support"

This reverts commit 9619c7f54d.

task_test.go

@@ -72,16 +72,17 @@ func TestVars(t *testing.T) {
 		Target:    "default",
 		TrimSpace: true,
 		Files: map[string]string{
+			// hello task:
 			"foo.txt":              "foo",
 			"bar.txt":              "bar",
 			"baz.txt":              "baz",
 			"tmpl_foo.txt":         "foo",
-			"tmpl_bar.txt":         "bar",
+			"tmpl_bar.txt":         "<no value>",
 			"tmpl_foo2.txt":        "foo2",
 			"tmpl_bar2.txt":        "bar2",
 			"shtmpl_foo.txt":       "foo",
 			"shtmpl_foo2.txt":      "foo2",
-			"nestedtmpl_foo.txt":   "<no value>",
+			"nestedtmpl_foo.txt":   "{{.FOO}}",
 			"nestedtmpl_foo2.txt":  "foo2",
 			"foo2.txt":             "foo2",
 			"bar2.txt":             "bar2",
@@ -89,10 +90,10 @@ func TestVars(t *testing.T) {
 			"tmpl2_foo.txt":        "<no value>",
 			"tmpl2_foo2.txt":       "foo2",
 			"tmpl2_bar.txt":        "<no value>",
-			"tmpl2_bar2.txt":       "bar2",
+			"tmpl2_bar2.txt":       "<no value>",
 			"shtmpl2_foo.txt":      "<no value>",
 			"shtmpl2_foo2.txt":     "foo2",
-			"nestedtmpl2_foo2.txt": "<no value>",
+			"nestedtmpl2_foo2.txt": "{{.FOO2}}",
 			"override.txt":         "bar",
 		},
 	}

variables.go

@@ -85,49 +85,82 @@ func (v *Var) UnmarshalYAML(unmarshal func(interface{}) error) error {
 }
 
 // getVariables returns fully resolved variables following the priority order:
-// 1. Task variables
+// 1. Call variables (should already have been resolved)
-// 2. Call variables
+// 2. Environment (should not need to be resolved)
-// 3. Taskvars file variables
+// 3. Task variables, resolved with access to:
-// 4. Environment variables
+//    - call, taskvars and environment variables
+// 4. Taskvars variables, resolved with access to:
+//    - environment variables
 func (e *Executor) getVariables(call Call) (Vars, error) {
 	t, ok := e.Tasks[call.Task]
 	if !ok {
-		return nil, &taskNotFoundError{taskName: call.Task}
+		return nil, &taskNotFoundError{call.Task}
 	}
-	vr := varResolver{e: e, vars: getEnvironmentVariables()}
-	vr.merge(e.taskvars)
-	vr.merge(e.taskvars)
-	vr.merge(call.Vars)
-	vr.merge(call.Vars)
-	vr.merge(t.Vars)
-	vr.merge(t.Vars)
-	return vr.vars, vr.err
-}
 
-type varResolver struct {
+	merge := func(dest Vars, srcs ...Vars) {
-	e    *Executor
+		for _, src := range srcs {
-	vars Vars
+			for k, v := range src {
-	err  error
+				dest[k] = v
-}
+			}
+		}
+	}
+	varsKeys := func(srcs ...Vars) []string {
+		m := make(map[string]struct{})
+		for _, src := range srcs {
+			for k := range src {
+				m[k] = struct{}{}
+			}
+		}
+		lst := make([]string, 0, len(m))
+		for k := range m {
+			lst = append(lst, k)
+		}
+		return lst
+	}
+	replaceVars := func(dest Vars, keys []string) error {
+		r := varReplacer{vars: dest}
+		for _, k := range keys {
+			v := dest[k]
+			dest[k] = Var{
+				Static: r.replace(v.Static),
+				Sh:     r.replace(v.Sh),
+			}
+		}
+		return r.err
+	}
+	resolveShell := func(dest Vars, keys []string) error {
+		for _, k := range keys {
+			v := dest[k]
+			static, err := e.handleShVar(v)
+			if err != nil {
+				return err
+			}
+			dest[k] = Var{Static: static}
+		}
+		return nil
+	}
+	update := func(dest Vars, srcs ...Vars) error {
+		merge(dest, srcs...)
+		// updatedKeys ensures template evaluation is run only once.
+		updatedKeys := varsKeys(srcs...)
+		if err := replaceVars(dest, updatedKeys); err != nil {
+			return err
+		}
+		return resolveShell(dest, updatedKeys)
+	}
 
-func (vr *varResolver) merge(vars Vars) {
+	// Resolve taskvars variables to "result" with environment override variables.
-	if vr.err != nil {
+	override := getEnvironmentVariables()
-		return
+	result := make(Vars, len(e.taskvars)+len(t.Vars)+len(override))
+	if err := update(result, e.taskvars, override); err != nil {
+		return nil, err
 	}
-	r := varReplacer{vars: vr.vars}
+	// Resolve task variables to "result" with environment and call override variables.
-	for k, v := range vars {
+	merge(override, call.Vars)
-		v = Var{
+	if err := update(result, t.Vars, override); err != nil {
-			Static: r.replace(v.Static),
+		return nil, err
-			Sh:     r.replace(v.Sh),
-		}
-		static, err := vr.e.handleShVar(v)
-		if err != nil {
-			vr.err = err
-			return
-		}
-		vr.vars[k] = Var{Static: static}
 	}
-	vr.err = r.err
+	return result, nil
 }
 
 func (e *Executor) handleShVar(v Var) (string, error) {