From 6df932c1a972c2bf0164a28596a9bb29fffe3dd9 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 21 Aug 2026 15:20:32 +0800 Subject: [PATCH 1/4] feat(execbroker): scope environment updates --- internal/execbroker/execbroker.go | 78 ++++++++++++ internal/execbroker/execbroker_test.go | 114 ++++++++++++++++++ internal/formula/formula.go | 2 + internal/formula/formula_test.go | 38 ++++++ .../formula/testdata/formula/env_llar.gox | 11 ++ x/autotools/autotools.go | 8 +- x/cmake/cmake.go | 8 +- x/pkgconfig/pkgconfig.go | 4 +- 8 files changed, 253 insertions(+), 10 deletions(-) create mode 100644 internal/formula/testdata/formula/env_llar.gox diff --git a/internal/execbroker/execbroker.go b/internal/execbroker/execbroker.go index 28e014f0..20d44703 100644 --- a/internal/execbroker/execbroker.go +++ b/internal/execbroker/execbroker.go @@ -66,6 +66,46 @@ func Do(scope Scope, fn func() error) error { return fn() } +// Getenv returns the value of key from the active command scope. Without a +// scope, it has the same behavior as os.Getenv. +func Getenv(key string) string { + id := goid.Get() + scopeMu.RLock() + scope, ok := scopes[id] + if ok && scope.Env != nil { + value := envValue(scope.Env, key) + scopeMu.RUnlock() + return value + } + scopeMu.RUnlock() + return os.Getenv(key) +} + +// Setenv sets key in the active command scope. The first scoped write copies +// the process environment so later commands inherit the update without +// changing the process-wide environment. Without a scope, it has the same +// behavior as os.Setenv. +func Setenv(key, value string) error { + if err := validateEnv(key, value); err != nil { + return err + } + + id := goid.Get() + scopeMu.Lock() + defer scopeMu.Unlock() + + scope, ok := scopes[id] + if !ok { + return os.Setenv(key, value) + } + if scope.Env == nil { + scope.Env = os.Environ() + } + scope.Env = setEnv(scope.Env, key, value) + scopes[id] = scope + return nil +} + // Println writes to the stdout configured for the active scope. func Println(a ...any) (int, error) { w := io.Writer(os.Stdout) @@ -182,3 +222,41 @@ func clone(in []string) []string { } return append([]string(nil), in...) } + +func envValue(env []string, key string) string { + prefix := key + "=" + for i := len(env) - 1; i >= 0; i-- { + if len(env[i]) >= len(prefix) && env[i][:len(prefix)] == prefix { + return env[i][len(prefix):] + } + } + return "" +} + +func setEnv(env []string, key, value string) []string { + prefix := key + "=" + for i := range env { + if len(env[i]) >= len(prefix) && env[i][:len(prefix)] == prefix { + env[i] = prefix + value + return env + } + } + return append(env, prefix+value) +} + +func validateEnv(key, value string) error { + if key == "" { + return fmt.Errorf("invalid environment variable name") + } + for i := 0; i < len(key); i++ { + if key[i] == '=' || key[i] == 0 { + return fmt.Errorf("invalid environment variable name %q", key) + } + } + for i := 0; i < len(value); i++ { + if value[i] == 0 { + return fmt.Errorf("invalid environment variable value for %q", key) + } + } + return nil +} diff --git a/internal/execbroker/execbroker_test.go b/internal/execbroker/execbroker_test.go index bf5f7ddf..95b92f23 100644 --- a/internal/execbroker/execbroker_test.go +++ b/internal/execbroker/execbroker_test.go @@ -236,6 +236,120 @@ func TestDoRestoresNestedScope(t *testing.T) { } } +func TestScopedGetenvAndSetenv(t *testing.T) { + key := "EXECBROKER_SCOPED_ENV_TEST" + if err := os.Setenv(key, "process"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Unsetenv(key) }) + + err := Do(Scope{}, func() error { + if got := Getenv(key); got != "process" { + t.Fatalf("Getenv before Setenv = %q, want process", got) + } + if err := Setenv(key, "scoped"); err != nil { + return err + } + if got := Getenv(key); got != "scoped" { + t.Fatalf("Getenv after Setenv = %q, want scoped", got) + } + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment = %q, want process", got) + } + var got string + prefix := key + "=" + for _, entry := range Command("command").Env { + if len(entry) >= len(prefix) && entry[:len(prefix)] == prefix { + got = entry[len(prefix):] + } + } + if got != "scoped" { + t.Fatalf("command environment = %q, want scoped", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if got := Getenv(key); got != "process" { + t.Fatalf("Getenv after scope = %q, want process", got) + } +} + +func TestScopedEnvironmentRestoresNestedScope(t *testing.T) { + key := "EXECBROKER_NESTED_ENV_TEST" + err := Do(Scope{Env: []string{key + "=outer"}}, func() error { + if got := Getenv(key); got != "outer" { + t.Fatalf("outer Getenv = %q, want outer", got) + } + if err := Do(Scope{Env: []string{key + "=inner"}}, func() error { + if got := Getenv(key); got != "inner" { + t.Fatalf("inner Getenv = %q, want inner", got) + } + return Setenv(key, "inner-updated") + }); err != nil { + return err + } + if got := Getenv(key); got != "outer" { + t.Fatalf("restored Getenv = %q, want outer", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +func TestScopedEnvironmentIsGoroutineLocal(t *testing.T) { + key := "EXECBROKER_GOROUTINE_ENV_TEST" + ready := make(chan struct{}, 2) + start := make(chan struct{}) + results := make(chan string, 2) + + for _, value := range []string{"one", "two"} { + value := value + go func() { + _ = Do(Scope{}, func() error { + if err := Setenv(key, value); err != nil { + return err + } + ready <- struct{}{} + <-start + results <- Getenv(key) + return nil + }) + }() + } + for range 2 { + <-ready + } + close(start) + + got := map[string]bool{<-results: true, <-results: true} + if !got["one"] || !got["two"] { + t.Fatalf("goroutine-scoped values = %v, want one and two", got) + } +} + +func TestScopedSetenvRejectsInvalidValues(t *testing.T) { + for _, test := range []struct { + label string + name string + value string + }{ + {label: "empty name", name: "", value: "value"}, + {label: "equals in name", name: "bad=name", value: "value"}, + {label: "nul in name", name: "bad\x00name", value: "value"}, + {label: "nul in value", name: "name", value: "bad\x00value"}, + } { + t.Run(test.label, func(t *testing.T) { + if err := Do(Scope{}, func() error { return Setenv(test.name, test.value) }); err == nil { + t.Fatalf("Setenv(%q, %q) error = nil", test.name, test.value) + } + }) + } +} + func TestDoReturnsError(t *testing.T) { want := errors.New("failed") if err := Do(Scope{}, func() error { return want }); !errors.Is(err, want) { diff --git a/internal/formula/formula.go b/internal/formula/formula.go index 0b75613b..5a494394 100644 --- a/internal/formula/formula.go +++ b/internal/formula/formula.go @@ -83,6 +83,8 @@ func loadFS(fs fs.ReadFileFS, path string) (*Formula, error) { // Formula types remain cached after loading, so later interpreters must not // reset the dynamic method slots used by earlier types. ctx := ixgo.NewContext(ixgo.SupportMultipleInterp) + ctx.RegisterExternal("os.Getenv", execbroker.Getenv) + ctx.RegisterExternal("os.Setenv", execbroker.Setenv) ctx.RegisterExternal("os/exec.Command", execbroker.Command) ctx.RegisterExternal("os/exec.CommandContext", execbroker.CommandContext) ctx.RegisterExternal("fmt.Println", execbroker.Println) diff --git a/internal/formula/formula_test.go b/internal/formula/formula_test.go index 5b356c75..b3a8337e 100644 --- a/internal/formula/formula_test.go +++ b/internal/formula/formula_test.go @@ -196,3 +196,41 @@ func TestFormulaPrintUsesBrokerScope(t *testing.T) { t.Fatalf("output = %q, want %q", got, want) } } + +func TestFormulaEnvironmentUsesBrokerScope(t *testing.T) { + const key = "FORMULA_ENV_TEST" + t.Setenv(key, "process") + + f, err := loadFS(os.DirFS("testdata/formula").(fs.ReadFileFS), "env_llar.gox") + if err != nil { + t.Fatalf("Load failed: %v", err) + } + + var stdout bytes.Buffer + err = execbroker.Do(execbroker.Scope{Stdout: &stdout}, func() error { + f.OnBuild(&formulapkg.Context{}) + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment = %q, want process", got) + } + var got string + prefix := key + "=" + for _, entry := range execbroker.Command("command").Env { + if len(entry) >= len(prefix) && entry[:len(prefix)] == prefix { + got = entry[len(prefix):] + } + } + if got != "formula" { + t.Fatalf("command environment = %q, want formula", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if got, want := stdout.String(), "process\nformula\n"; got != want { + t.Fatalf("output = %q, want %q", got, want) + } + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment after scope = %q, want process", got) + } +} diff --git a/internal/formula/testdata/formula/env_llar.gox b/internal/formula/testdata/formula/env_llar.gox new file mode 100644 index 00000000..810a7234 --- /dev/null +++ b/internal/formula/testdata/formula/env_llar.gox @@ -0,0 +1,11 @@ +import "os" + +id "test/env" + +fromVer "v1.0.0" + +onBuild ctx => { + echo os.getenv("FORMULA_ENV_TEST") + os.setenv("FORMULA_ENV_TEST", "formula")! + echo os.getenv("FORMULA_ENV_TEST") +} diff --git a/x/autotools/autotools.go b/x/autotools/autotools.go index bfa59b41..2cc0bae8 100644 --- a/x/autotools/autotools.go +++ b/x/autotools/autotools.go @@ -126,16 +126,16 @@ func prependPath(key, value string) { if runtime.GOOS == "windows" { sep = ";" } - if cur := os.Getenv(key); cur != "" { + if cur := execbroker.Getenv(key); cur != "" { value += sep + cur } - os.Setenv(key, value) + _ = execbroker.Setenv(key, value) } // appendFlag appends a space-separated flag to an env var. func appendFlag(key, flag string) { - if cur := os.Getenv(key); cur != "" { + if cur := execbroker.Getenv(key); cur != "" { flag = cur + " " + flag } - os.Setenv(key, flag) + _ = execbroker.Setenv(key, flag) } diff --git a/x/cmake/cmake.go b/x/cmake/cmake.go index 1c1625ec..d7767091 100644 --- a/x/cmake/cmake.go +++ b/x/cmake/cmake.go @@ -194,16 +194,16 @@ func prependPath(key, value string) { if runtime.GOOS == "windows" { sep = ";" } - if cur := os.Getenv(key); cur != "" { + if cur := execbroker.Getenv(key); cur != "" { value += sep + cur } - os.Setenv(key, value) + _ = execbroker.Setenv(key, value) } // appendFlag appends a space-separated flag to an env var. func appendFlag(key, flag string) { - if cur := os.Getenv(key); cur != "" { + if cur := execbroker.Getenv(key); cur != "" { flag = cur + " " + flag } - os.Setenv(key, flag) + _ = execbroker.Setenv(key, flag) } diff --git a/x/pkgconfig/pkgconfig.go b/x/pkgconfig/pkgconfig.go index 0e9b06c1..0c5f08d2 100644 --- a/x/pkgconfig/pkgconfig.go +++ b/x/pkgconfig/pkgconfig.go @@ -17,10 +17,10 @@ func Use(root string) { if _, err := os.Stat(dir); err != nil { return } - if current := os.Getenv("PKG_CONFIG_PATH"); current != "" { + if current := execbroker.Getenv("PKG_CONFIG_PATH"); current != "" { dir += string(os.PathListSeparator) + current } - os.Setenv("PKG_CONFIG_PATH", dir) + _ = execbroker.Setenv("PKG_CONFIG_PATH", dir) } // Lookup returns the compiler and linker flags for name. From 034d5edb705003e4c49d9cd1c8372cb8338612ce Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 21 Aug 2026 15:55:31 +0800 Subject: [PATCH 2/4] feat(execbroker): broker formula environment APIs --- internal/execbroker/execbroker.go | 108 ++++++++++++++++-- internal/execbroker/execbroker_test.go | 55 +++++++++ internal/formula/formula.go | 2 - internal/formula/formula_test.go | 6 +- .../formula/testdata/formula/env_llar.gox | 15 +++ internal/ixgo/exec.go | 27 +++++ internal/ixgo/exec_test.go | 71 ++++++++++++ 7 files changed, 270 insertions(+), 14 deletions(-) create mode 100644 internal/ixgo/exec_test.go diff --git a/internal/execbroker/execbroker.go b/internal/execbroker/execbroker.go index 20d44703..7e7ea5f5 100644 --- a/internal/execbroker/execbroker.go +++ b/internal/execbroker/execbroker.go @@ -81,14 +81,34 @@ func Getenv(key string) string { return os.Getenv(key) } +// LookupEnv returns the value of key and whether it is present in the active +// command scope. Without a scope, it has the same behavior as os.LookupEnv. +func LookupEnv(key string) (string, bool) { + id := goid.Get() + scopeMu.RLock() + scope, ok := scopes[id] + if ok && scope.Env != nil { + value, present := envLookup(scope.Env, key) + scopeMu.RUnlock() + return value, present + } + scopeMu.RUnlock() + return os.LookupEnv(key) +} + // Setenv sets key in the active command scope. The first scoped write copies // the process environment so later commands inherit the update without // changing the process-wide environment. Without a scope, it has the same // behavior as os.Setenv. func Setenv(key, value string) error { - if err := validateEnv(key, value); err != nil { + if err := validateEnvKey(key); err != nil { return err } + for i := 0; i < len(value); i++ { + if value[i] == 0 { + return fmt.Errorf("invalid environment variable value for %q", key) + } + } id := goid.Get() scopeMu.Lock() @@ -106,6 +126,65 @@ func Setenv(key, value string) error { return nil } +// Unsetenv removes key from the active command scope. Without a scope, it has +// the same behavior as os.Unsetenv. +func Unsetenv(key string) error { + if err := validateEnvKey(key); err != nil { + return err + } + + id := goid.Get() + scopeMu.Lock() + defer scopeMu.Unlock() + + scope, ok := scopes[id] + if !ok { + return os.Unsetenv(key) + } + if scope.Env == nil { + scope.Env = os.Environ() + } + scope.Env = unsetEnv(scope.Env, key) + scopes[id] = scope + return nil +} + +// Clearenv removes all variables from the active command scope. Without a +// scope, it has the same behavior as os.Clearenv. +func Clearenv() { + id := goid.Get() + scopeMu.Lock() + defer scopeMu.Unlock() + + scope, ok := scopes[id] + if !ok { + os.Clearenv() + return + } + scope.Env = []string{} + scopes[id] = scope +} + +// Environ returns a copy of the active command scope environment. Without a +// scope, it has the same behavior as os.Environ. +func Environ() []string { + id := goid.Get() + scopeMu.RLock() + scope, ok := scopes[id] + if ok && scope.Env != nil { + env := clone(scope.Env) + scopeMu.RUnlock() + return env + } + scopeMu.RUnlock() + return os.Environ() +} + +// ExpandEnv expands variables using the active command scope environment. +func ExpandEnv(s string) string { + return os.Expand(s, Getenv) +} + // Println writes to the stdout configured for the active scope. func Println(a ...any) (int, error) { w := io.Writer(os.Stdout) @@ -224,13 +303,18 @@ func clone(in []string) []string { } func envValue(env []string, key string) string { + value, _ := envLookup(env, key) + return value +} + +func envLookup(env []string, key string) (string, bool) { prefix := key + "=" for i := len(env) - 1; i >= 0; i-- { if len(env[i]) >= len(prefix) && env[i][:len(prefix)] == prefix { - return env[i][len(prefix):] + return env[i][len(prefix):], true } } - return "" + return "", false } func setEnv(env []string, key, value string) []string { @@ -244,7 +328,18 @@ func setEnv(env []string, key, value string) []string { return append(env, prefix+value) } -func validateEnv(key, value string) error { +func unsetEnv(env []string, key string) []string { + prefix := key + "=" + out := env[:0] + for _, entry := range env { + if len(entry) < len(prefix) || entry[:len(prefix)] != prefix { + out = append(out, entry) + } + } + return out +} + +func validateEnvKey(key string) error { if key == "" { return fmt.Errorf("invalid environment variable name") } @@ -253,10 +348,5 @@ func validateEnv(key, value string) error { return fmt.Errorf("invalid environment variable name %q", key) } } - for i := 0; i < len(value); i++ { - if value[i] == 0 { - return fmt.Errorf("invalid environment variable value for %q", key) - } - } return nil } diff --git a/internal/execbroker/execbroker_test.go b/internal/execbroker/execbroker_test.go index 95b92f23..ae241827 100644 --- a/internal/execbroker/execbroker_test.go +++ b/internal/execbroker/execbroker_test.go @@ -276,6 +276,61 @@ func TestScopedGetenvAndSetenv(t *testing.T) { } } +func TestScopedEnvironmentAPIs(t *testing.T) { + key := "EXECBROKER_ENV_APIS_TEST" + if err := os.Setenv(key, "process"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Unsetenv(key) }) + + err := Do(Scope{}, func() error { + if got, ok := LookupEnv(key); got != "process" || !ok { + t.Fatalf("LookupEnv before Setenv = %q, %v; want process, true", got, ok) + } + if got := ExpandEnv("$" + key); got != "process" { + t.Fatalf("ExpandEnv before Setenv = %q, want process", got) + } + if err := Setenv(key, "scoped"); err != nil { + return err + } + if got, ok := LookupEnv(key); got != "scoped" || !ok { + t.Fatalf("LookupEnv after Setenv = %q, %v; want scoped, true", got, ok) + } + if got := ExpandEnv("${" + key + "}"); got != "scoped" { + t.Fatalf("ExpandEnv after Setenv = %q, want scoped", got) + } + if got := envValue(Environ(), key); got != "scoped" { + t.Fatalf("Environ value = %q, want scoped", got) + } + if err := Unsetenv(key); err != nil { + return err + } + if _, ok := LookupEnv(key); ok { + t.Fatal("LookupEnv after Unsetenv = present, want absent") + } + if err := Setenv(key, "scoped-again"); err != nil { + return err + } + Clearenv() + if got := len(Environ()); got != 0 { + t.Fatalf("Environ after Clearenv = %d entries, want zero", got) + } + if got := Getenv(key); got != "" { + t.Fatalf("Getenv after Clearenv = %q, want empty", got) + } + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment = %q, want process", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment after scope = %q, want process", got) + } +} + func TestScopedEnvironmentRestoresNestedScope(t *testing.T) { key := "EXECBROKER_NESTED_ENV_TEST" err := Do(Scope{Env: []string{key + "=outer"}}, func() error { diff --git a/internal/formula/formula.go b/internal/formula/formula.go index 5a494394..0b75613b 100644 --- a/internal/formula/formula.go +++ b/internal/formula/formula.go @@ -83,8 +83,6 @@ func loadFS(fs fs.ReadFileFS, path string) (*Formula, error) { // Formula types remain cached after loading, so later interpreters must not // reset the dynamic method slots used by earlier types. ctx := ixgo.NewContext(ixgo.SupportMultipleInterp) - ctx.RegisterExternal("os.Getenv", execbroker.Getenv) - ctx.RegisterExternal("os.Setenv", execbroker.Setenv) ctx.RegisterExternal("os/exec.Command", execbroker.Command) ctx.RegisterExternal("os/exec.CommandContext", execbroker.CommandContext) ctx.RegisterExternal("fmt.Println", execbroker.Println) diff --git a/internal/formula/formula_test.go b/internal/formula/formula_test.go index b3a8337e..d82bc606 100644 --- a/internal/formula/formula_test.go +++ b/internal/formula/formula_test.go @@ -219,15 +219,15 @@ func TestFormulaEnvironmentUsesBrokerScope(t *testing.T) { got = entry[len(prefix):] } } - if got != "formula" { - t.Fatalf("command environment = %q, want formula", got) + if got != "formula-again" { + t.Fatalf("command environment = %q, want formula-again", got) } return nil }) if err != nil { t.Fatal(err) } - if got, want := stdout.String(), "process\nformula\n"; got != want { + if got, want := stdout.String(), "process\nprocess\ntrue\nformula\nformula\nenvironment\n\n\nformula-again\n"; got != want { t.Fatalf("output = %q, want %q", got, want) } if got := os.Getenv(key); got != "process" { diff --git a/internal/formula/testdata/formula/env_llar.gox b/internal/formula/testdata/formula/env_llar.gox index 810a7234..bd9f5787 100644 --- a/internal/formula/testdata/formula/env_llar.gox +++ b/internal/formula/testdata/formula/env_llar.gox @@ -6,6 +6,21 @@ fromVer "v1.0.0" onBuild ctx => { echo os.getenv("FORMULA_ENV_TEST") + value, ok := os.lookupEnv("FORMULA_ENV_TEST") + echo value + echo ok os.setenv("FORMULA_ENV_TEST", "formula")! + echo "${FORMULA_ENV_TEST}" + echo os.expandEnv("$FORMULA_ENV_TEST") + for _, entry := range os.environ() { + if entry == "FORMULA_ENV_TEST=formula" { + echo "environment" + } + } + os.unsetenv("FORMULA_ENV_TEST")! + echo os.getenv("FORMULA_ENV_TEST") + os.clearenv() + echo os.getenv("FORMULA_ENV_TEST") + os.setenv("FORMULA_ENV_TEST", "formula-again")! echo os.getenv("FORMULA_ENV_TEST") } diff --git a/internal/ixgo/exec.go b/internal/ixgo/exec.go index 336c5643..19f10064 100644 --- a/internal/ixgo/exec.go +++ b/internal/ixgo/exec.go @@ -6,7 +6,9 @@ package ixgo import ( "os/exec" + "reflect" + ixgoapi "github.com/goplus/ixgo" "github.com/goplus/llar/internal/execbroker" "github.com/qiniu/x/gsh" ) @@ -19,6 +21,31 @@ func (brokerOS) Run(cmd *exec.Cmd) error { return execbroker.Run(cmd) } +func (brokerOS) Environ() []string { + return execbroker.Environ() +} + +func (brokerOS) ExpandEnv(s string) string { + return execbroker.ExpandEnv(s) +} + +func (brokerOS) Getenv(key string) string { + return execbroker.Getenv(key) +} + func init() { gsh.Sys = brokerOS{OS: gsh.Sys} + ixgoapi.RegisterPackage(&ixgoapi.Package{ + Name: "os", + Path: "os", + Funcs: map[string]reflect.Value{ + "Clearenv": reflect.ValueOf(execbroker.Clearenv), + "Environ": reflect.ValueOf(execbroker.Environ), + "ExpandEnv": reflect.ValueOf(execbroker.ExpandEnv), + "Getenv": reflect.ValueOf(execbroker.Getenv), + "LookupEnv": reflect.ValueOf(execbroker.LookupEnv), + "Setenv": reflect.ValueOf(execbroker.Setenv), + "Unsetenv": reflect.ValueOf(execbroker.Unsetenv), + }, + }) } diff --git a/internal/ixgo/exec_test.go b/internal/ixgo/exec_test.go new file mode 100644 index 00000000..d8395df0 --- /dev/null +++ b/internal/ixgo/exec_test.go @@ -0,0 +1,71 @@ +// Copyright (c) 2026 The XGo Authors (xgo.dev). All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package ixgo + +import ( + "os" + "reflect" + "testing" + + ixgoapi "github.com/goplus/ixgo" + "github.com/goplus/llar/internal/execbroker" + "github.com/qiniu/x/gsh" +) + +func TestBrokerOSPackageMergePreservesExports(t *testing.T) { + pkg, ok := ixgoapi.LookupPackage("os") + if !ok { + t.Fatal("os package is not registered") + } + for _, name := range []string{"ReadFile", "WriteFile", "MkdirAll"} { + if _, ok := pkg.Funcs[name]; !ok { + t.Fatalf("os package lost existing function %q", name) + } + } + fn, ok := pkg.Funcs["Getenv"] + if !ok || fn.Pointer() != reflect.ValueOf(execbroker.Getenv).Pointer() { + t.Fatal("os.Getenv was not replaced by the broker implementation") + } +} + +func TestBrokerOSEnvironmentUsesScope(t *testing.T) { + key := "IXGO_BROKER_ENV_TEST" + if err := os.Setenv(key, "process"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Unsetenv(key) }) + + err := execbroker.Do(execbroker.Scope{}, func() error { + if err := execbroker.Setenv(key, "scoped"); err != nil { + return err + } + if got := gsh.Sys.Getenv(key); got != "scoped" { + t.Fatalf("gsh.Sys.Getenv = %q, want scoped", got) + } + if got := gsh.Sys.ExpandEnv("$" + key); got != "scoped" { + t.Fatalf("gsh.Sys.ExpandEnv = %q, want scoped", got) + } + if got := envValue(gsh.Sys.Environ(), key); got != "scoped" { + t.Fatalf("gsh.Sys.Environ = %q, want scoped", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if got := os.Getenv(key); got != "process" { + t.Fatalf("process environment = %q, want process", got) + } +} + +func envValue(env []string, key string) string { + prefix := key + "=" + for i := len(env) - 1; i >= 0; i-- { + if len(env[i]) >= len(prefix) && env[i][:len(prefix)] == prefix { + return env[i][len(prefix):] + } + } + return "" +} From d4130cfe778f3745d831d3a18f9243a47a17d9d0 Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 21 Aug 2026 15:59:14 +0800 Subject: [PATCH 3/4] feat(execbroker): broker syscall environment APIs --- internal/formula/formula_test.go | 2 +- .../formula/testdata/formula/env_llar.gox | 5 +++++ internal/ixgo/exec.go | 11 ++++++++++ internal/ixgo/exec_test.go | 20 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/internal/formula/formula_test.go b/internal/formula/formula_test.go index d82bc606..f01edc8f 100644 --- a/internal/formula/formula_test.go +++ b/internal/formula/formula_test.go @@ -227,7 +227,7 @@ func TestFormulaEnvironmentUsesBrokerScope(t *testing.T) { if err != nil { t.Fatal(err) } - if got, want := stdout.String(), "process\nprocess\ntrue\nformula\nformula\nenvironment\n\n\nformula-again\n"; got != want { + if got, want := stdout.String(), "process\nprocess\ntrue\nformula\nformula\nenvironment\nsyscall\ntrue\n\n\nformula-again\n"; got != want { t.Fatalf("output = %q, want %q", got, want) } if got := os.Getenv(key); got != "process" { diff --git a/internal/formula/testdata/formula/env_llar.gox b/internal/formula/testdata/formula/env_llar.gox index bd9f5787..c9cdc5fa 100644 --- a/internal/formula/testdata/formula/env_llar.gox +++ b/internal/formula/testdata/formula/env_llar.gox @@ -1,4 +1,5 @@ import "os" +import "syscall" id "test/env" @@ -17,6 +18,10 @@ onBuild ctx => { echo "environment" } } + syscall.setenv("FORMULA_ENV_TEST", "syscall")! + value, ok = syscall.getenv("FORMULA_ENV_TEST") + echo value + echo ok os.unsetenv("FORMULA_ENV_TEST")! echo os.getenv("FORMULA_ENV_TEST") os.clearenv() diff --git a/internal/ixgo/exec.go b/internal/ixgo/exec.go index 19f10064..895b976d 100644 --- a/internal/ixgo/exec.go +++ b/internal/ixgo/exec.go @@ -48,4 +48,15 @@ func init() { "Unsetenv": reflect.ValueOf(execbroker.Unsetenv), }, }) + ixgoapi.RegisterPackage(&ixgoapi.Package{ + Name: "syscall", + Path: "syscall", + Funcs: map[string]reflect.Value{ + "Clearenv": reflect.ValueOf(execbroker.Clearenv), + "Environ": reflect.ValueOf(execbroker.Environ), + "Getenv": reflect.ValueOf(execbroker.LookupEnv), + "Setenv": reflect.ValueOf(execbroker.Setenv), + "Unsetenv": reflect.ValueOf(execbroker.Unsetenv), + }, + }) } diff --git a/internal/ixgo/exec_test.go b/internal/ixgo/exec_test.go index d8395df0..3528c58c 100644 --- a/internal/ixgo/exec_test.go +++ b/internal/ixgo/exec_test.go @@ -30,6 +30,26 @@ func TestBrokerOSPackageMergePreservesExports(t *testing.T) { } } +func TestBrokerSyscallPackageMergePreservesExports(t *testing.T) { + pkg, ok := ixgoapi.LookupPackage("syscall") + if !ok { + t.Fatal("syscall package is not registered") + } + if _, ok := pkg.Funcs["Read"]; !ok { + t.Fatal("syscall package lost existing function Read") + } + for name, want := range map[string]uintptr{ + "Getenv": reflect.ValueOf(execbroker.LookupEnv).Pointer(), + "Setenv": reflect.ValueOf(execbroker.Setenv).Pointer(), + "Unsetenv": reflect.ValueOf(execbroker.Unsetenv).Pointer(), + } { + fn, ok := pkg.Funcs[name] + if !ok || fn.Pointer() != want { + t.Fatalf("syscall.%s was not replaced by the broker implementation", name) + } + } +} + func TestBrokerOSEnvironmentUsesScope(t *testing.T) { key := "IXGO_BROKER_ENV_TEST" if err := os.Setenv(key, "process"); err != nil { From 15c8ce6bf4f02d5464e15fa4f819dcb38ad4c81e Mon Sep 17 00:00:00 2001 From: Rick Guo Date: Fri, 21 Aug 2026 16:04:08 +0800 Subject: [PATCH 4/4] chore: route x environment access through execbroker --- AGENTS.md | 7 +++++ x/autotools/autotools_test.go | 57 +++++++++++++++++++++++------------ x/cmake/cmake_test.go | 43 ++++++++++++++++++-------- x/pkgconfig/pkgconfig_test.go | 33 ++++++++++++++------ 4 files changed, 98 insertions(+), 42 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2d7b70fb..a2838269 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,6 +8,13 @@ ## Project Conventions +### Environment Access in `x` + +- All environment reads and writes under `x/`, including tests and test + helpers, must use `internal/execbroker` (`Getenv`, `LookupEnv`, `Setenv`, + `Unsetenv`, `Clearenv`, `Environ`, or `ExpandEnv`). Do not call the `os` or + `syscall` environment APIs directly from `x/`. + ### Command and Flag Changes - Follow the existing Cobra command style. diff --git a/x/autotools/autotools_test.go b/x/autotools/autotools_test.go index 2f24a476..b5ae4f1d 100644 --- a/x/autotools/autotools_test.go +++ b/x/autotools/autotools_test.go @@ -7,8 +7,25 @@ import ( "runtime" "strings" "testing" + + "github.com/goplus/llar/internal/execbroker" ) +func setenv(t *testing.T, key, value string) { + t.Helper() + old, existed := execbroker.LookupEnv(key) + if err := execbroker.Setenv(key, value); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if existed { + _ = execbroker.Setenv(key, old) + } else { + _ = execbroker.Unsetenv(key) + } + }) +} + func TestUseSetsEnv(t *testing.T) { root := t.TempDir() includeDir := filepath.Join(root, "include") @@ -24,7 +41,7 @@ func TestUseSetsEnv(t *testing.T) { "PKG_CONFIG_PATH", "CMAKE_PREFIX_PATH", "CMAKE_INCLUDE_PATH", "CMAKE_LIBRARY_PATH", "INCLUDE", "LIB", "CPPFLAGS", "LDFLAGS", } { - t.Setenv(key, "") + setenv(t, key, "") } a := New("", "", "") @@ -36,23 +53,23 @@ func TestUseSetsEnv(t *testing.T) { "CMAKE_INCLUDE_PATH": includeDir, "CMAKE_LIBRARY_PATH": libDir, } { - if got := os.Getenv(key); got != want { + if got := execbroker.Getenv(key); got != want { t.Errorf("%s = %q, want %q", key, got, want) } } if runtime.GOOS == "windows" { - if got := os.Getenv("INCLUDE"); got != includeDir { + if got := execbroker.Getenv("INCLUDE"); got != includeDir { t.Errorf("INCLUDE = %q, want %q", got, includeDir) } - if got := os.Getenv("LIB"); got != libDir { + if got := execbroker.Getenv("LIB"); got != libDir { t.Errorf("LIB = %q, want %q", got, libDir) } } else { - if got := os.Getenv("CPPFLAGS"); strings.TrimSpace(got) != "-I"+includeDir { + if got := execbroker.Getenv("CPPFLAGS"); strings.TrimSpace(got) != "-I"+includeDir { t.Errorf("CPPFLAGS = %q, want %q", got, "-I"+includeDir) } - if got := os.Getenv("LDFLAGS"); strings.TrimSpace(got) != "-L"+libDir { + if got := execbroker.Getenv("LDFLAGS"); strings.TrimSpace(got) != "-L"+libDir { t.Errorf("LDFLAGS = %q, want %q", got, "-L"+libDir) } } @@ -66,18 +83,18 @@ func TestUseMultipleDeps(t *testing.T) { os.MkdirAll(filepath.Join(r, "lib"), 0o755) } - t.Setenv("CMAKE_INCLUDE_PATH", "") - t.Setenv("CMAKE_LIBRARY_PATH", "") - t.Setenv("CMAKE_PREFIX_PATH", "") - t.Setenv("CPPFLAGS", "") - t.Setenv("LDFLAGS", "") + setenv(t, "CMAKE_INCLUDE_PATH", "") + setenv(t, "CMAKE_LIBRARY_PATH", "") + setenv(t, "CMAKE_PREFIX_PATH", "") + setenv(t, "CPPFLAGS", "") + setenv(t, "LDFLAGS", "") a := New("", "", "") a.Use(root1) a.Use(root2) // prependPath: root2 should be prepended before root1 - got := os.Getenv("CMAKE_PREFIX_PATH") + got := execbroker.Getenv("CMAKE_PREFIX_PATH") if !strings.HasPrefix(got, root2) { t.Errorf("CMAKE_PREFIX_PATH = %q, expected %q to be first", got, root2) } @@ -86,7 +103,7 @@ func TestUseMultipleDeps(t *testing.T) { } // appendFlag: root1 flag should come before root2 flag - cppflags := os.Getenv("CPPFLAGS") + cppflags := execbroker.Getenv("CPPFLAGS") i1 := strings.Index(cppflags, filepath.Join(root1, "include")) i2 := strings.Index(cppflags, filepath.Join(root2, "include")) if i1 < 0 || i2 < 0 || i1 >= i2 { @@ -101,16 +118,16 @@ func TestUsePartialDirs(t *testing.T) { for _, key := range []string{ "PKG_CONFIG_PATH", "CMAKE_LIBRARY_PATH", "CPPFLAGS", "LDFLAGS", } { - t.Setenv(key, "") + setenv(t, key, "") } a := New("", "", "") a.Use(root) - if got := os.Getenv("PKG_CONFIG_PATH"); got != "" { + if got := execbroker.Getenv("PKG_CONFIG_PATH"); got != "" { t.Errorf("PKG_CONFIG_PATH = %q, want empty", got) } - if got := os.Getenv("CMAKE_LIBRARY_PATH"); got != "" { + if got := execbroker.Getenv("CMAKE_LIBRARY_PATH"); got != "" { t.Errorf("CMAKE_LIBRARY_PATH = %q, want empty", got) } } @@ -142,7 +159,7 @@ func TestWorkDir(t *testing.T) { } func TestPrependPath(t *testing.T) { - t.Setenv("TEST_PREPEND", "/existing") + setenv(t, "TEST_PREPEND", "/existing") prependPath("TEST_PREPEND", "/new") sep := ":" @@ -150,17 +167,17 @@ func TestPrependPath(t *testing.T) { sep = ";" } want := "/new" + sep + "/existing" - if got := os.Getenv("TEST_PREPEND"); got != want { + if got := execbroker.Getenv("TEST_PREPEND"); got != want { t.Errorf("TEST_PREPEND = %q, want %q", got, want) } } func TestAppendFlag(t *testing.T) { - t.Setenv("TEST_FLAGS", "-Ifoo") + setenv(t, "TEST_FLAGS", "-Ifoo") appendFlag("TEST_FLAGS", "-Ibar") want := "-Ifoo -Ibar" - if got := os.Getenv("TEST_FLAGS"); got != want { + if got := execbroker.Getenv("TEST_FLAGS"); got != want { t.Errorf("TEST_FLAGS = %q, want %q", got, want) } } diff --git a/x/cmake/cmake_test.go b/x/cmake/cmake_test.go index 1b87ece0..0c844370 100644 --- a/x/cmake/cmake_test.go +++ b/x/cmake/cmake_test.go @@ -7,8 +7,25 @@ import ( "runtime" "strings" "testing" + + "github.com/goplus/llar/internal/execbroker" ) +func setenv(t *testing.T, key, value string) { + t.Helper() + old, existed := execbroker.LookupEnv(key) + if err := execbroker.Setenv(key, value); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if existed { + _ = execbroker.Setenv(key, old) + } else { + _ = execbroker.Unsetenv(key) + } + }) +} + func TestUseSetsEnv(t *testing.T) { root := t.TempDir() includeDir := filepath.Join(root, "include") @@ -24,7 +41,7 @@ func TestUseSetsEnv(t *testing.T) { "PKG_CONFIG_PATH", "CMAKE_PREFIX_PATH", "CMAKE_INCLUDE_PATH", "CMAKE_LIBRARY_PATH", "INCLUDE", "LIB", "CPPFLAGS", "LDFLAGS", } { - t.Setenv(key, "") + setenv(t, key, "") } c := New("", "", "") @@ -36,23 +53,23 @@ func TestUseSetsEnv(t *testing.T) { "CMAKE_INCLUDE_PATH": includeDir, "CMAKE_LIBRARY_PATH": libDir, } { - if got := os.Getenv(key); got != want { + if got := execbroker.Getenv(key); got != want { t.Errorf("%s = %q, want %q", key, got, want) } } if runtime.GOOS == "windows" { - if got := os.Getenv("INCLUDE"); got != includeDir { + if got := execbroker.Getenv("INCLUDE"); got != includeDir { t.Errorf("INCLUDE = %q, want %q", got, includeDir) } - if got := os.Getenv("LIB"); got != libDir { + if got := execbroker.Getenv("LIB"); got != libDir { t.Errorf("LIB = %q, want %q", got, libDir) } } else { - if got := os.Getenv("CPPFLAGS"); strings.TrimSpace(got) != "-I"+includeDir { + if got := execbroker.Getenv("CPPFLAGS"); strings.TrimSpace(got) != "-I"+includeDir { t.Errorf("CPPFLAGS = %q, want %q", got, "-I"+includeDir) } - if got := os.Getenv("LDFLAGS"); strings.TrimSpace(got) != "-L"+libDir { + if got := execbroker.Getenv("LDFLAGS"); strings.TrimSpace(got) != "-L"+libDir { t.Errorf("LDFLAGS = %q, want %q", got, "-L"+libDir) } } @@ -65,16 +82,16 @@ func TestUsePartialDirs(t *testing.T) { for _, key := range []string{ "PKG_CONFIG_PATH", "CMAKE_LIBRARY_PATH", } { - t.Setenv(key, "") + setenv(t, key, "") } c := New("", "", "") c.Use(root) - if got := os.Getenv("PKG_CONFIG_PATH"); got != "" { + if got := execbroker.Getenv("PKG_CONFIG_PATH"); got != "" { t.Errorf("PKG_CONFIG_PATH = %q, want empty", got) } - if got := os.Getenv("CMAKE_LIBRARY_PATH"); got != "" { + if got := execbroker.Getenv("CMAKE_LIBRARY_PATH"); got != "" { t.Errorf("CMAKE_LIBRARY_PATH = %q, want empty", got) } } @@ -129,7 +146,7 @@ func TestSource(t *testing.T) { } func TestPrependPath(t *testing.T) { - t.Setenv("TEST_PREPEND", "/existing") + setenv(t, "TEST_PREPEND", "/existing") prependPath("TEST_PREPEND", "/new") sep := ":" @@ -137,16 +154,16 @@ func TestPrependPath(t *testing.T) { sep = ";" } want := "/new" + sep + "/existing" - if got := os.Getenv("TEST_PREPEND"); got != want { + if got := execbroker.Getenv("TEST_PREPEND"); got != want { t.Errorf("TEST_PREPEND = %q, want %q", got, want) } } func TestAppendFlag(t *testing.T) { - t.Setenv("TEST_FLAGS", "-Ifoo") + setenv(t, "TEST_FLAGS", "-Ifoo") appendFlag("TEST_FLAGS", "-Ibar") - if got := os.Getenv("TEST_FLAGS"); got != "-Ifoo -Ibar" { + if got := execbroker.Getenv("TEST_FLAGS"); got != "-Ifoo -Ibar" { t.Errorf("TEST_FLAGS = %q, want %q", got, "-Ifoo -Ibar") } } diff --git a/x/pkgconfig/pkgconfig_test.go b/x/pkgconfig/pkgconfig_test.go index 63e55a63..8ffde461 100644 --- a/x/pkgconfig/pkgconfig_test.go +++ b/x/pkgconfig/pkgconfig_test.go @@ -11,27 +11,42 @@ import ( "github.com/goplus/llar/internal/execbroker" ) +func setenv(t *testing.T, key, value string) { + t.Helper() + old, existed := execbroker.LookupEnv(key) + if err := execbroker.Setenv(key, value); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if existed { + _ = execbroker.Setenv(key, old) + } else { + _ = execbroker.Unsetenv(key) + } + }) +} + func TestUse(t *testing.T) { root := t.TempDir() dir := filepath.Join(root, "lib", "pkgconfig") if err := os.MkdirAll(dir, 0o755); err != nil { t.Fatal(err) } - t.Setenv("PKG_CONFIG_PATH", "/existing") + setenv(t, "PKG_CONFIG_PATH", "/existing") Use(root) - if got, want := os.Getenv("PKG_CONFIG_PATH"), dir+string(os.PathListSeparator)+"/existing"; got != want { + if got, want := execbroker.Getenv("PKG_CONFIG_PATH"), dir+string(os.PathListSeparator)+"/existing"; got != want { t.Fatalf("PKG_CONFIG_PATH = %q, want %q", got, want) } } func TestUseIgnoresMissingDirectory(t *testing.T) { - t.Setenv("PKG_CONFIG_PATH", "/existing") + setenv(t, "PKG_CONFIG_PATH", "/existing") Use(t.TempDir()) - if got := os.Getenv("PKG_CONFIG_PATH"); got != "/existing" { + if got := execbroker.Getenv("PKG_CONFIG_PATH"); got != "/existing" { t.Fatalf("PKG_CONFIG_PATH = %q, want unchanged", got) } } @@ -56,7 +71,7 @@ func TestQueries(t *testing.T) { request = req req.Name = os.Args[0] req.Args = []string{"-test.run=TestLookupHelperProcess"} - req.Env = append(os.Environ(), "GO_WANT_PKGCONFIG_HELPER=1") + req.Env = append(execbroker.Environ(), "GO_WANT_PKGCONFIG_HELPER=1") return req, nil }, }, func() error { @@ -95,7 +110,7 @@ func TestQueryErrors(t *testing.T) { Middleware: func(req execbroker.Request) (execbroker.Request, error) { req.Name = os.Args[0] req.Args = []string{"-test.run=TestLookupHelperProcess"} - req.Env = append(os.Environ(), + req.Env = append(execbroker.Environ(), "GO_WANT_PKGCONFIG_HELPER=1", "GO_PKGCONFIG_HELPER_FAIL=1", "GO_PKGCONFIG_HELPER_STDERR="+tt.detail, @@ -114,11 +129,11 @@ func TestQueryErrors(t *testing.T) { } func TestLookupHelperProcess(t *testing.T) { - if os.Getenv("GO_WANT_PKGCONFIG_HELPER") != "1" { + if execbroker.Getenv("GO_WANT_PKGCONFIG_HELPER") != "1" { return } - if os.Getenv("GO_PKGCONFIG_HELPER_FAIL") == "1" { - if detail := os.Getenv("GO_PKGCONFIG_HELPER_STDERR"); detail != "" { + if execbroker.Getenv("GO_PKGCONFIG_HELPER_FAIL") == "1" { + if detail := execbroker.Getenv("GO_PKGCONFIG_HELPER_STDERR"); detail != "" { fmt.Fprintln(os.Stderr, detail) } os.Exit(1)