Bring extension update check in line with gh check

This commit is a bit of refactoring to bring the extension update checking logic up to par with what is done with `gh` including creation of state file per extension and listening to env vars for disabling version checking.

This work is not complete as it does not address necessary test changes.

internal/ghcmd/cmd.go

@@ -25,7 +25,6 @@ import (
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/utils"
 	"github.com/cli/safeexec"
-	"github.com/mattn/go-isatty"
 	"github.com/mgutz/ansi"
 	"github.com/spf13/cobra"
 )
@@ -216,29 +215,8 @@ func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) {
 	}
 }
 
-func shouldCheckForUpdate() bool {
-	if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" {
-		return false
-	}
-	if os.Getenv("CODESPACES") != "" {
-		return false
-	}
-	return updaterEnabled != "" && !isCI() && isTerminal(os.Stdout) && isTerminal(os.Stderr)
-}
-
-func isTerminal(f *os.File) bool {
-	return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
-}
-
-// based on https://github.com/watson/ci-info/blob/HEAD/index.js
-func isCI() bool {
-	return os.Getenv("CI") != "" || // GitHub Actions, Travis CI, CircleCI, Cirrus CI, GitLab CI, AppVeyor, CodeShip, dsari
-		os.Getenv("BUILD_NUMBER") != "" || // Jenkins, TeamCity
-		os.Getenv("RUN_ID") != "" // TaskCluster, dsari
-}
-
 func checkForUpdate(ctx context.Context, f *cmdutil.Factory, currentVersion string) (*update.ReleaseInfo, error) {
-	if !shouldCheckForUpdate() {
+	if !update.ShouldCheckForUpdate(updaterEnabled) {
 		return nil, nil
 	}
 	httpClient, err := f.HttpClient()

internal/update/update.go

@@ -13,7 +13,9 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cli/cli/v2/pkg/extensions"
 	"github.com/hashicorp/go-version"
+	"github.com/mattn/go-isatty"
 	"gopkg.in/yaml.v3"
 )
 
@@ -31,6 +33,49 @@ type StateEntry struct {
 	LatestRelease      ReleaseInfo `yaml:"latest_release"`
 }
 
+func ShouldCheckForExtensionUpdate() bool {
+	if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" {
+		return false
+	}
+	if os.Getenv("CODESPACES") != "" {
+		return false
+	}
+	return !IsCI() && IsTerminal(os.Stdout) && IsTerminal(os.Stderr)
+}
+
+func CheckForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Extension, stateFilePath string) (*ReleaseInfo, error) {
+	stateEntry, _ := getStateEntry(stateFilePath)
+	if stateEntry != nil && time.Since(stateEntry.CheckedForUpdateAt).Hours() < 24 {
+		return nil, nil
+	}
+
+	releaseInfo := &ReleaseInfo{
+		Version: ext.LatestVersion(),
+		URL:     ext.URL(),
+	}
+
+	err := setStateEntry(stateFilePath, time.Now(), *releaseInfo)
+	if err != nil {
+		return nil, err
+	}
+
+	if ext.UpdateAvailable() {
+		return releaseInfo, nil
+	}
+
+	return nil, nil
+}
+
+func ShouldCheckForUpdate(updaterEnabled string) bool {
+	if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" {
+		return false
+	}
+	if os.Getenv("CODESPACES") != "" {
+		return false
+	}
+	return updaterEnabled != "" && !IsCI() && IsTerminal(os.Stdout) && IsTerminal(os.Stderr)
+}
+
 // CheckForUpdate checks whether this software has had a newer release on GitHub
 func CheckForUpdate(ctx context.Context, client *http.Client, stateFilePath, repo, currentVersion string) (*ReleaseInfo, error) {
 	stateEntry, _ := getStateEntry(stateFilePath)
@@ -122,3 +167,14 @@ func versionGreaterThan(v, w string) bool {
 
 	return ve == nil && we == nil && vv.GreaterThan(vw)
 }
+
+func IsTerminal(f *os.File) bool {
+	return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd())
+}
+
+// based on https://github.com/watson/ci-info/blob/HEAD/index.js
+func IsCI() bool {
+	return os.Getenv("CI") != "" || // GitHub Actions, Travis CI, CircleCI, Cirrus CI, GitLab CI, AppVeyor, CodeShip, dsari
+		os.Getenv("BUILD_NUMBER") != "" || // Jenkins, TeamCity
+		os.Getenv("RUN_ID") != "" // TaskCluster, dsari
+}

pkg/cmd/extension/extension.go

@@ -41,7 +41,11 @@ type Extension struct {
 }
 
 func (e *Extension) Name() string {
-	return strings.TrimPrefix(filepath.Base(e.path), "gh-")
+	return strings.TrimPrefix(e.FullName(), "gh-")
+}
+
+func (e *Extension) FullName() string {
+	return filepath.Base(e.path)
 }
 
 func (e *Extension) Path() string {

pkg/cmd/root/extension.go

@@ -4,11 +4,14 @@ import (
 	"errors"
 	"fmt"
 	"os/exec"
+	"path/filepath"
 	"strings"
-	"time"
 
+	"github.com/cli/cli/v2/internal/config"
+	"github.com/cli/cli/v2/internal/update"
 	"github.com/cli/cli/v2/pkg/extensions"
 	"github.com/cli/cli/v2/pkg/iostreams"
+	"github.com/cli/cli/v2/utils"
 	"github.com/spf13/cobra"
 )
 
@@ -16,16 +19,10 @@ type ExternalCommandExitError struct {
 	*exec.ExitError
 }
 
-type extensionReleaseInfo struct {
-	CurrentVersion string
-	LatestVersion  string
-	Pinned         bool
-	URL            string
-}
-
 func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command {
-	updateMessageChan := make(chan *extensionReleaseInfo)
+	updateMessageChan := make(chan *update.ReleaseInfo)
 	cs := io.ColorScheme()
+	hasDebug, _ := utils.IsDebugEnabled()
 
 	return &cobra.Command{
 		Use:   ext.Name(),
@@ -33,14 +30,11 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 		// PreRun handles looking up whether extension has a latest version only when the command is ran.
 		PreRun: func(c *cobra.Command, args []string) {
 			go func() {
-				if ext.UpdateAvailable() {
-					updateMessageChan <- &extensionReleaseInfo{
-						CurrentVersion: ext.CurrentVersion(),
-						LatestVersion:  ext.LatestVersion(),
-						Pinned:         ext.IsPinned(),
-						URL:            ext.URL(),
-					}
+				rel, err := checkForExtensionUpdate(em, ext)
+				if err != nil && hasDebug {
+					fmt.Fprintf(io.ErrOut, "warning: checking for update failed: %v", err)
 				}
+				updateMessageChan <- rel
 			}()
 		},
 		RunE: func(c *cobra.Command, args []string) error {
@@ -62,9 +56,9 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 					stderr := io.ErrOut
 					fmt.Fprintf(stderr, "\n\n%s %s → %s\n",
 						cs.Yellowf("A new release of %s is available:", ext.Name()),
-						cs.Cyan(strings.TrimPrefix(releaseInfo.CurrentVersion, "v")),
-						cs.Cyan(strings.TrimPrefix(releaseInfo.LatestVersion, "v")))
-					if releaseInfo.Pinned {
+						cs.Cyan(strings.TrimPrefix(ext.CurrentVersion(), "v")),
+						cs.Cyan(strings.TrimPrefix(releaseInfo.Version, "v")))
+					if ext.IsPinned() {
 						fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name())
 					} else {
 						fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name())
@@ -72,7 +66,7 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 					fmt.Fprintf(stderr, "%s\n\n",
 						cs.Yellow(releaseInfo.URL))
 				}
-			case <-time.After(1 * time.Second):
+			default:
 				// Bail on checking for new extension update as its taking too long
 			}
 		},
@@ -83,3 +77,12 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 		DisableFlagParsing: true,
 	}
 }
+
+func checkForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Extension) (*update.ReleaseInfo, error) {
+	if !update.ShouldCheckForExtensionUpdate() {
+		return nil, nil
+	}
+
+	stateFilePath := filepath.Join(config.StateDir(), "extensions", ext.FullName(), "state.yml")
+	return update.CheckForExtensionUpdate(em, ext, stateFilePath)
+}

pkg/extensions/extension.go

@@ -16,8 +16,9 @@ const (
 
 //go:generate moq -rm -out extension_mock.go . Extension
 type Extension interface {
-	Name() string // Extension Name without gh-
-	Path() string // Path to executable
+	Name() string     // Extension Name without gh-
+	FullName() string // Extension Name with gh-
+	Path() string     // Path to executable
 	URL() string
 	CurrentVersion() string
 	LatestVersion() string

pkg/extensions/extension_mock.go

@@ -20,6 +20,9 @@ var _ Extension = &ExtensionMock{}
 //			CurrentVersionFunc: func() string {
 //				panic("mock out the CurrentVersion method")
 //			},
+//			FullNameFunc: func() string {
+//				panic("mock out the FullName method")
+//			},
 //			IsBinaryFunc: func() bool {
 //				panic("mock out the IsBinary method")
 //			},
@@ -57,6 +60,9 @@ type ExtensionMock struct {
 	// CurrentVersionFunc mocks the CurrentVersion method.
 	CurrentVersionFunc func() string
 
+	// FullNameFunc mocks the FullName method.
+	FullNameFunc func() string
+
 	// IsBinaryFunc mocks the IsBinary method.
 	IsBinaryFunc func() bool
 
@@ -89,6 +95,9 @@ type ExtensionMock struct {
 		// CurrentVersion holds details about calls to the CurrentVersion method.
 		CurrentVersion []struct {
 		}
+		// FullName holds details about calls to the FullName method.
+		FullName []struct {
+		}
 		// IsBinary holds details about calls to the IsBinary method.
 		IsBinary []struct {
 		}
@@ -118,6 +127,7 @@ type ExtensionMock struct {
 		}
 	}
 	lockCurrentVersion  sync.RWMutex
+	lockFullName        sync.RWMutex
 	lockIsBinary        sync.RWMutex
 	lockIsLocal         sync.RWMutex
 	lockIsPinned        sync.RWMutex
@@ -156,6 +166,33 @@ func (mock *ExtensionMock) CurrentVersionCalls() []struct {
 	return calls
 }
 
+// FullName calls FullNameFunc.
+func (mock *ExtensionMock) FullName() string {
+	if mock.FullNameFunc == nil {
+		panic("ExtensionMock.FullNameFunc: method is nil but Extension.FullName was just called")
+	}
+	callInfo := struct {
+	}{}
+	mock.lockFullName.Lock()
+	mock.calls.FullName = append(mock.calls.FullName, callInfo)
+	mock.lockFullName.Unlock()
+	return mock.FullNameFunc()
+}
+
+// FullNameCalls gets all the calls that were made to FullName.
+// Check the length with:
+//
+//	len(mockedExtension.FullNameCalls())
+func (mock *ExtensionMock) FullNameCalls() []struct {
+} {
+	var calls []struct {
+	}
+	mock.lockFullName.RLock()
+	calls = mock.calls.FullName
+	mock.lockFullName.RUnlock()
+	return calls
+}
+
 // IsBinary calls IsBinaryFunc.
 func (mock *ExtensionMock) IsBinary() bool {
 	if mock.IsBinaryFunc == nil {