Fix determining current process location

mislav · mislav · commit d731cb9c73d7 · 2021-09-06T16:57:59.000+02:00
diff --git 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) {
