diff --git a/README.md b/README.md index 935b6fc87..e05e9a693 100644 --- a/README.md +++ b/README.md @@ -587,4 +587,3 @@ HOOKS if a hook fails and a new hash is synced during the backoff period, the retried hook will fire for the newest hash. ``` - diff --git a/env.go b/env.go index 80c64d7a9..9b401cf66 100644 --- a/env.go +++ b/env.go @@ -29,17 +29,42 @@ import ( "github.com/spf13/pflag" ) +// Tests can set this or set it to nil. +var envWarnfOverride func(format string, args ...any) + +func envWarnf(format string, args ...any) { + if envWarnfOverride != nil { + envWarnfOverride(format, args...) + } else { + fmt.Fprintf(os.Stderr, format, args...) + } +} + func envString(def string, key string, alts ...string) string { - if val := os.Getenv(key); val != "" { - return val + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return val + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = alt } } - return def + if found == 0 { + return def + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + return result } func envFlagString(key string, def string, usage string, alts ...string) *string { registerEnvFlag(key, "string", usage) @@ -60,16 +85,31 @@ func envStringArray(def string, key string, alts ...string) []string { return strings.Split(s, ":") } - if val := os.Getenv(key); val != "" { - return parse(val) + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return parse(val) + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = key } } - return parse(def) + if found == 0 { + return parse(def) + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + + return parse(result) } func envBoolOrError(def bool, key string, alts ...string) (bool, error) { @@ -81,16 +121,30 @@ func envBoolOrError(def bool, key string, alts ...string) (bool, error) { return false, fmt.Errorf("invalid bool env %s=%q: %w", key, val, err) } - if val := os.Getenv(key); val != "" { - return parse(key, val) + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return parse(alt, val) + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = key } } - return def, nil + if found == 0 { + return def, nil + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + return parse(resultKey, result) } func envBool(def bool, key string, alts ...string) bool { val, err := envBoolOrError(def, key, alts...) @@ -111,16 +165,30 @@ func envIntOrError(def int, key string, alts ...string) (int, error) { return 0, fmt.Errorf("invalid int env %s=%q: %w", key, val, err) } - if val := os.Getenv(key); val != "" { - return parse(key, val) + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return parse(alt, val) + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = key } } - return def, nil + if found == 0 { + return def, nil + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + return parse(resultKey, result) } func envInt(def int, key string, alts ...string) int { val, err := envIntOrError(def, key, alts...) @@ -141,16 +209,30 @@ func envFloatOrError(def float64, key string, alts ...string) (float64, error) { return 0, fmt.Errorf("invalid float env %s=%q: %w", key, val, err) } - if val := os.Getenv(key); val != "" { - return parse(key, val) + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return parse(alt, val) + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = key } } - return def, nil + if found == 0 { + return def, nil + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + return parse(resultKey, result) } func envFloat(def float64, key string, alts ...string) float64 { val, err := envFloatOrError(def, key, alts...) @@ -171,16 +253,30 @@ func envDurationOrError(def time.Duration, key string, alts ...string) (time.Dur return 0, fmt.Errorf("invalid duration env %s=%q: %w", key, val, err) } - if val := os.Getenv(key); val != "" { - return parse(key, val) + found := 0 + result := "" + resultKey := "" + + if val, ok := os.LookupEnv(key); ok { + found++ + result = val + resultKey = key } for _, alt := range alts { - if val := os.Getenv(alt); val != "" { - fmt.Fprintf(os.Stderr, "env $%s has been deprecated, use $%s instead\n", alt, key) - return parse(alt, val) + if val, ok := os.LookupEnv(alt); ok { + envWarnf("env $%s has been deprecated, use $%s instead\n", alt, key) + found++ + result = val + resultKey = key } } - return def, nil + if found == 0 { + return def, nil + } + if found > 1 { + envWarnf("env $%s was overridden by $%s\n", key, resultKey) + } + return parse(resultKey, result) } func envDuration(def time.Duration, key string, alts ...string) time.Duration { val, err := envDurationOrError(def, key, alts...) diff --git a/env_test.go b/env_test.go index b369db2f6..8b5b52ca0 100644 --- a/env_test.go +++ b/env_test.go @@ -24,120 +24,187 @@ import ( const ( testKey = "KEY" + alt1Key = "ALT1" + alt2Key = "ALT2" ) +func setupEnv(val, alt1, alt2 string) { + if val != "" { + os.Setenv(testKey, val) + } + if alt1 != "" { + os.Setenv(alt1Key, alt1) + } + if alt2 != "" { + os.Setenv(alt2Key, alt2) + } +} + +func resetEnv() { + os.Unsetenv(testKey) + os.Unsetenv(alt1Key) + os.Unsetenv(alt2Key) +} + func TestEnvBool(t *testing.T) { + envWarnfOverride = func(format string, args ...any) { + t.Logf(format, args...) + } + defer func() { envWarnfOverride = nil }() + cases := []struct { value string + alt1 string + alt2 string def bool exp bool err bool }{ - {"true", true, true, false}, - {"true", false, true, false}, - {"", true, true, false}, - {"", false, false, false}, - {"false", true, false, false}, - {"false", false, false, false}, - {"", true, true, false}, - {"", false, false, false}, - {"no true", false, false, true}, - {"no false", false, false, true}, - } - - for _, testCase := range cases { - os.Setenv(testKey, testCase.value) - val, err := envBoolOrError(testCase.def, testKey) - if err != nil && !testCase.err { - t.Fatalf("%q: unexpected error: %v", testCase.value, err) + {"true", "", "", true, true, false}, + {"true", "", "", false, true, false}, + {"", "", "", true, true, false}, + {"", "", "", false, false, false}, + {"false", "", "", true, false, false}, + {"false", "", "", false, false, false}, + {"", "", "", true, true, false}, + {"", "", "", false, false, false}, + {"invalid", "", "", false, false, true}, + {"invalid", "true", "", false, true, false}, + {"true", "invalid", "", false, false, true}, + {"invalid", "invalid", "true", false, true, false}, + {"true", "true", "invalid", false, false, true}, + {"invalid", "invalid", "invalid", false, false, true}, + } + + for i, tc := range cases { + resetEnv() + setupEnv(tc.value, tc.alt1, tc.alt2) + val, err := envBoolOrError(tc.def, testKey, alt1Key, alt2Key) + if err != nil && !tc.err { + t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err) } - if err == nil && testCase.err { - t.Fatalf("%q: unexpected success", testCase.value) + if err == nil && tc.err { + t.Fatalf("%d: %q: unexpected success", i, tc.value) } - if val != testCase.exp { - t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) + if val != tc.exp { + t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val) } } } func TestEnvString(t *testing.T) { + envWarnfOverride = func(format string, args ...any) { + t.Logf(format, args...) + } + defer func() { envWarnfOverride = nil }() + cases := []struct { value string + alt1 string + alt2 string def string exp string }{ - {"true", "true", "true"}, - {"true", "false", "true"}, - {"", "true", "true"}, - {"", "false", "false"}, - {"false", "true", "false"}, - {"false", "false", "false"}, - {"", "true", "true"}, - {"", "false", "false"}, - } - - for _, testCase := range cases { - os.Setenv(testKey, testCase.value) - val := envString(testCase.def, testKey) - if val != testCase.exp { - t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) + {"foo", "", "", "foo", "foo"}, + {"foo", "", "", "bar", "foo"}, + {"", "", "", "foo", "foo"}, + {"", "", "", "bar", "bar"}, + {"bar", "", "", "foo", "bar"}, + {"bar", "", "", "bar", "bar"}, + {"", "", "", "foo", "foo"}, + {"", "", "", "bar", "bar"}, + {"foo1", "foo2", "", "bar", "foo2"}, + {"foo1", "foo2", "foo3", "bar", "foo3"}, + } + + for i, tc := range cases { + resetEnv() + setupEnv(tc.value, tc.alt1, tc.alt2) + val := envString(tc.def, testKey, alt1Key, alt2Key) + if val != tc.exp { + t.Fatalf("%d: expected: %q, got: %q", i, tc.exp, val) } } } func TestEnvInt(t *testing.T) { + envWarnfOverride = func(format string, args ...any) { + t.Logf(format, args...) + } + defer func() { envWarnfOverride = nil }() + cases := []struct { value string + alt1 string + alt2 string def int exp int err bool }{ - {"0", 1, 0, false}, - {"", 0, 0, false}, - {"-1", 0, -1, false}, - {"abcd", 0, 0, true}, - {"abcd", 0, 0, true}, - } - - for _, testCase := range cases { - os.Setenv(testKey, testCase.value) - val, err := envIntOrError(testCase.def, testKey) - if err != nil && !testCase.err { - t.Fatalf("%q: unexpected error: %v", testCase.value, err) + {"0", "", "", 1, 0, false}, + {"", "", "", 0, 0, false}, + {"-1", "", "", 0, -1, false}, + {"invalid", "", "", 0, 0, true}, + {"invalid", "0", "", 1, 0, false}, + {"0", "invalid", "", 0, 0, true}, + {"invalid", "invalid", "0", 1, 0, false}, + {"0", "0", "invalid", 0, 0, true}, + {"invalid", "invalid", "invalid", 0, 0, true}, + } + + for i, tc := range cases { + resetEnv() + setupEnv(tc.value, tc.alt1, tc.alt2) + val, err := envIntOrError(tc.def, testKey, alt1Key, alt2Key) + if err != nil && !tc.err { + t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err) } - if err == nil && testCase.err { - t.Fatalf("%q: unexpected success", testCase.value) + if err == nil && tc.err { + t.Fatalf("%d: %q: unexpected success", i, tc.value) } - if val != testCase.exp { - t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) + if val != tc.exp { + t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val) } } } func TestEnvFloat(t *testing.T) { + envWarnfOverride = func(format string, args ...any) { + t.Logf(format, args...) + } + defer func() { envWarnfOverride = nil }() + cases := []struct { value string + alt1 string + alt2 string def float64 exp float64 err bool }{ - {"0.5", 0, 0.5, false}, - {"", 0.5, 0.5, false}, - {"-0.5", 0, -0.5, false}, - {"abcd", 0, 0, true}, + {"0.5", "", "", 0, 0.5, false}, + {"", "", "", 0.5, 0.5, false}, + {"-0.5", "", "", 0, -0.5, false}, + {"invalid", "", "", 0, 0, true}, + {"invalid", "0.5", "", 0, 0.5, false}, + {"0.5", "invalid", "", 0, 0, true}, + {"invalid", "invalid", "0.5", 0, 0.5, false}, + {"0.5", "0.5", "invalid", 0, 0, true}, + {"invalid", "invalid", "invalid", 0, 0, true}, } - for _, testCase := range cases { - os.Setenv(testKey, testCase.value) - val, err := envFloatOrError(testCase.def, testKey) - if err != nil && !testCase.err { - t.Fatalf("%q: unexpected error: %v", testCase.value, err) + for i, tc := range cases { + resetEnv() + setupEnv(tc.value, tc.alt1, tc.alt2) + val, err := envFloatOrError(tc.def, testKey, alt1Key, alt2Key) + if err != nil && !tc.err { + t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err) } - if err == nil && testCase.err { - t.Fatalf("%q: unexpected success", testCase.value) + if err == nil && tc.err { + t.Fatalf("%d: %q: unexpected success", i, tc.value) } - if val != testCase.exp { - t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) + if val != tc.exp { + t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val) } } } @@ -145,27 +212,35 @@ func TestEnvFloat(t *testing.T) { func TestEnvDuration(t *testing.T) { cases := []struct { value string + alt1 string + alt2 string def time.Duration exp time.Duration err bool }{ - {"1s", 0, time.Second, false}, - {"", time.Minute, time.Minute, false}, - {"1h", 0, time.Hour, false}, - {"abcd", 0, 0, true}, + {"1s", "", "", 0, time.Second, false}, + {"", "", "", time.Minute, time.Minute, false}, + {"1h", "", "", 0, time.Hour, false}, + {"invalid", "", "", 0, 0, true}, + {"invalid", "1s", "", 0, time.Second, false}, + {"1s", "invalid", "", 0, 0, true}, + {"invalid", "invalid", "1s", 0, time.Second, false}, + {"1s", "1s", "invalid", 0, 0, true}, + {"invalid", "invalid", "invalid", 0, 0, true}, } - for _, testCase := range cases { - os.Setenv(testKey, testCase.value) - val, err := envDurationOrError(testCase.def, testKey) - if err != nil && !testCase.err { - t.Fatalf("%q: unexpected error: %v", testCase.value, err) + for i, tc := range cases { + resetEnv() + setupEnv(tc.value, tc.alt1, tc.alt2) + val, err := envDurationOrError(tc.def, testKey, alt1Key, alt2Key) + if err != nil && !tc.err { + t.Fatalf("%d: %q: unexpected error: %v", i, tc.value, err) } - if err == nil && testCase.err { - t.Fatalf("%q: unexpected success", testCase.value) + if err == nil && tc.err { + t.Fatalf("%d: %q: unexpected success", i, tc.value) } - if val != testCase.exp { - t.Fatalf("%q: expected %v but %v returned", testCase.value, testCase.exp, val) + if val != tc.exp { + t.Fatalf("%d: expected: %v, got: %v", i, tc.exp, val) } } } diff --git a/v3-to-v4.md b/v3-to-v4.md index aa1639be9..19bc13b25 100644 --- a/v3-to-v4.md +++ b/v3-to-v4.md @@ -159,6 +159,11 @@ Most flags can also be configured by environment variables. In v3 the variables all start with `GIT_SYNC_`. In v4 they all start with `GITSYNC_`, though the old names are still accepted for compatibility. +If both an old (`GIT_SYNC_*`) name and a new (`GITSYNC_*`) name are specified, +the behavior is: +* v4.0.x - v4.3.x: the new name is used +* v4.4.x and up: the old name is used + ## Defaults ### Depth