Skip to content

Commit 80265a8

Browse files
andrecloutierandrecloutier
authored
fix: always propagate env variables when executing commands (#1982)
Fixes #1896
1 parent 4b9297b commit 80265a8

File tree

7 files changed

+167
-121
lines changed

7 files changed

+167
-121
lines changed

‎internal/cli/cli.go

+7-9
Original file line numberDiff line numberDiff line change
@@ -521,8 +521,6 @@ func envCommand(logger *log.Logger, shimmedCommand string, args []string) error
521521
"PATH": setPath(execPaths),
522522
}
523523

524-
env = execenv.MergeEnv(execenv.SliceToMap(os.Environ()), env)
525-
526524
if parsedVersion.Type != "system" {
527525
env, err = execenv.Generate(plugin, env)
528526
if _, ok := err.(plugins.NoCallbackError); !ok && err != nil {
@@ -535,7 +533,8 @@ func envCommand(logger *log.Logger, shimmedCommand string, args []string) error
535533
return err
536534
}
537535

538-
err = exec.Exec(fname, realArgs, execute.MapToSlice(env))
536+
finalEnv := execute.MergeWithCurrentEnv(env)
537+
err = exec.Exec(fname, realArgs, finalEnv)
539538
if err != nil {
540539
fmt.Printf("err %#+v\n", err.Error())
541540
}
@@ -581,8 +580,6 @@ func execCommand(logger *log.Logger, command string, args []string) error {
581580
"PATH": setPath(execPaths),
582581
}
583582

584-
env = execenv.MergeEnv(execenv.SliceToMap(os.Environ()), env)
585-
586583
if parsedVersion.Type != "system" {
587584
env, err = execenv.Generate(plugin, env)
588585
if _, ok := err.(plugins.NoCallbackError); !ok && err != nil {
@@ -596,7 +593,8 @@ func execCommand(logger *log.Logger, command string, args []string) error {
596593
return err
597594
}
598595

599-
return exec.Exec(executable, args, execute.MapToSlice(env))
596+
finalEnv := execute.MergeWithCurrentEnv(env)
597+
return exec.Exec(executable, args, finalEnv)
600598
}
601599

602600
func extensionCommand(logger *log.Logger, args []string) error {
@@ -615,12 +613,12 @@ func extensionCommand(logger *log.Logger, args []string) error {
615613
pluginName := args[0]
616614
plugin := plugins.New(conf, pluginName)
617615

618-
err = runExtensionCommand(plugin, args[1:], execenv.SliceToMap(os.Environ()))
616+
err = runExtensionCommand(plugin, args[1:])
619617
logger.Printf("error running extension command: %s", err.Error())
620618
return err
621619
}
622620

623-
func runExtensionCommand(plugin plugins.Plugin, args []string, environment map[string]string) (err error) {
621+
func runExtensionCommand(plugin plugins.Plugin, args []string) (err error) {
624622
path := ""
625623
if len(args) > 0 {
626624
path, err = plugin.ExtensionCommandPath(args[0])
@@ -640,7 +638,7 @@ func runExtensionCommand(plugin plugins.Plugin, args []string, environment map[s
640638
}
641639
}
642640

643-
return exec.Exec(path, args, execute.MapToSlice(environment))
641+
return exec.Exec(path, args, os.Environ())
644642
}
645643

646644
func getExecutable(logger *log.Logger, conf config.Config, command string) (executable string, plugin plugins.Plugin, version string, err error) {

‎internal/execenv/execenv.go

+1-38
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package execenv
44

55
import (
66
"fmt"
7-
"os"
87
"strings"
98

109
"github.com/asdf-vm/asdf/internal/execute"
@@ -13,20 +12,6 @@ import (
1312

1413
const execEnvCallbackName = "exec-env"
1514

16-
// CurrentEnv returns the current environment as a map
17-
func CurrentEnv() map[string]string {
18-
return SliceToMap(os.Environ())
19-
}
20-
21-
// MergeEnv takes two maps with string keys and values and merges them.
22-
func MergeEnv(map1, map2 map[string]string) map[string]string {
23-
for key, value := range map2 {
24-
map1[key] = value
25-
}
26-
27-
return map1
28-
}
29-
3015
// Generate runs exec-env callback if available and captures the environment
3116
// variables it sets. It then parses them and returns them as a map.
3217
func Generate(plugin plugins.Plugin, callbackEnv map[string]string) (env map[string]string, err error) {
@@ -48,27 +33,5 @@ func Generate(plugin plugins.Plugin, callbackEnv map[string]string) (env map[str
4833
err = expression.Run()
4934

5035
str := stdout.String()
51-
return SliceToMap(strings.Split(str, "\n")), err
52-
}
53-
54-
// SliceToMap converts an env map to env slice suitable for syscall.Exec
55-
func SliceToMap(env []string) map[string]string {
56-
envMap := map[string]string{}
57-
58-
var previousKey string
59-
60-
for _, envVar := range env {
61-
varValue := strings.SplitN(envVar, "=", 2)
62-
if len(varValue) == 2 {
63-
// new var=value line
64-
previousKey = varValue[0]
65-
envMap[varValue[0]] = varValue[1]
66-
} else {
67-
// value from variable defined on a previous line, append
68-
val := envMap[previousKey]
69-
envMap[previousKey] = val + "\n" + varValue[0]
70-
}
71-
}
72-
73-
return envMap
36+
return execute.SliceToMap(strings.Split(str, "\n")), err
7437
}

‎internal/execenv/execenv_test.go

-69
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package execenv
22

33
import (
4-
"fmt"
54
"testing"
65

76
"github.com/asdf-vm/asdf/internal/config"
@@ -15,41 +14,7 @@ const (
1514
testPluginName2 = "ruby"
1615
)
1716

18-
func TestCurrentEnv(t *testing.T) {
19-
t.Run("returns map of current environment", func(t *testing.T) {
20-
envMap := CurrentEnv()
21-
path, found := envMap["PATH"]
22-
assert.True(t, found)
23-
assert.NotEmpty(t, path)
24-
})
25-
}
26-
27-
func TestMergeEnv(t *testing.T) {
28-
t.Run("merges two maps", func(t *testing.T) {
29-
map1 := map[string]string{"Key": "value"}
30-
map2 := map[string]string{"Key2": "value2"}
31-
map3 := MergeEnv(map1, map2)
32-
assert.Equal(t, map3["Key"], "value")
33-
assert.Equal(t, map3["Key2"], "value2")
34-
})
35-
36-
t.Run("doesn't change original map", func(t *testing.T) {
37-
map1 := map[string]string{"Key": "value"}
38-
map2 := map[string]string{"Key2": "value2"}
39-
_ = MergeEnv(map1, map2)
40-
assert.Equal(t, map1["Key2"], "value2")
41-
})
42-
43-
t.Run("second map overwrites values in first", func(t *testing.T) {
44-
map1 := map[string]string{"Key": "value"}
45-
map2 := map[string]string{"Key": "value2"}
46-
map3 := MergeEnv(map1, map2)
47-
assert.Equal(t, map3["Key"], "value2")
48-
})
49-
}
50-
5117
func TestGenerate(t *testing.T) {
52-
5318
t.Run("returns map of environment variables", func(t *testing.T) {
5419
testDataDir := t.TempDir()
5520
conf := config.Config{DataDir: testDataDir}
@@ -101,37 +66,3 @@ func TestGenerate(t *testing.T) {
10166
assert.Equal(t, "abc\n123", env["EQUALSTEST"])
10267
})
10368
}
104-
105-
func TestSliceToMap(t *testing.T) {
106-
tests := []struct {
107-
input []string
108-
output map[string]string
109-
}{
110-
{
111-
input: []string{"VAR=value"},
112-
output: map[string]string{"VAR": "value"},
113-
},
114-
{
115-
input: []string{"BASH_FUNC_bats_readlinkf%%=() { readlink -f \"$1\"\n}"},
116-
output: map[string]string{"BASH_FUNC_bats_readlinkf%%": "() { readlink -f \"$1\"\n}"},
117-
},
118-
{
119-
input: []string{"MYVAR=some things = with = in it"},
120-
output: map[string]string{"MYVAR": "some things = with = in it"},
121-
},
122-
{
123-
input: []string{"MYVAR=value\nwith\nnewlines"},
124-
output: map[string]string{"MYVAR": "value\nwith\nnewlines"},
125-
},
126-
{
127-
input: []string{"MYVAR=value", "with", "newlines"},
128-
output: map[string]string{"MYVAR": "value\nwith\nnewlines"},
129-
},
130-
}
131-
132-
for _, tt := range tests {
133-
t.Run(fmt.Sprintf("input: %s, output: %s", tt.input, tt.output), func(t *testing.T) {
134-
assert.Equal(t, tt.output, SliceToMap(tt.input))
135-
})
136-
}
137-
}

‎internal/execute/execute.go

+49-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package execute
66
import (
77
"fmt"
88
"io"
9+
"os"
910
"os/exec"
1011
"strings"
1112
)
@@ -47,7 +48,12 @@ func (c Command) Run() error {
4748

4849
cmd := exec.Command("bash", "-c", command)
4950

50-
cmd.Env = MapToSlice(c.Env)
51+
if len(c.Env) > 0 {
52+
cmd.Env = MergeWithCurrentEnv(c.Env)
53+
} else {
54+
cmd.Env = os.Environ()
55+
}
56+
5157
cmd.Stdin = c.Stdin
5258

5359
// Capture stdout and stderr
@@ -57,6 +63,25 @@ func (c Command) Run() error {
5763
return cmd.Run()
5864
}
5965

66+
// MergeWithCurrentEnv merges the provided map into the current environment variables
67+
func MergeWithCurrentEnv(env map[string]string) (slice []string) {
68+
return MapToSlice(MergeEnv(CurrentEnv(), env))
69+
}
70+
71+
// CurrentEnv returns the current environment as a map
72+
func CurrentEnv() map[string]string {
73+
return SliceToMap(os.Environ())
74+
}
75+
76+
// MergeEnv takes two maps with string keys and values and merges them.
77+
func MergeEnv(map1, map2 map[string]string) map[string]string {
78+
for key, value := range map2 {
79+
map1[key] = value
80+
}
81+
82+
return map1
83+
}
84+
6085
// MapToSlice converts an env map to env slice suitable for syscall.Exec
6186
func MapToSlice(env map[string]string) (slice []string) {
6287
for key, value := range env {
@@ -66,6 +91,29 @@ func MapToSlice(env map[string]string) (slice []string) {
6691
return slice
6792
}
6893

94+
// SliceToMap converts an env map to env slice suitable for syscall.Exec
95+
func SliceToMap(env []string) map[string]string {
96+
envMap := map[string]string{}
97+
98+
var previousKey string
99+
100+
for _, envVar := range env {
101+
varValue := strings.SplitN(envVar, "=", 2)
102+
103+
if len(varValue) == 2 {
104+
// new var=value line
105+
previousKey = varValue[0]
106+
envMap[varValue[0]] = varValue[1]
107+
} else {
108+
// value from variable defined on a previous line, append
109+
val := envMap[previousKey]
110+
envMap[previousKey] = val + "\n" + varValue[0]
111+
}
112+
}
113+
114+
return envMap
115+
}
116+
69117
func formatArgString(args []string) string {
70118
var newArgs []string
71119
for _, str := range args {

0 commit comments

Comments
 (0)