Only set necessary environment variables.

Signed-off-by: Daniel Nephin <[email protected]>

main.go

@@ -161,13 +161,15 @@ Severity override map (default is "warning"):
 `, formatLinters(), formatSeverity())
 	kingpin.Parse()
 
-	configureEnvironment()
-
 	if config.Install {
+		if config.VendoredLinters {
+			configureEnvironmentForInstall()
+		}
 		installLinters()
 		return
 	}
 
+	configureEnvironment()
 	include, exclude := processConfig(config)
 
 	start := time.Now()
@@ -419,62 +421,66 @@ func getGoPathList() []string {
 	return strings.Split(getGoPath(), string(os.PathListSeparator))
 }
 
-// addPath appends p to paths and returns it if:
-// 1. p is not a blank string
-// 2. p doesn't already exist in paths
-// Otherwise paths is returned unchanged.
-func addPath(p string, paths []string) []string {
-	if p == "" {
-		return paths
-	}
-	for _, path := range paths {
-		if p == path {
+// addPath appends path to paths if path does not already exist in paths. Returns
+// the new paths.
+func addPath(paths []string, path string) []string {
+	for _, existingpath := range paths {
+		if path == existingpath {
 			return paths
 		}
 	}
-	return append(paths, p)
+	return append(paths, path)
 }
 
-// Ensure all "bin" directories from GOPATH exists in PATH, as well as GOBIN if set.
+// configureEnvironment adds all `bin/` directories from $GOPATH to $PATH
 func configureEnvironment() {
-	gopaths := getGoPathList()
+	paths := addGoBinsToPath(getGoPathList())
+	setEnv("PATH", strings.Join(paths, string(os.PathListSeparator)))
+	debugPrintEnv()
+}
+
+func addGoBinsToPath(gopaths []string) []string {
 	paths := strings.Split(os.Getenv("PATH"), string(os.PathListSeparator))
+	for _, p := range gopaths {
+		paths = addPath(paths, filepath.Join(p, "bin"))
+	}
 	gobin := os.Getenv("GOBIN")
+	if gobin != "" {
+		paths = addPath(paths, gobin)
+	}
+	return paths
+}
 
-	if config.VendoredLinters && config.Install {
-		vendorRoot := findVendoredLinters()
-		if vendorRoot == "" {
-			kingpin.Fatalf("could not find vendored linters in GOPATH=%q", getGoPath())
-		}
-		debug("found vendored linters at %s, updating environment", vendorRoot)
-		if gobin == "" {
-			gobin = filepath.Join(gopaths[0], "bin")
-		}
-		// "go install" panics when one GOPATH element is beneath another, so we just set
-		// our vendor root instead.
-		gopaths = []string{vendorRoot}
+// configureEnvironmentForInstall sets GOPATH and GOBIN so that vendored linters
+// can be installed
+func configureEnvironmentForInstall() {
+	gopaths := getGoPathList()
+	vendorRoot := findVendoredLinters()
+	if vendorRoot == "" {
+		kingpin.Fatalf("could not find vendored linters in GOPATH=%q", getGoPath())
 	}
+	debug("found vendored linters at %s, updating environment", vendorRoot)
 
-	for _, p := range gopaths {
-		paths = addPath(filepath.Join(p, "bin"), paths)
+	gobin := os.Getenv("GOBIN")
+	if gobin == "" {
+		gobin = filepath.Join(gopaths[0], "bin")
 	}
-	paths = addPath(gobin, paths)
+	setEnv("GOBIN", gobin)
 
-	path := strings.Join(paths, string(os.PathListSeparator))
-	gopath := strings.Join(gopaths, string(os.PathListSeparator))
+	// "go install" panics when one GOPATH element is beneath another, so set
+	// GOPATH to the vendor root
+	setEnv("GOPATH", vendorRoot)
+	debugPrintEnv()
+}
 
-	if err := os.Setenv("PATH", path); err != nil {
-		warning("setenv PATH: %s", err)
+func setEnv(key string, value string) {
+	if err := os.Setenv(key, value); err != nil {
+		warning("setenv %s: %s", key, err)
 	}
-	debug("PATH=%s", os.Getenv("PATH"))
+}
 
-	if err := os.Setenv("GOPATH", gopath); err != nil {
-		warning("setenv GOPATH: %s", err)
-	}
+func debugPrintEnv() {
+	debug("PATH=%s", os.Getenv("PATH"))
 	debug("GOPATH=%s", os.Getenv("GOPATH"))
-
-	if err := os.Setenv("GOBIN", gobin); err != nil {
-		warning("setenv GOBIN: %s", err)
-	}
 	debug("GOBIN=%s", os.Getenv("GOBIN"))
 }

main_test.go

@@ -155,3 +155,10 @@ func TestDeadlineFlag(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, 2*time.Minute, config.Deadline.Duration())
 }
+
+func TestAddPath(t *testing.T) {
+	paths := []string{"existing"}
+	assert.Equal(t, paths, addPath(paths, "existing"))
+	expected := []string{"existing", "new"}
+	assert.Equal(t, expected, addPath(paths, "new"))
+}