From 148b090d8e729d4ad02cd31c4d78831cf2e968e9 Mon Sep 17 00:00:00 2001
From: Pete Davison <pd93.uk@outlook.com>
Date: Mon, 4 Nov 2024 13:30:39 +0000
Subject: [PATCH] fix: bug where non-nil, empty dynamic variables are returned
 as an empty interface (#1904)

---
 internal/compiler/compiler.go | 15 ++++++++++-----
 taskfile/ast/var.go           | 10 +++++-----
 variables.go                  |  4 ++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go
index 67ea07e456..5fc8e989da 100644
--- a/internal/compiler/compiler.go
+++ b/internal/compiler/compiler.go
@@ -74,8 +74,8 @@ func (c *Compiler) getVariables(t *ast.Task, call *ast.Call, evaluateShVars bool
 			if err := cache.Err(); err != nil {
 				return err
 			}
-			// If the variable is not dynamic, we can set it and return
-			if newVar.Value != nil || newVar.Sh == "" {
+			// If the variable is already set, we can set it and return
+			if newVar.Value != nil {
 				result.Set(k, ast.Var{Value: newVar.Value})
 				return nil
 			}
@@ -136,10 +136,15 @@ func (c *Compiler) HandleDynamicVar(v ast.Var, dir string) (string, error) {
 	c.muDynamicCache.Lock()
 	defer c.muDynamicCache.Unlock()
 
+	// If the variable is not dynamic or it is empty, return an empty string
+	if v.Sh == nil || *v.Sh == "" {
+		return "", nil
+	}
+
 	if c.dynamicCache == nil {
 		c.dynamicCache = make(map[string]string, 30)
 	}
-	if result, ok := c.dynamicCache[v.Sh]; ok {
+	if result, ok := c.dynamicCache[*v.Sh]; ok {
 		return result, nil
 	}
 
@@ -150,7 +155,7 @@ func (c *Compiler) HandleDynamicVar(v ast.Var, dir string) (string, error) {
 
 	var stdout bytes.Buffer
 	opts := &execext.RunCommandOptions{
-		Command: v.Sh,
+		Command: *v.Sh,
 		Dir:     dir,
 		Stdout:  &stdout,
 		Stderr:  c.Logger.Stderr,
@@ -164,7 +169,7 @@ func (c *Compiler) HandleDynamicVar(v ast.Var, dir string) (string, error) {
 	result := strings.TrimSuffix(stdout.String(), "\r\n")
 	result = strings.TrimSuffix(result, "\n")
 
-	c.dynamicCache[v.Sh] = result
+	c.dynamicCache[*v.Sh] = result
 	c.Logger.VerboseErrf(logger.Magenta, "task: dynamic variable: %q result: %q\n", v.Sh, result)
 
 	return result, nil
diff --git a/taskfile/ast/var.go b/taskfile/ast/var.go
index dce6288452..88d407943e 100644
--- a/taskfile/ast/var.go
+++ b/taskfile/ast/var.go
@@ -20,7 +20,7 @@ type Vars struct {
 func (vs *Vars) ToCacheMap() (m map[string]any) {
 	m = make(map[string]any, vs.Len())
 	_ = vs.Range(func(k string, v Var) error {
-		if v.Sh != "" {
+		if v.Sh != nil && *v.Sh != "" {
 			// Dynamic variable is not yet resolved; trigger
 			// <no value> to be used in templates.
 			return nil
@@ -81,7 +81,7 @@ func (vs *Vars) DeepCopy() *Vars {
 type Var struct {
 	Value any
 	Live  any
-	Sh    string
+	Sh    *string
 	Ref   string
 	Dir   string
 }
@@ -98,7 +98,7 @@ func (v *Var) UnmarshalYAML(node *yaml.Node) error {
 			// If the value is a string and it starts with $, then it's a shell command
 			if str, ok := value.(string); ok {
 				if str, ok = strings.CutPrefix(str, "$"); ok {
-					v.Sh = str
+					v.Sh = &str
 					return nil
 				}
 				if str, ok = strings.CutPrefix(str, "#"); ok {
@@ -118,7 +118,7 @@ func (v *Var) UnmarshalYAML(node *yaml.Node) error {
 				switch key {
 				case "sh", "ref", "map":
 					var m struct {
-						Sh  string
+						Sh  *string
 						Ref string
 						Map any
 					}
@@ -150,7 +150,7 @@ func (v *Var) UnmarshalYAML(node *yaml.Node) error {
 		switch key {
 		case "sh", "ref":
 			var m struct {
-				Sh  string
+				Sh  *string
 				Ref string
 			}
 			if err := node.Decode(&m); err != nil {
diff --git a/variables.go b/variables.go
index 6dac3a6c3e..67703e810b 100644
--- a/variables.go
+++ b/variables.go
@@ -112,7 +112,7 @@ func (e *Executor) compiledTask(call *ast.Call, evaluateShVars bool) (*ast.Task,
 	if evaluateShVars {
 		err = new.Env.Range(func(k string, v ast.Var) error {
 			// If the variable is not dynamic, we can set it and return
-			if v.Value != nil || v.Sh == "" {
+			if v.Value != nil || v.Sh == nil {
 				new.Env.Set(k, ast.Var{Value: v.Value})
 				return nil
 			}
@@ -301,7 +301,7 @@ func itemsFromFor(
 			// If the variable is dynamic, then it hasn't been resolved yet
 			// and we can't use it as a list. This happens when fast compiling a task
 			// for use in --list or --list-all etc.
-			if v.Value != nil && v.Sh == "" {
+			if v.Value != nil && v.Sh == nil {
 				switch value := v.Value.(type) {
 				case string:
 					if f.Split != "" {