Avoid resolving executable() until requested at runtime

mislav · mislav · commit d853ce5bc980 · 2021-09-24T14:42:41.000+02:00
This is to avoid hitting the filesystem and resolving symlinks
unnecessarily. The value of executable is just used conditionally by a
handful of commands.
diff --git a/cmd/gh/main.go b/cmd/gh/main.go
@@ -236,7 +236,7 @@ func mainRun() exitCode {
 
 	newRelease := <-updateMessageChan
 	if newRelease != nil {
-		isHomebrew := isUnderHomebrew(cmdFactory.Executable)
+		isHomebrew := isUnderHomebrew(cmdFactory.Executable())
 		if isHomebrew && isRecentRelease(newRelease.PublishedAt) {
 			// do not notify Homebrew users before the version bump had a chance to get merged into homebrew-core
 			return exitOK
diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go
@@ -38,8 +38,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 		IO:         f.IOStreams,
 		Config:     f.Config,
 		HttpClient: f.HttpClient,
-
-		MainExecutable: f.Executable,
 	}
 
 	var tokenStdin bool
@@ -103,6 +101,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
 				}
 			}
 
+			opts.MainExecutable = f.Executable()
 			if runF != nil {
 				return runF(opts)
 			}
diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go
@@ -147,7 +147,8 @@ func Test_NewCmdLogin(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			io, stdin, _, _ := iostreams.Test()
 			f := &cmdutil.Factory{
-				IOStreams: io,
+				IOStreams:  io,
+				Executable: func() string { return "/path/to/gh" },
 			}
 
 			io.SetStdoutTTY(true)
diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go
@@ -36,7 +36,6 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
 			_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes)
 			return err
 		},
-		MainExecutable: f.Executable,
 	}
 
 	cmd := &cobra.Command{
@@ -62,6 +61,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
 				return &cmdutil.FlagError{Err: errors.New("--hostname required when not running interactively")}
 			}
 
+			opts.MainExecutable = f.Executable()
 			if runF != nil {
 				return runF(opts)
 			}
diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go
@@ -88,7 +88,8 @@ func Test_NewCmdRefresh(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			io, _, _, _ := iostreams.Test()
 			f := &cmdutil.Factory{
-				IOStreams: io,
+				IOStreams:  io,
+				Executable: func() string { return "/path/to/gh" },
 			}
 			io.SetStdinTTY(tt.tty)
 			io.SetStdoutTTY(tt.tty)
diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go
@@ -19,12 +19,17 @@ import (
 )
 
 func New(appVersion string) *cmdutil.Factory {
+	var exe string
 	f := &cmdutil.Factory{
-		Config:     configFunc(),     // No factory dependencies
-		Branch:     branchFunc(),     // No factory dependencies
-		Executable: executable("gh"), // No factory dependencies
-
-		ExtensionManager: extension.NewManager(),
+		Config: configFunc(), // No factory dependencies
+		Branch: branchFunc(), // No factory dependencies
+		Executable: func() string {
+			if exe != "" {
+				return exe
+			}
+			exe = executable("gh")
+			return exe
+		},
 	}
 
 	f.IOStreams = ioStreams(f)                   // Depends on Config
diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go
@@ -27,5 +27,5 @@ type Factory struct {
 	ExtensionManager extensions.ExtensionManager
 
 	// Executable is the path to the currently invoked gh binary
-	Executable string
+	Executable func() string
 }

Original file line number	Diff line number	Diff line change
`@@ -38,8 +38,6 @@ func NewCmdLogin(f cmdutil.Factory, runF func(LoginOptions) error) *cobra.Comm`
`38`	`38`	`IO: f.IOStreams,`
`39`	`39`	`Config: f.Config,`
`40`	`40`	`HttpClient: f.HttpClient,`
`41`		`-`
`42`		`- MainExecutable: f.Executable,`
`43`	`41`	`}`
`44`	`42`
`45`	`43`	`var tokenStdin bool`
`@@ -103,6 +101,7 @@ func NewCmdLogin(f cmdutil.Factory, runF func(LoginOptions) error) *cobra.Comm`
`103`	`101`	`}`
`104`	`102`	`}`
`105`	`103`
	`104`	`+ opts.MainExecutable = f.Executable()`
`106`	`105`	`if runF != nil {`
`107`	`106`	`return runF(opts)`
`108`	`107`	`}`
Original file line number	Diff line number	Diff line change
`@@ -147,7 +147,8 @@ func Test_NewCmdLogin(t *testing.T) {`
`147`	`147`	`t.Run(tt.name, func(t *testing.T) {`
`148`	`148`	`io, stdin, _, _ := iostreams.Test()`
`149`	`149`	`f := &cmdutil.Factory{`
`150`		`- IOStreams: io,`
	`150`	`+ IOStreams: io,`
	`151`	`+ Executable: func() string { return "/path/to/gh" },`
`151`	`152`	`}`
`152`	`153`
`153`	`154`	`io.SetStdoutTTY(true)`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,6 @@ func NewCmdRefresh(f cmdutil.Factory, runF func(RefreshOptions) error) *cobra.`
`36`	`36`	`_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes)`
`37`	`37`	`return err`
`38`	`38`	`},`
`39`		`- MainExecutable: f.Executable,`
`40`	`39`	`}`
`41`	`40`
`42`	`41`	`cmd := &cobra.Command{`
`@@ -62,6 +61,7 @@ func NewCmdRefresh(f cmdutil.Factory, runF func(RefreshOptions) error) *cobra.`
`62`	`61`	`return &cmdutil.FlagError{Err: errors.New("--hostname required when not running interactively")}`
`63`	`62`	`}`
`64`	`63`
	`64`	`+ opts.MainExecutable = f.Executable()`
`65`	`65`	`if runF != nil {`
`66`	`66`	`return runF(opts)`
`67`	`67`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,5 +27,5 @@ type Factory struct {`
`27`	`27`	`ExtensionManager extensions.ExtensionManager`
`28`	`28`
`29`	`29`	`// Executable is the path to the currently invoked gh binary`
`30`		`- Executable string`
	`30`	`+ Executable func() string`
`31`	`31`	`}`