Skip to content

Commit 1cd5b00

Browse files
committed
Warn when plz update updates a binary not in PATH
1 parent e128a94 commit 1cd5b00

2 files changed

Lines changed: 52 additions & 0 deletions

File tree

src/update/update.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"io"
1616
"net/http"
1717
"os"
18+
"os/exec"
1819
"path/filepath"
1920
"runtime"
2021
"strconv"
@@ -91,6 +92,9 @@ func CheckAndUpdate(config *core.Configuration, updatesEnabled, updateCommand, f
9192
// Clean out any old ones
9293
clean(config, updateCommand)
9394

95+
// Warn if the binary in PATH isn't the one we just updated.
96+
warnIfNotInPath(config)
97+
9498
// Now run the new one.
9599
core.ReturnToInitialWorkingDir()
96100
args := filterArgs(forceUpdate, append([]string{newPlease}, os.Args[1:]...))
@@ -406,6 +410,28 @@ func writeTarFile(hdr *tar.Header, r io.Reader, destination string) error {
406410
return err
407411
}
408412

413+
// warnIfNotInPath emits a warning if the binary found via PATH is not the one we just updated.
414+
// This catches the common case where e.g. a Homebrew-installed plz shadows the one in ~/.please.
415+
func warnIfNotInPath(config *core.Configuration) {
416+
name := filepath.Base(os.Args[0])
417+
pathBinary, err := exec.LookPath(name)
418+
if err != nil {
419+
return
420+
}
421+
pathBinary, _ = filepath.Abs(pathBinary)
422+
if resolved, err := filepath.EvalSymlinks(pathBinary); err == nil {
423+
pathBinary = resolved
424+
}
425+
location, _ := filepath.Abs(config.Please.Location)
426+
if !strings.HasPrefix(pathBinary, location+string(filepath.Separator)) && pathBinary != location {
427+
log.Warning("Updated %s in %s but %q in your PATH resolves to %s",
428+
config.Please.Version.VersionString(), location, name, pathBinary)
429+
log.Warning("To use the updated version, add %s to your PATH ahead of %s:",
430+
location, filepath.Dir(pathBinary))
431+
log.Warning(" export PATH=%s:$PATH", location)
432+
}
433+
}
434+
409435
// filterArgs filters out the --force update if forced updates were specified.
410436
// This is important so that we don't end up in a loop of repeatedly forcing re-downloads.
411437
func filterArgs(forceUpdate bool, args []string) []string {

src/update/update_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,32 @@ func TestFilterArgs(t *testing.T) {
151151
assert.Equal(t, []string{"plz", "update"}, filterArgs(true, []string{"plz", "update", "--force"}))
152152
}
153153

154+
func TestWarnIfNotInPath(t *testing.T) {
155+
// Create a fake binary in a temp directory and put it on PATH.
156+
tmpDir := t.TempDir()
157+
fakeBin := filepath.Join(tmpDir, "fake_plz")
158+
assert.NoError(t, os.WriteFile(fakeBin, []byte("#!/bin/sh\n"), 0755))
159+
160+
oldPath := os.Getenv("PATH")
161+
t.Setenv("PATH", tmpDir+string(os.PathListSeparator)+oldPath)
162+
oldArgs := os.Args
163+
defer func() { os.Args = oldArgs }()
164+
os.Args = []string{"fake_plz"}
165+
166+
// When location matches the directory containing the binary, no warning expected.
167+
c := makeConfig("warnpath")
168+
c.Please.Location = tmpDir
169+
warnIfNotInPath(c) // should not warn
170+
171+
// When location doesn't match, it should warn but not panic.
172+
c.Please.Location = "/nonexistent/path"
173+
warnIfNotInPath(c) // should warn but not panic
174+
175+
// When the binary isn't in PATH at all, should silently return.
176+
os.Args = []string{"not_in_path_at_all"}
177+
warnIfNotInPath(c) // should not panic
178+
}
179+
154180
func TestFullDistVersion(t *testing.T) {
155181
var v cli.Version
156182
v.UnmarshalFlag("13.1.9")

0 commit comments

Comments
 (0)