Skip to content

Commit 3cce16e

Browse files
mislavmislav
committed
Refactor factory.Executable() to be a method rather than a func
This way, factory can satisfy an interface that requires `Executable()`.
1 parent 38eb894 commit 3cce16e

File tree

4 files changed

+62
-63
lines changed

4 files changed

+62
-63
lines changed

‎pkg/cmd/auth/login/login_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,7 @@ func Test_NewCmdLogin(t *testing.T) {
147147
t.Run(tt.name, func(t *testing.T) {
148148
io, stdin, _, _ := iostreams.Test()
149149
f := &cmdutil.Factory{
150-
IOStreams: io,
151-
Executable: func() string { return "/path/to/gh" },
150+
IOStreams: io,
152151
}
153152

154153
io.SetStdoutTTY(true)

‎pkg/cmd/auth/refresh/refresh_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ func Test_NewCmdRefresh(t *testing.T) {
9191
t.Run(tt.name, func(t *testing.T) {
9292
io, _, _, _ := iostreams.Test()
9393
f := &cmdutil.Factory{
94-
IOStreams: io,
95-
Executable: func() string { return "/path/to/gh" },
94+
IOStreams: io,
9695
}
9796
io.SetStdinTTY(tt.tty)
9897
io.SetStdoutTTY(tt.tty)

‎pkg/cmd/factory/default.go‎

Lines changed: 3 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"net/http"
77
"os"
8-
"path/filepath"
98
"time"
109

1110
"github.com/cli/cli/v2/api"
@@ -19,17 +18,10 @@ import (
1918
)
2019

2120
func New(appVersion string) *cmdutil.Factory {
22-
var exe string
2321
f := &cmdutil.Factory{
24-
Config: configFunc(), // No factory dependencies
25-
Branch: branchFunc(), // No factory dependencies
26-
Executable: func() string {
27-
if exe != "" {
28-
return exe
29-
}
30-
exe = executable("gh")
31-
return exe
32-
},
22+
Config: configFunc(), // No factory dependencies
23+
Branch: branchFunc(), // No factory dependencies
24+
ExecutableName: "gh",
3325
}
3426

3527
f.IOStreams = ioStreams(f) // Depends on Config
@@ -121,52 +113,6 @@ func browserLauncher(f *cmdutil.Factory) string {
121113
return os.Getenv("BROWSER")
122114
}
123115

124-
// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
125-
// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
126-
// PATH, return the absolute location to the program.
127-
//
128-
// The idea is that the result of this function is callable in the future and refers to the same
129-
// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
130-
// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
131-
// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
132-
// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
133-
// location.
134-
//
135-
// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
136-
// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
137-
// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
138-
// auth login`, running `brew update` will print out authentication errors as git is unable to locate
139-
// Homebrew-installed `gh`.
140-
func executable(fallbackName string) string {
141-
exe, err := os.Executable()
142-
if err != nil {
143-
return fallbackName
144-
}
145-
146-
base := filepath.Base(exe)
147-
path := os.Getenv("PATH")
148-
for _, dir := range filepath.SplitList(path) {
149-
p, err := filepath.Abs(filepath.Join(dir, base))
150-
if err != nil {
151-
continue
152-
}
153-
f, err := os.Stat(p)
154-
if err != nil {
155-
continue
156-
}
157-
158-
if p == exe {
159-
return p
160-
} else if f.Mode()&os.ModeSymlink != 0 {
161-
if t, err := os.Readlink(p); err == nil && t == exe {
162-
return p
163-
}
164-
}
165-
}
166-
167-
return exe
168-
}
169-
170116
func configFunc() func() (config.Config, error) {
171117
var cachedConfig config.Config
172118
var configError error

‎pkg/cmdutil/factory.go‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package cmdutil
22

33
import (
44
"net/http"
5+
"os"
6+
"path/filepath"
7+
"strings"
58

69
"github.com/cli/cli/v2/context"
710
"github.com/cli/cli/v2/internal/config"
@@ -25,7 +28,59 @@ type Factory struct {
2528
Branch func() (string, error)
2629

2730
ExtensionManager extensions.ExtensionManager
31+
ExecutableName string
32+
}
33+
34+
// Executable is the path to the currently invoked binary
35+
func (f *Factory) Executable() string {
36+
if !strings.ContainsRune(f.ExecutableName, os.PathSeparator) {
37+
f.ExecutableName = executable(f.ExecutableName)
38+
}
39+
return f.ExecutableName
40+
}
41+
42+
// Finds the location of the executable for the current process as it's found in PATH, respecting symlinks.
43+
// If the process couldn't determine its location, return fallbackName. If the executable wasn't found in
44+
// PATH, return the absolute location to the program.
45+
//
46+
// The idea is that the result of this function is callable in the future and refers to the same
47+
// installation of gh, even across upgrades. This is needed primarily for Homebrew, which installs software
48+
// under a location such as `/usr/local/Cellar/gh/1.13.1/bin/gh` and symlinks it from `/usr/local/bin/gh`.
49+
// When the version is upgraded, Homebrew will often delete older versions, but keep the symlink. Because of
50+
// this, we want to refer to the `gh` binary as `/usr/local/bin/gh` and not as its internal Homebrew
51+
// location.
52+
//
53+
// None of this would be needed if we could just refer to GitHub CLI as `gh`, i.e. without using an absolute
54+
// path. However, for some reason Homebrew does not include `/usr/local/bin` in PATH when it invokes git
55+
// commands to update its taps. If `gh` (no path) is being used as git credential helper, as set up by `gh
56+
// auth login`, running `brew update` will print out authentication errors as git is unable to locate
57+
// Homebrew-installed `gh`.
58+
func executable(fallbackName string) string {
59+
exe, err := os.Executable()
60+
if err != nil {
61+
return fallbackName
62+
}
63+
64+
base := filepath.Base(exe)
65+
path := os.Getenv("PATH")
66+
for _, dir := range filepath.SplitList(path) {
67+
p, err := filepath.Abs(filepath.Join(dir, base))
68+
if err != nil {
69+
continue
70+
}
71+
f, err := os.Stat(p)
72+
if err != nil {
73+
continue
74+
}
75+
76+
if p == exe {
77+
return p
78+
} else if f.Mode()&os.ModeSymlink != 0 {
79+
if t, err := os.Readlink(p); err == nil && t == exe {
80+
return p
81+
}
82+
}
83+
}
2884

29-
// Executable is the path to the currently invoked gh binary
30-
Executable func() string
85+
return exe
3186
}

0 commit comments

Comments
 (0)