Implement partial feedback before trunk update

internal/ghcmd/cmd.go

@@ -29,6 +29,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// updaterEnabled is a statically linked build property set in gh formula within homebrew/homebrew-core
+// used to control whether users are notified of newer GitHub CLI releases.
+// Development builds leave this empty as impossible to determine if newer or not.
+// For more information, <https://github.com/Homebrew/homebrew-core/blob/master/Formula/g/gh.rb>.
 var updaterEnabled = ""
 
 type exitCode int
@@ -216,7 +220,7 @@ func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) {
 }
 
 func checkForUpdate(ctx context.Context, f *cmdutil.Factory, currentVersion string) (*update.ReleaseInfo, error) {
-	if !update.ShouldCheckForUpdate(updaterEnabled) {
+	if updaterEnabled == "" || !update.ShouldCheckForUpdate() {
 		return nil, nil
 	}
 	httpClient, err := f.HttpClient()

internal/update/update.go

@@ -33,7 +33,7 @@ type StateEntry struct {
 	LatestRelease      ReleaseInfo `yaml:"latest_release"`
 }
 
-func ShouldCheckForExtensionUpdate() bool {
+func ShouldCheckForUpdate() bool {
 	if os.Getenv("GH_NO_UPDATE_NOTIFIER") != "" {
 		return false
 	}
@@ -44,6 +44,7 @@ func ShouldCheckForExtensionUpdate() bool {
 }
 
 func CheckForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Extension, now time.Time) (*ReleaseInfo, error) {
+	// local extensions cannot have updates, so avoid work that ultimately returns nothing.
 	if ext.IsLocal() {
 		return nil, nil
 	}
@@ -71,16 +72,6 @@ func CheckForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Exte
 	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)

pkg/cmd/extension/manager.go

@@ -542,7 +542,7 @@ func (m *Manager) upgradeBinExtension(ext *Extension) error {
 }
 
 func (m *Manager) Remove(name string) error {
-	name = ensurePrefixed(name)
+	name = normalizeExtension(name)
 	targetDir := filepath.Join(m.installDir(), name)
 	if _, err := os.Lstat(targetDir); os.IsNotExist(err) {
 		return fmt.Errorf("no extension found: %q", targetDir)
@@ -562,7 +562,7 @@ func (m *Manager) installDir() string {
 
 // UpdateDir returns the extension-specific directory where updates are stored.
 func (m *Manager) UpdateDir(name string) string {
-	return filepath.Join(m.updateDir(), ensurePrefixed(name))
+	return filepath.Join(m.updateDir(), normalizeExtension(name))
 }
 
 //go:embed ext_tmpls/goBinMain.go.txt
@@ -826,20 +826,23 @@ func codesignBinary(binPath string) error {
 	return cmd.Run()
 }
 
-// cleanExtensionUpdateDir should be used before installing extensions to avoid past metadata creating problems.
+// cleanExtensionUpdateDir deletes extension state directory to avoid problems reinstalling extensions.
 func (m *Manager) cleanExtensionUpdateDir(name string) error {
 	updatePath := m.UpdateDir(name)
-	if _, err := os.Stat(updatePath); err == nil {
-		if err := os.RemoveAll(updatePath); err != nil {
-			return fmt.Errorf("failed to remove previous extension update state: %w", err)
+	if _, err := os.Stat(updatePath); err != nil {
+		if os.IsNotExist(err) {
+			return nil
 		}
+		return fmt.Errorf("failed to check previous extension update state: %w", err)
+	}
+	if err := os.RemoveAll(updatePath); err != nil {
+		return fmt.Errorf("failed to remove previous extension update state: %w", err)
 	}
-
 	return nil
 }
 
-// ensurePrefixed just makes sure that the provided extension name is prefixed with "gh-".
-func ensurePrefixed(name string) string {
+// normalizeExtension makes sure that the provided extension name is prefixed with "gh-".
+func normalizeExtension(name string) string {
 	if !strings.HasPrefix(name, "gh-") {
 		name = "gh-" + name
 	}

pkg/cmd/extension/manager_test.go

@@ -1311,7 +1311,7 @@ func Test_ensurePrefixed(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			require.Equal(t, tt.expected, ensurePrefixed(tt.input))
+			require.Equal(t, tt.expected, normalizeExtension(tt.input))
 		})
 	}
 }

pkg/cmd/root/extension.go

@@ -18,12 +18,12 @@ type ExternalCommandExitError struct {
 	*exec.ExitError
 }
 
-func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension, checkFunc func(extensions.ExtensionManager, extensions.Extension) (*update.ReleaseInfo, error)) *cobra.Command {
+func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension, checkExtensionReleaseInfo func(extensions.ExtensionManager, extensions.Extension) (*update.ReleaseInfo, error)) *cobra.Command {
 	updateMessageChan := make(chan *update.ReleaseInfo)
 	cs := io.ColorScheme()
 	hasDebug, _ := utils.IsDebugEnabled()
-	if checkFunc == nil {
-		checkFunc = checkForExtensionUpdate
+	if checkExtensionReleaseInfo == nil {
+		checkExtensionReleaseInfo = checkForExtensionUpdate
 	}
 
 	return &cobra.Command{
@@ -32,11 +32,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() {
-				rel, err := checkFunc(em, ext)
+				releaseInfo, err := checkExtensionReleaseInfo(em, ext)
 				if err != nil && hasDebug {
 					fmt.Fprintf(io.ErrOut, "warning: checking for update failed: %v", err)
 				}
-				updateMessageChan <- rel
+				updateMessageChan <- releaseInfo
 			}()
 		},
 		RunE: func(c *cobra.Command, args []string) error {
@@ -77,7 +77,7 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
 }
 
 func checkForExtensionUpdate(em extensions.ExtensionManager, ext extensions.Extension) (*update.ReleaseInfo, error) {
-	if !update.ShouldCheckForExtensionUpdate() {
+	if !update.ShouldCheckForUpdate() {
 		return nil, nil
 	}