Merge pull request #295 from lets-cli/feature/fix/env-sequential-eval

Fix sequential env evaluation and global context for command env

Author: kindermax

docs/docs/changelog.md

@@ -8,6 +8,7 @@ title: Changelog
 * `[Added]` Show similar command suggestions on typos.
 * `[Changed]` Exit code 2 on unknown command.
 * `[Removed]` Drop deprecated `eval_env` directive. Use `env` with `sh` execution mode instead.
+* `[Fixed]` Evaluate `env` entries sequentially so `sh` values can reference previously resolved env keys (including global env for command-level env).
 
 ## [0.0.59](https://github.com/lets-cli/lets/releases/tag/v0.0.59)
 

docs/docs/config.md

@@ -77,6 +77,8 @@ Specify global env for all commands.
 
 Env can be declared as static value or with execution mode:
 
+Env entries are evaluated sequentially in declaration order. `sh` can reference variables that are declared earlier in the same `env` block.
+
 Example:
 
 ```yaml
@@ -89,6 +91,16 @@ env:
     checksum: [Readme.md, package.json]
 ```
 
+Reference previously declared env:
+
+```yaml
+shell: bash
+env:
+  ENGINE: docker
+  COMPOSE:
+    sh: echo "${ENGINE}-compose"
+```
+
 ### Global before
 
 `key: before`
@@ -653,6 +665,8 @@ Env is as simple as it sounds. Define additional env for a command:
 
 Env can be declared as static value or with execution mode:
 
+Command `env` entries are also evaluated sequentially in declaration order. During command env evaluation, values from global `env` are available too.
+
 Example:
 
 ```yaml
@@ -669,6 +683,18 @@ commands:
     cmd: go build -o lets *.go
 ```
 
+Reference previously declared command env:
+
+```yaml
+commands:
+  up:
+    env:
+      ENGINE: docker
+      COMPOSE:
+        sh: echo "${ENGINE}-compose"
+    cmd: ${COMPOSE} up
+```
+
 
 ### `checksum`
 

internal/config/config/command.go

@@ -127,7 +127,7 @@ func (c *Command) UnmarshalYAML(unmarshal func(interface{}) error) error {
 }
 
 func (c *Command) GetEnv(cfg Config) (map[string]string, error) {
-	if err := c.Env.Execute(cfg); err != nil {
+	if err := c.Env.Execute(cfg, cfg.GetEnv()); err != nil {
 		return nil, err
 	}
 

internal/config/config/config.go

@@ -287,7 +287,7 @@ func (c *Config) GetEnv() map[string]string {
 // SetupEnv must be called once. It is not intended to be called
 // multiple times hence does not have mutex.
 func (c *Config) SetupEnv() error {
-	if err := c.Env.Execute(*c); err != nil {
+	if err := c.Env.Execute(*c, nil); err != nil {
 		return err
 	}
 

internal/config/config/env.go

@@ -3,6 +3,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"os"
 	"os/exec"
 	"slices"
 	"strings"
@@ -187,9 +188,26 @@ func (e *Envs) Set(key string, value Env) {
 	e.Mapping[key] = value
 }
 
+func convertEnvMapToList(envMap map[string]string) []string {
+	if len(envMap) == 0 {
+		return []string{}
+	}
+
+	envList := make([]string, 0, len(envMap))
+	for k, v := range envMap {
+		envList = append(envList, fmt.Sprintf("%s=%s", k, v))
+	}
+
+	return envList
+}
+
 // eval env value and trim result string.
-func executeScript(shell string, script string) (string, error) {
+func executeScript(shell string, script string, envMap map[string]string) (string, error) {
 	cmd := exec.Command(shell, "-c", script)
+	envList := os.Environ()
+	// Append resolved env last so it overrides process env keys (Go 1.21+ cmd.Env dedup: last value wins).
+	envList = append(envList, convertEnvMapToList(envMap)...)
+	cmd.Env = envList
 
 	out, err := cmd.Output()
 	if err != nil {
@@ -202,7 +220,7 @@ func executeScript(shell string, script string) (string, error) {
 
 // Execute executes env entries for sh scrips and calculate checksums
 // It is lazy and caches data on first call.
-func (e *Envs) Execute(cfg Config) error {
+func (e *Envs) Execute(cfg Config, baseEnv map[string]string) error {
 	if e == nil {
 		return nil
 	}
@@ -211,10 +229,15 @@ func (e *Envs) Execute(cfg Config) error {
 		return nil
 	}
 
+	resolvedEnv := cloneMap(baseEnv)
+	if resolvedEnv == nil {
+		resolvedEnv = make(map[string]string)
+	}
+
 	for _, key := range e.Keys {
 		env := e.Mapping[key]
 		if env.Sh != "" {
-			result, err := executeScript(cfg.Shell, env.Sh)
+			result, err := executeScript(cfg.Shell, env.Sh, resolvedEnv)
 			if err != nil {
 				return err
 			}
@@ -229,6 +252,8 @@ func (e *Envs) Execute(cfg Config) error {
 			env.Value = result
 			e.Mapping[key] = env
 		}
+
+		resolvedEnv[key] = env.Value
 	}
 	e.ready = true
 

internal/config/config/env_execute_test.go

@@ -0,0 +1,75 @@
+package config
+
+import "testing"
+
+func TestEnvsExecute(t *testing.T) {
+	cfg := Config{
+		Shell:   "bash",
+		WorkDir: ".",
+	}
+
+	t.Run("resolves env entries sequentially", func(t *testing.T) {
+		envs := &Envs{}
+		envs.Set("ENGINE", Env{Name: "ENGINE", Value: "docker"})
+		envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`})
+
+		err := envs.Execute(cfg, nil)
+		if err != nil {
+			t.Fatalf("unexpected execute error: %s", err)
+		}
+
+		if got := envs.Mapping["COMPOSE"].Value; got != "docker-compose" {
+			t.Fatalf("expected COMPOSE=docker-compose, got %q", got)
+		}
+	})
+
+	t.Run("uses base env for sh evaluation", func(t *testing.T) {
+		envs := &Envs{}
+		envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`})
+
+		err := envs.Execute(cfg, map[string]string{"ENGINE": "docker"})
+		if err != nil {
+			t.Fatalf("unexpected execute error: %s", err)
+		}
+
+		if got := envs.Mapping["COMPOSE"].Value; got != "docker-compose" {
+			t.Fatalf("expected COMPOSE=docker-compose, got %q", got)
+		}
+	})
+
+	t.Run("resolved lets env overrides process env", func(t *testing.T) {
+		t.Setenv("ENGINE", "podman")
+
+		envs := &Envs{}
+		envs.Set("ENGINE", Env{Name: "ENGINE", Value: "docker"})
+		envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`})
+
+		err := envs.Execute(cfg, nil)
+		if err != nil {
+			t.Fatalf("unexpected execute error: %s", err)
+		}
+
+		if got := envs.Mapping["COMPOSE"].Value; got != "docker-compose" {
+			t.Fatalf("expected COMPOSE=docker-compose, got %q", got)
+		}
+	})
+
+	t.Run("keeps cached values after first execution", func(t *testing.T) {
+		envs := &Envs{}
+		envs.Set("COMPOSE", Env{Name: "COMPOSE", Sh: `echo "${ENGINE}-compose"`})
+
+		err := envs.Execute(cfg, map[string]string{"ENGINE": "docker"})
+		if err != nil {
+			t.Fatalf("unexpected execute error: %s", err)
+		}
+
+		err = envs.Execute(cfg, map[string]string{"ENGINE": "podman"})
+		if err != nil {
+			t.Fatalf("unexpected execute error: %s", err)
+		}
+
+		if got := envs.Mapping["COMPOSE"].Value; got != "docker-compose" {
+			t.Fatalf("expected cached COMPOSE=docker-compose, got %q", got)
+		}
+	})
+}

tests/env_dependency.bats

@@ -0,0 +1,32 @@
+load test_helpers
+
+setup() {
+    load "${BATS_UTILS_PATH}/bats-support/load.bash"
+    load "${BATS_UTILS_PATH}/bats-assert/load.bash"
+    cd ./tests/env_dependency
+}
+
+@test "env_dependency: global env sh can use previously resolved global env" {
+    run lets global-env-dependency
+    assert_success
+    assert_line --index 0 "GLOBAL_COMPOSE=docker-compose"
+}
+
+@test "env_dependency: command env sh can use previously resolved command env" {
+    run lets command-env-dependency
+    assert_success
+    assert_line --index 0 "COMMAND_COMPOSE=podman-compose"
+}
+
+@test "env_dependency: command env sh can use global env" {
+    run lets command-env-uses-global
+    assert_success
+    assert_line --index 0 "COMMAND_COMPOSE=docker-compose"
+}
+
+@test "env_dependency: forward references stay unresolved with sequential evaluation" {
+    run env -u LETS_TEST_FORWARD_VAR lets command-forward-reference
+    assert_success
+    assert_line --index 0 "COMMAND_COMPOSE="
+    assert_line --index 1 "LETS_TEST_FORWARD_VAR=from-command-env"
+}

tests/env_dependency/lets.yaml

@@ -0,0 +1,32 @@
+shell: bash
+
+env:
+  ENGINE: docker
+  GLOBAL_COMPOSE:
+    sh: echo "${ENGINE}-compose"
+
+commands:
+  global-env-dependency:
+    cmd: echo "GLOBAL_COMPOSE=${GLOBAL_COMPOSE}"
+
+  command-env-dependency:
+    env:
+      ENGINE: podman
+      COMMAND_COMPOSE:
+        sh: echo "${ENGINE}-compose"
+    cmd: echo "COMMAND_COMPOSE=${COMMAND_COMPOSE}"
+
+  command-env-uses-global:
+    env:
+      COMMAND_COMPOSE:
+        sh: echo "${ENGINE}-compose"
+    cmd: echo "COMMAND_COMPOSE=${COMMAND_COMPOSE}"
+
+  command-forward-reference:
+    env:
+      COMMAND_COMPOSE:
+        sh: echo "${LETS_TEST_FORWARD_VAR}"
+      LETS_TEST_FORWARD_VAR: from-command-env
+    cmd: |
+      echo "COMMAND_COMPOSE=${COMMAND_COMPOSE}"
+      echo "LETS_TEST_FORWARD_VAR=${LETS_TEST_FORWARD_VAR}"