From 4cfa74e52bcec64e6c2432fe7ee4c1a89bd528d7 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 30 Jun 2020 14:48:26 +0200 Subject: [PATCH 01/12] Deprecate -endpoint flag This removes the documentation of the endpoint flag but still allows it to be used to override either the config or ennvironment variables. --- CHANGELOG.md | 3 + cmd/src/main.go | 45 +++++++---- cmd/src/main_test.go | 179 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 cmd/src/main_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f0e541527..9d5213135f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,10 @@ All notable changes to `src-cli` are documented in this file. ### Changed - The terminal UI has been replaced by the logger-based UI that was previously only visible in verbose-mode (`-v`). [#228](https://github.com/sourcegraph/src-cli/pull/228) +- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. ### Fixed ### Removed + +- Remove the `-endpoint` flag. [#225](https://github.com/sourcegraph/src-cli/pull/225) diff --git a/cmd/src/main.go b/cmd/src/main.go index e3bcc3b642..c74b6f34f2 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -9,6 +9,8 @@ import ( "os/user" "path/filepath" "strings" + + "github.com/pkg/errors" ) const usageText = `src is a tool that provides access to Sourcegraph instances. @@ -24,7 +26,6 @@ Environment variables The options are: - -endpoint= specifies the endpoint to use e.g. "https://sourcegraph.com" (overrides SRC_ENDPOINT if set) -v print verbose output The commands are: @@ -47,9 +48,11 @@ Use "src [command] -h" for more information about a command. ` var ( + verbose = flag.Bool("v", false, "print verbose output") + + // The following arguments are deprecated which is why they are no longer documented configPath = flag.String("config", "", "") endpoint = flag.String("endpoint", "", "") - verbose = flag.Bool("v", false, "print verbose output") ) // commands contains all registered subcommands. @@ -76,14 +79,14 @@ func readConfig() (*config, error) { cfgPath := *configPath userSpecified := *configPath != "" - user, err := user.Current() + u, err := user.Current() if err != nil { return nil, err } if !userSpecified { - cfgPath = filepath.Join(user.HomeDir, "src-config.json") + cfgPath = filepath.Join(u.HomeDir, "src-config.json") } else if strings.HasPrefix(cfgPath, "~/") { - cfgPath = filepath.Join(user.HomeDir, cfgPath[2:]) + cfgPath = filepath.Join(u.HomeDir, cfgPath[2:]) } data, err := ioutil.ReadFile(os.ExpandEnv(cfgPath)) if err != nil && (!os.IsNotExist(err) || userSpecified) { @@ -96,23 +99,39 @@ func readConfig() (*config, error) { } } + envToken := os.Getenv("SRC_ACCESS_TOKEN") + envEndpoint := os.Getenv("SRC_ENDPOINT") + + if userSpecified { + // If a config file is present, either zero or both environment variables must be present. + // We don't want to partially apply environment variables. + if envToken == "" && envEndpoint != "" { + return nil, errConfigMerge + } + if envToken != "" && envEndpoint == "" { + return nil, errConfigMerge + } + } + // Apply config overrides. - if envToken := os.Getenv("SRC_ACCESS_TOKEN"); envToken != "" { + if envToken != "" { cfg.AccessToken = envToken } - if *endpoint != "" { - cfg.Endpoint = *endpoint - } - if cfg.Endpoint == "" { - if endpoint := os.Getenv("SRC_ENDPOINT"); endpoint != "" { - cfg.Endpoint = endpoint - } + if envEndpoint != "" { + cfg.Endpoint = envEndpoint } if cfg.Endpoint == "" { cfg.Endpoint = "https://sourcegraph.com" } + // Lastly, apply endpoint flag if set + if endpoint != nil && *endpoint != "" { + cfg.Endpoint = *endpoint + } + cfg.Endpoint = strings.TrimSuffix(cfg.Endpoint, "/") return &cfg, nil } + +var errConfigMerge = errors.New("config merging not supported, zero or both environment variables must be set") diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go new file mode 100644 index 0000000000..91bbff7885 --- /dev/null +++ b/cmd/src/main_test.go @@ -0,0 +1,179 @@ +package main + +import ( + "encoding/json" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestReadConfig(t *testing.T) { + makeTempConfig := func(t *testing.T, c config) (string, func()) { + data, err := json.Marshal(c) + if err != nil { + t.Fatal(err) + } + tmpDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + filePath := filepath.Join(tmpDir, "config.json") + err = ioutil.WriteFile(filePath, data, 0600) + if err != nil { + t.Fatal(err) + } + return filePath, func() { os.RemoveAll(tmpDir) } + } + + tests := []struct { + name string + fileContents *config + envToken string + envEndpoint string + flagEndpoint string + want *config + wantErr string + }{ + { + name: "defaults", + want: &config{ + Endpoint: "https://sourcegraph.com", + }, + }, + { + name: "config file, no overrides, trim slash", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + want: &config{ + Endpoint: "https://example.com", + AccessToken: "deadbeef", + }, + }, + { + name: "config file, token override only", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envToken: "abc", + want: nil, + wantErr: errConfigMerge.Error(), + }, + { + name: "config file, endpoint override only", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envEndpoint: "https://exmaple2.com", + want: nil, + wantErr: errConfigMerge.Error(), + }, + { + name: "config file, both override", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + envToken: "abc", + envEndpoint: "https://override.com", + want: &config{ + Endpoint: "https://override.com", + AccessToken: "abc", + }, + }, + { + name: "no config file, token from environment", + envToken: "abc", + want: &config{ + Endpoint: "https://sourcegraph.com", + AccessToken: "abc", + }, + }, + { + name: "no config file, endpoint from environment", + envEndpoint: "https://example.com", + want: &config{ + Endpoint: "https://example.com", + AccessToken: "", + }, + }, + { + name: "no config file, both variables", + envEndpoint: "https://example.com", + envToken: "abc", + want: &config{ + Endpoint: "https://example.com", + AccessToken: "abc", + }, + }, + { + name: "endpoint flag should override config", + flagEndpoint: "https://override.com/", + fileContents: &config{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + }, + want: &config{ + Endpoint: "https://override.com", + AccessToken: "deadbeef", + }, + }, + { + name: "endpoint flag should override environment", + flagEndpoint: "https://override.com/", + envEndpoint: "https://example.com", + envToken: "abc", + want: &config{ + Endpoint: "https://override.com", + AccessToken: "abc", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + oldConfigPath := *configPath + defer func() { *configPath = oldConfigPath }() + + if test.flagEndpoint != "" { + val := test.flagEndpoint + endpoint = &val + defer func() { endpoint = nil }() + } + + if test.fileContents != nil { + p, cleanup := makeTempConfig(t, *test.fileContents) + defer cleanup() + *configPath = p + } + oldToken := os.Getenv("SRC_ACCESS_TOKEN") + defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }() + oldEndpoint := os.Getenv("SRC_ENDPOINT") + defer func() { os.Setenv("SRC_ENDPOINT", oldEndpoint) }() + + if err := os.Setenv("SRC_ACCESS_TOKEN", test.envToken); err != nil { + t.Fatal(err) + } + if err := os.Setenv("SRC_ENDPOINT", test.envEndpoint); err != nil { + t.Fatal(err) + } + config, err := readConfig() + if diff := cmp.Diff(test.want, config); diff != "" { + t.Errorf("config: %v", diff) + } + var errMsg string + if err != nil { + errMsg = err.Error() + } + if diff := cmp.Diff(test.wantErr, errMsg); diff != "" { + t.Errorf("err: %v", diff) + } + }) + } +} From b269aee437175f8bfef739c1bb95454bed082fc1 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Tue, 30 Jun 2020 15:06:44 +0200 Subject: [PATCH 02/12] Update changelog. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d5213135f..527f41207c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ All notable changes to `src-cli` are documented in this file. ### Changed - The terminal UI has been replaced by the logger-based UI that was previously only visible in verbose-mode (`-v`). [#228](https://github.com/sourcegraph/src-cli/pull/228) -- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. +- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) ### Fixed From 14590187fc5e302ba68c5e4f806ee74cf3bad7ca Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 11:43:18 +0200 Subject: [PATCH 03/12] Update cmd/src/main.go Co-authored-by: Thorsten Ball --- cmd/src/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index c74b6f34f2..c91e9d0095 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -134,4 +134,4 @@ func readConfig() (*config, error) { return &cfg, nil } -var errConfigMerge = errors.New("config merging not supported, zero or both environment variables must be set") +var errConfigMerge = errors.New("when using a configuration file, zero or all environment variables must be set") From 83f9013553ed4d5821f31930121b73c4cd6cab2a Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 11:48:21 +0200 Subject: [PATCH 04/12] Clean up CHANGELOG --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527f41207c..207529ecec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,6 @@ All notable changes to `src-cli` are documented in this file. ### Added - Pull missing docker images automatically. [#191](https://github.com/sourcegraph/src-cli/pull/191) - - Searches that result in errors will now display any alerts returned by Sourcegraph, including suggestions for how the search could be corrected. [#221](https://github.com/sourcegraph/src-cli/pull/221) ### Changed @@ -26,4 +25,3 @@ All notable changes to `src-cli` are documented in this file. ### Removed -- Remove the `-endpoint` flag. [#225](https://github.com/sourcegraph/src-cli/pull/225) From 68e2befe077adf12b0ea1ca079a4b1be910dcebb Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 11:51:52 +0200 Subject: [PATCH 05/12] Use t.Cleanup --- cmd/src/main_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index 91bbff7885..9f2c97dec3 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -11,7 +11,7 @@ import ( ) func TestReadConfig(t *testing.T) { - makeTempConfig := func(t *testing.T, c config) (string, func()) { + makeTempConfig := func(t *testing.T, c config) string { data, err := json.Marshal(c) if err != nil { t.Fatal(err) @@ -25,7 +25,8 @@ func TestReadConfig(t *testing.T) { if err != nil { t.Fatal(err) } - return filePath, func() { os.RemoveAll(tmpDir) } + t.Cleanup(func() { os.RemoveAll(tmpDir) }) + return filePath } tests := []struct { @@ -148,9 +149,7 @@ func TestReadConfig(t *testing.T) { } if test.fileContents != nil { - p, cleanup := makeTempConfig(t, *test.fileContents) - defer cleanup() - *configPath = p + *configPath = makeTempConfig(t, *test.fileContents) } oldToken := os.Getenv("SRC_ACCESS_TOKEN") defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }() From 05841909bf62868ef9838fd1546e5665a3fb731d Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 11:56:11 +0200 Subject: [PATCH 06/12] Use Go 1.14 --- appveyor.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/appveyor.yml b/appveyor.yml index cdc900d85b..1a3e4c73ad 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,6 +2,7 @@ version: "{build}" clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli environment: GOPATH: c:\gopath +stack: go 1.14 install: - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% - echo %PATH% From e52af5fbeeae806a7a710e0cda9f65d001374f80 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 11:57:09 +0200 Subject: [PATCH 07/12] Use Go 1.14 in Travis too --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index ba01499a08..37821cac6d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: go services: - docker go: -- 1.13.x +- 1.14.x go_import_path: github.com/sourcegraph/src-cli install: - go get -d -t ./... From bcea809d6182ac8a89d56b35f74fef2197153b37 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 12:05:52 +0200 Subject: [PATCH 08/12] Try specific version of Go for Appveyor --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 1a3e4c73ad..b78d031f5e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,7 +2,7 @@ version: "{build}" clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli environment: GOPATH: c:\gopath -stack: go 1.14 +stack: go 1.14.3 install: - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% - echo %PATH% From 5f86ecef8a92ec5063428fa4a7458fee40e49923 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 12:11:41 +0200 Subject: [PATCH 09/12] Try example appveyor.yml --- appveyor.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index b78d031f5e..cb607b581a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,14 +1,14 @@ -version: "{build}" -clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli +build: off + +clone_folder: c:\gopath\src\github.com\souregraph/src-cli + environment: GOPATH: c:\gopath -stack: go 1.14.3 -install: - - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% - - echo %PATH% - - echo %GOPATH% - - go version - - go env - - go get -t -v ./... -build_script: - - go test -v ./... + +stack: go 1.14 + +before_test: + - go vet ./... + +test_script: + - go test ./... From ef2781825cdd1b34688671e8780471c4bcfa8e54 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 12:15:01 +0200 Subject: [PATCH 10/12] Revert "Try example appveyor.yml" This reverts commit 5f86ecef8a92ec5063428fa4a7458fee40e49923. --- appveyor.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index cb607b581a..b78d031f5e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,14 +1,14 @@ -build: off - -clone_folder: c:\gopath\src\github.com\souregraph/src-cli - +version: "{build}" +clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli environment: GOPATH: c:\gopath - -stack: go 1.14 - -before_test: - - go vet ./... - -test_script: - - go test ./... +stack: go 1.14.3 +install: + - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% + - echo %PATH% + - echo %GOPATH% + - go version + - go env + - go get -t -v ./... +build_script: + - go test -v ./... From b79431015daf9be5ad24da506f14329d104908b6 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 12:19:55 +0200 Subject: [PATCH 11/12] One more attempt, use different GOPATH Apparently Go 1.14.3 is installed by default: https://www.appveyor.com/docs/windows-images-software/#golang --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index b78d031f5e..64d5745d9a 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,7 +1,7 @@ version: "{build}" clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli environment: - GOPATH: c:\gopath + GOPATH: c:\go stack: go 1.14.3 install: - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% From 8a6f6917dc9fea4a9ea8b77fbaab0efd8a227ff4 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 1 Jul 2020 12:24:35 +0200 Subject: [PATCH 12/12] Revert back to being Go 1.13 compatible --- .travis.yml | 2 +- appveyor.yml | 3 +-- cmd/src/main_test.go | 9 +++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index 37821cac6d..ba01499a08 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,7 @@ language: go services: - docker go: -- 1.14.x +- 1.13.x go_import_path: github.com/sourcegraph/src-cli install: - go get -d -t ./... diff --git a/appveyor.yml b/appveyor.yml index 64d5745d9a..cdc900d85b 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,8 +1,7 @@ version: "{build}" clone_folder: c:\gopath\src\github.com\sourcegraph\src-cli environment: - GOPATH: c:\go -stack: go 1.14.3 + GOPATH: c:\gopath install: - set PATH=%GOPATH%\bin;c:\go\bin;%PATH% - echo %PATH% diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index 9f2c97dec3..91bbff7885 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -11,7 +11,7 @@ import ( ) func TestReadConfig(t *testing.T) { - makeTempConfig := func(t *testing.T, c config) string { + makeTempConfig := func(t *testing.T, c config) (string, func()) { data, err := json.Marshal(c) if err != nil { t.Fatal(err) @@ -25,8 +25,7 @@ func TestReadConfig(t *testing.T) { if err != nil { t.Fatal(err) } - t.Cleanup(func() { os.RemoveAll(tmpDir) }) - return filePath + return filePath, func() { os.RemoveAll(tmpDir) } } tests := []struct { @@ -149,7 +148,9 @@ func TestReadConfig(t *testing.T) { } if test.fileContents != nil { - *configPath = makeTempConfig(t, *test.fileContents) + p, cleanup := makeTempConfig(t, *test.fileContents) + defer cleanup() + *configPath = p } oldToken := os.Getenv("SRC_ACCESS_TOKEN") defer func() { os.Setenv("SRC_ACCESS_TOKEN", oldToken) }()