Skip to content

Commit 69eb591

Browse files
thockinthockin
committed
Env var handling should prefer old names
For back-compat, if we find both an old and new env var for the same flag, prefer the old. This matters because the docker image sets GITSYNC_ROOT but some users still set GIT_SYNC_ROOT.
1 parent cffb28d commit 69eb591

File tree

4 files changed

+290
-115
lines changed

4 files changed

+290
-115
lines changed

Diff for: ‎README.md

-1
Original file line numberDiff line numberDiff line change
@@ -587,4 +587,3 @@ HOOKS
587587
if a hook fails and a new hash is synced during the backoff period, the
588588
retried hook will fire for the newest hash.
589589
```
590-

Diff for: ‎env.go

+132-36
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,42 @@ import (
2929
"github.com/spf13/pflag"
3030
)
3131

32+
// Tests can set this or set it to nil.
33+
var envWarnfOverride func(format string, args ...any)
34+
35+
func envWarnf(format string, args ...any) {
36+
if envWarnfOverride != nil {
37+
envWarnfOverride(format, args...)
38+
} else {
39+
fmt.Fprintf(os.Stderr, format, args...)
40+
}
41+
}
42+
3243
func envString(def string, key string, alts ...string) string {
33-
if val := os.Getenv(key); val != "" {
34-
return val
44+
found := 0
45+
result := ""
46+
resultKey := ""
47+
48+
if val, ok := os.LookupEnv(key); ok {
49+
found++
50+
result = val
51+
resultKey = key
3552
}
3653
for _, alt := range alts {
37-
if val := os.Getenv(alt); val != "" {
38-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
39-
return val
54+
if val, ok := os.LookupEnv(alt); ok {
55+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
56+
found++
57+
result = val
58+
resultKey = alt
4059
}
4160
}
42-
return def
61+
if found == 0 {
62+
return def
63+
}
64+
if found > 1 {
65+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
66+
}
67+
return result
4368
}
4469
func envFlagString(key string, def string, usage string, alts ...string) *string {
4570
registerEnvFlag(key, "string", usage)
@@ -60,16 +85,31 @@ func envStringArray(def string, key string, alts ...string) []string {
6085
return strings.Split(s, ":")
6186
}
6287

63-
if val := os.Getenv(key); val != "" {
64-
return parse(val)
88+
found := 0
89+
result := ""
90+
resultKey := ""
91+
92+
if val, ok := os.LookupEnv(key); ok {
93+
found++
94+
result = val
95+
resultKey = key
6596
}
6697
for _, alt := range alts {
67-
if val := os.Getenv(alt); val != "" {
68-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
69-
return parse(val)
98+
if val, ok := os.LookupEnv(alt); ok {
99+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
100+
found++
101+
result = val
102+
resultKey = key
70103
}
71104
}
72-
return parse(def)
105+
if found == 0 {
106+
return parse(def)
107+
}
108+
if found > 1 {
109+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
110+
}
111+
112+
return parse(result)
73113
}
74114

75115
func envBoolOrError(def bool, key string, alts ...string) (bool, error) {
@@ -81,16 +121,30 @@ func envBoolOrError(def bool, key string, alts ...string) (bool, error) {
81121
return false, fmt.Errorf("invalid bool env %s=%q: %w", key, val, err)
82122
}
83123

84-
if val := os.Getenv(key); val != "" {
85-
return parse(key, val)
124+
found := 0
125+
result := ""
126+
resultKey := ""
127+
128+
if val, ok := os.LookupEnv(key); ok {
129+
found++
130+
result = val
131+
resultKey = key
86132
}
87133
for _, alt := range alts {
88-
if val := os.Getenv(alt); val != "" {
89-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
90-
return parse(alt, val)
134+
if val, ok := os.LookupEnv(alt); ok {
135+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
136+
found++
137+
result = val
138+
resultKey = key
91139
}
92140
}
93-
return def, nil
141+
if found == 0 {
142+
return def, nil
143+
}
144+
if found > 1 {
145+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
146+
}
147+
return parse(resultKey, result)
94148
}
95149
func envBool(def bool, key string, alts ...string) bool {
96150
val, err := envBoolOrError(def, key, alts...)
@@ -111,16 +165,30 @@ func envIntOrError(def int, key string, alts ...string) (int, error) {
111165
return 0, fmt.Errorf("invalid int env %s=%q: %w", key, val, err)
112166
}
113167

114-
if val := os.Getenv(key); val != "" {
115-
return parse(key, val)
168+
found := 0
169+
result := ""
170+
resultKey := ""
171+
172+
if val, ok := os.LookupEnv(key); ok {
173+
found++
174+
result = val
175+
resultKey = key
116176
}
117177
for _, alt := range alts {
118-
if val := os.Getenv(alt); val != "" {
119-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
120-
return parse(alt, val)
178+
if val, ok := os.LookupEnv(alt); ok {
179+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
180+
found++
181+
result = val
182+
resultKey = key
121183
}
122184
}
123-
return def, nil
185+
if found == 0 {
186+
return def, nil
187+
}
188+
if found > 1 {
189+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
190+
}
191+
return parse(resultKey, result)
124192
}
125193
func envInt(def int, key string, alts ...string) int {
126194
val, err := envIntOrError(def, key, alts...)
@@ -141,16 +209,30 @@ func envFloatOrError(def float64, key string, alts ...string) (float64, error) {
141209
return 0, fmt.Errorf("invalid float env %s=%q: %w", key, val, err)
142210
}
143211

144-
if val := os.Getenv(key); val != "" {
145-
return parse(key, val)
212+
found := 0
213+
result := ""
214+
resultKey := ""
215+
216+
if val, ok := os.LookupEnv(key); ok {
217+
found++
218+
result = val
219+
resultKey = key
146220
}
147221
for _, alt := range alts {
148-
if val := os.Getenv(alt); val != "" {
149-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
150-
return parse(alt, val)
222+
if val, ok := os.LookupEnv(alt); ok {
223+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
224+
found++
225+
result = val
226+
resultKey = key
151227
}
152228
}
153-
return def, nil
229+
if found == 0 {
230+
return def, nil
231+
}
232+
if found > 1 {
233+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
234+
}
235+
return parse(resultKey, result)
154236
}
155237
func envFloat(def float64, key string, alts ...string) float64 {
156238
val, err := envFloatOrError(def, key, alts...)
@@ -171,16 +253,30 @@ func envDurationOrError(def time.Duration, key string, alts ...string) (time.Dur
171253
return 0, fmt.Errorf("invalid duration env %s=%q: %w", key, val, err)
172254
}
173255

174-
if val := os.Getenv(key); val != "" {
175-
return parse(key, val)
256+
found := 0
257+
result := ""
258+
resultKey := ""
259+
260+
if val, ok := os.LookupEnv(key); ok {
261+
found++
262+
result = val
263+
resultKey = key
176264
}
177265
for _, alt := range alts {
178-
if val := os.Getenv(alt); val != "" {
179-
fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key)
180-
return parse(alt, val)
266+
if val, ok := os.LookupEnv(alt); ok {
267+
envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key)
268+
found++
269+
result = val
270+
resultKey = key
181271
}
182272
}
183-
return def, nil
273+
if found == 0 {
274+
return def, nil
275+
}
276+
if found > 1 {
277+
envWarnf("env $%s was overridden by $%s\n", key, resultKey)
278+
}
279+
return parse(resultKey, result)
184280
}
185281
func envDuration(def time.Duration, key string, alts ...string) time.Duration {
186282
val, err := envDurationOrError(def, key, alts...)

0 commit comments

Comments
 (0)