diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index 786b2212a..a373f087d 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -6,7 +6,6 @@ import ( "net/http" "os" "path/filepath" - "strings" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/context" @@ -16,14 +15,13 @@ import ( "github.com/cli/cli/v2/pkg/cmd/extension" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/cli/safeexec" ) func New(appVersion string) *cmdutil.Factory { f := &cmdutil.Factory{ - Config: configFunc(), // No factory dependencies - Branch: branchFunc(), // No factory dependencies - Executable: executable(), // No factory dependencies + Config: configFunc(), // No factory dependencies + Branch: branchFunc(), // No factory dependencies + Executable: executable("gh"), // No factory dependencies ExtensionManager: extension.NewManager(), } @@ -116,24 +114,50 @@ func browserLauncher(f *cmdutil.Factory) string { return os.Getenv("BROWSER") } -func executable() string { - exe, _ := os.Executable() +// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks. +// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in +// PATH, return the absolute location to the program. +// +// The idea is that the result of this function is callable in the future and refers to the same +// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software +// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`. +// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of +// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew +// location. +// +// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute +// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git +// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh +// auth login`, running `brew update` will print out authentication errors as git is unable to locate +// Homebrew-installed `gh`. +func executable(fallbackName string) string { + exe, err := os.Executable() + if err != nil { + return fallbackName + } + base := filepath.Base(exe) path := os.Getenv("PATH") for _, dir := range filepath.SplitList(path) { - if strings.HasSuffix(dir, "gh") { - if dir == exe { - return dir - } - if symlink, _ := os.Readlink(dir); symlink == exe { - return dir + p, err := filepath.Abs(filepath.Join(dir, base)) + if err != nil { + continue + } + f, err := os.Stat(p) + if err != nil { + continue + } + + if p == exe { + return p + } else if f.Mode()&os.ModeSymlink != 0 { + if t, err := os.Readlink(p); err == nil && t == exe { + return p } } } - gh, _ := safeexec.LookPath("gh") - - return gh + return exe } func configFunc() func() (config.Config, error) {