diff --git a/AGENTS.md b/AGENTS.md index 2d7b70f..a283826 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/internal/execbroker/execbroker.go b/internal/execbroker/execbroker.go index 28e014f..7e7ea5f 100644 --- a/internal/execbroker/execbroker.go +++ b/internal/execbroker/execbroker.go @@ -66,6 +66,125 @@ 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) +} + +// 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 := 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() + 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 +} + +// 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) @@ -182,3 +301,52 @@ func clone(in []string) []string { } return append([]string(nil), in...) } + +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):], true + } + } + return "", false +} + +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 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") + } + for i := 0; i < len(key); i++ { + if key[i] == '=' || key[i] == 0 { + return fmt.Errorf("invalid environment variable name %q", key) + } + } + return nil +} diff --git a/internal/execbroker/execbroker_test.go b/internal/execbroker/execbroker_test.go index bf5f7dd..ae24182 100644 --- a/internal/execbroker/execbroker_test.go +++ b/internal/execbroker/execbroker_test.go @@ -236,6 +236,175 @@ 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 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 { + 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_test.go b/internal/formula/formula_test.go index 5b356c7..f01edc8 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-again" { + t.Fatalf("command environment = %q, want formula-again", got) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + 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" { + 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 0000000..c9cdc5f --- /dev/null +++ b/internal/formula/testdata/formula/env_llar.gox @@ -0,0 +1,31 @@ +import "os" +import "syscall" + +id "test/env" + +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" + } + } + 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() + 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 336c564..895b976 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,42 @@ 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), + }, + }) + 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 new file mode 100644 index 0000000..3528c58 --- /dev/null +++ b/internal/ixgo/exec_test.go @@ -0,0 +1,91 @@ +// 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 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 { + 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 "" +} diff --git a/x/autotools/autotools.go b/x/autotools/autotools.go index bfa59b4..2cc0bae 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/autotools/autotools_test.go b/x/autotools/autotools_test.go index 2f24a47..b5ae4f1 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.go b/x/cmake/cmake.go index 1c1625e..d776709 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/cmake/cmake_test.go b/x/cmake/cmake_test.go index 1b87ece..0c84437 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.go b/x/pkgconfig/pkgconfig.go index 0e9b06c..0c5f08d 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. diff --git a/x/pkgconfig/pkgconfig_test.go b/x/pkgconfig/pkgconfig_test.go index 63e55a6..8ffde46 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)