Warn when plz update updates a binary not in PATH

Author: sean-

src/update/update.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strconv"
@@ -91,6 +92,9 @@ func CheckAndUpdate(config *core.Configuration, updatesEnabled, updateCommand, f
 	// Clean out any old ones
 	clean(config, updateCommand)
 
+	// Warn if the binary in PATH isn't the one we just updated.
+	warnIfNotInPath(config)
+
 	// Now run the new one.
 	core.ReturnToInitialWorkingDir()
 	args := filterArgs(forceUpdate, append([]string{newPlease}, os.Args[1:]...))
@@ -406,6 +410,28 @@ func writeTarFile(hdr *tar.Header, r io.Reader, destination string) error {
 	return err
 }
 
+// warnIfNotInPath emits a warning if the binary found via PATH is not the one we just updated.
+// This catches the common case where e.g. a Homebrew-installed plz shadows the one in ~/.please.
+func warnIfNotInPath(config *core.Configuration) {
+	name := filepath.Base(os.Args[0])
+	pathBinary, err := exec.LookPath(name)
+	if err != nil {
+		return
+	}
+	pathBinary, _ = filepath.Abs(pathBinary)
+	if resolved, err := filepath.EvalSymlinks(pathBinary); err == nil {
+		pathBinary = resolved
+	}
+	location, _ := filepath.Abs(config.Please.Location)
+	if !strings.HasPrefix(pathBinary, location+string(filepath.Separator)) && pathBinary != location {
+		log.Warning("Updated %s in %s but %q in your PATH resolves to %s",
+			config.Please.Version.VersionString(), location, name, pathBinary)
+		log.Warning("To use the updated version, add %s to your PATH ahead of %s:",
+			location, filepath.Dir(pathBinary))
+		log.Warning("  export PATH=%s:$PATH", location)
+	}
+}
+
 // filterArgs filters out the --force update if forced updates were specified.
 // This is important so that we don't end up in a loop of repeatedly forcing re-downloads.
 func filterArgs(forceUpdate bool, args []string) []string {

src/update/update_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
 	"testing"
@@ -151,6 +152,31 @@ func TestFilterArgs(t *testing.T) {
 	assert.Equal(t, []string{"plz", "update"}, filterArgs(true, []string{"plz", "update", "--force"}))
 }
 
+func TestWarnIfNotInPath(t *testing.T) {
+	// Set os.Args[0] to something findable in PATH
+	oldArgs := os.Args
+	defer func() { os.Args = oldArgs }()
+	os.Args = []string{"go"}
+
+	// Look up where "go" actually lives
+	goPath, err := exec.LookPath("go")
+	assert.NoError(t, err)
+	goDir, _ := filepath.Abs(filepath.Dir(goPath))
+	if resolved, err := filepath.EvalSymlinks(goDir); err == nil {
+		goDir = resolved
+	}
+
+	// When location matches, no warning is expected (we can't easily assert on log output,
+	// but we can at least verify it doesn't panic).
+	c := makeConfig("warnpath")
+	c.Please.Location = goDir
+	warnIfNotInPath(c) // should not warn
+
+	// When location doesn't match, it should warn (again, just verify no panic).
+	c.Please.Location = "/nonexistent/path"
+	warnIfNotInPath(c) // should warn but not panic
+}
+
 func TestFullDistVersion(t *testing.T) {
 	var v cli.Version
 	v.UnmarshalFlag("13.1.9")