Add extension upgrade --dry-run

cmd/gh/main.go

@@ -176,7 +176,7 @@ func mainRun() exitCode {
 				}
 			}
 		}
-		for _, ext := range cmdFactory.ExtensionManager.List(false) {
+		for _, ext := range cmdFactory.ExtensionManager.List() {
 			if strings.HasPrefix(ext.Name(), toComplete) {
 				results = append(results, ext.Name())
 			}

pkg/cmd/alias/set/set.go

@@ -82,7 +82,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command
 					return true
 				}
 
-				for _, ext := range f.ExtensionManager.List(false) {
+				for _, ext := range f.ExtensionManager.List() {
 					if ext.Name() == split[0] {
 						return true
 					}

pkg/cmd/alias/set/set_test.go

@@ -30,7 +30,7 @@ func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.Cmd
 			return cfg, nil
 		},
 		ExtensionManager: &extensions.ExtensionManagerMock{
-			ListFunc: func(bool) []extensions.Extension {
+			ListFunc: func() []extensions.Extension {
 				return []extensions.Extension{}
 			},
 		},

pkg/cmd/extension/command.go

@@ -45,7 +45,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 			Aliases: []string{"ls"},
 			Args:    cobra.NoArgs,
 			RunE: func(cmd *cobra.Command, args []string) error {
-				cmds := m.List(true)
+				cmds := m.List()
 				if len(cmds) == 0 {
 					return errors.New("no extensions installed")
 				}
@@ -61,22 +61,13 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 
 					t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil)
 					t.AddField(repo, nil, nil)
-					version := c.CurrentVersion()
-					if !c.IsBinary() && len(version) > 8 {
-						version = version[:8]
-					}
-
+					version := displayExtensionVersion(c, c.CurrentVersion())
 					if c.IsPinned() {
 						t.AddField(version, nil, cs.Cyan)
 					} else {
 						t.AddField(version, nil, nil)
 					}
 
-					var updateAvailable string
-					if c.UpdateAvailable() {
-						updateAvailable = "Upgrade available"
-					}
-					t.AddField(updateAvailable, nil, cs.Green)
 					t.EndRow()
 				}
 				return t.Render()
@@ -152,6 +143,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 		func() *cobra.Command {
 			var flagAll bool
 			var flagForce bool
+			var flagDryRun bool
 			cmd := &cobra.Command{
 				Use:   "upgrade {<name> | --all}",
 				Short: "Upgrade installed extensions",
@@ -172,6 +164,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 					if len(args) > 0 {
 						name = normalizeExtensionSelector(args[0])
 					}
+					if flagDryRun {
+						m.EnableDryRunMode()
+					}
 					cs := io.ColorScheme()
 					err := m.Upgrade(name, flagForce)
 					if err != nil && !errors.Is(err, upToDateError) {
@@ -186,19 +181,24 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
 						return cmdutil.SilentError
 					}
 					if io.IsStdoutTTY() {
+						successStr := "Successfully"
+						if flagDryRun {
+							successStr = "Would have"
+						}
 						if errors.Is(err, upToDateError) {
 							fmt.Fprintf(io.Out, "%s Extension already up to date\n", cs.SuccessIcon())
 						} else if name != "" {
-							fmt.Fprintf(io.Out, "%s Successfully upgraded extension %s\n", cs.SuccessIcon(), name)
+							fmt.Fprintf(io.Out, "%s %s upgraded extension %s\n", cs.SuccessIcon(), successStr, name)
 						} else {
-							fmt.Fprintf(io.Out, "%s Successfully upgraded extensions\n", cs.SuccessIcon())
+							fmt.Fprintf(io.Out, "%s %s upgraded extensions\n", cs.SuccessIcon(), successStr)
 						}
 					}
 					return nil
 				},
 			}
 			cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions")
 			cmd.Flags().BoolVar(&flagForce, "force", false, "Force upgrade extension")
+			cmd.Flags().BoolVar(&flagDryRun, "dry-run", false, "Only display upgrades")
 			return cmd
 		}(),
 		&cobra.Command{
@@ -355,7 +355,7 @@ func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager,
 		return fmt.Errorf("%q matches the name of a built-in command", commandName)
 	}
 
-	for _, ext := range m.List(false) {
+	for _, ext := range m.List() {
 		if ext.Name() == commandName {
 			return fmt.Errorf("there is already an installed extension that provides the %q command", commandName)
 		}
@@ -370,3 +370,10 @@ func normalizeExtensionSelector(n string) string {
 	}
 	return strings.TrimPrefix(n, "gh-")
 }
+
+func displayExtensionVersion(ext extensions.Extension, version string) string {
+	if !ext.IsBinary() && len(version) > 8 {
+		return version[:8]
+	}
+	return version
+}

pkg/cmd/extension/command_test.go

@@ -41,7 +41,7 @@ func TestNewCmdExtension(t *testing.T) {
 			name: "install an extension",
 			args: []string{"install", "owner/gh-some-ext"},
 			managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
-				em.ListFunc = func(bool) []extensions.Extension {
+				em.ListFunc = func() []extensions.Extension {
 					return []extensions.Extension{}
 				}
 				em.InstallFunc = func(_ ghrepo.Interface, _ string) error {
@@ -60,7 +60,7 @@ func TestNewCmdExtension(t *testing.T) {
 			name: "install an extension with same name as existing extension",
 			args: []string{"install", "owner/gh-existing-ext"},
 			managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
-				em.ListFunc = func(bool) []extensions.Extension {
+				em.ListFunc = func() []extensions.Extension {
 					e := &Extension{path: "owner2/gh-existing-ext"}
 					return []extensions.Extension{e}
 				}
@@ -99,6 +99,12 @@ func TestNewCmdExtension(t *testing.T) {
 			wantErr: true,
 			errMsg:  "specify an extension to upgrade or `--all`",
 		},
+		{
+			name:    "upgrade --all with extension name error",
+			args:    []string{"upgrade", "test", "--all"},
+			wantErr: true,
+			errMsg:  "cannot use `--all` with extension name",
+		},
 		{
 			name: "upgrade an extension",
 			args: []string{"upgrade", "hello"},
@@ -115,6 +121,26 @@ func TestNewCmdExtension(t *testing.T) {
 			isTTY:      true,
 			wantStdout: "✓ Successfully upgraded extension hello\n",
 		},
+		{
+			name: "upgrade an extension dry run",
+			args: []string{"upgrade", "hello", "--dry-run"},
+			managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
+				em.EnableDryRunModeFunc = func() {}
+				em.UpgradeFunc = func(name string, force bool) error {
+					return nil
+				}
+				return func(t *testing.T) {
+					dryRunCalls := em.EnableDryRunModeCalls()
+					assert.Equal(t, 1, len(dryRunCalls))
+					upgradeCalls := em.UpgradeCalls()
+					assert.Equal(t, 1, len(upgradeCalls))
+					assert.Equal(t, "hello", upgradeCalls[0].Name)
+					assert.False(t, upgradeCalls[0].Force)
+				}
+			},
+			isTTY:      true,
+			wantStdout: "✓ Would have upgraded extension hello\n",
+		},
 		{
 			name: "upgrade an extension notty",
 			args: []string{"upgrade", "hello"},
@@ -214,6 +240,26 @@ func TestNewCmdExtension(t *testing.T) {
 			isTTY:      true,
 			wantStdout: "✓ Successfully upgraded extensions\n",
 		},
+		{
+			name: "upgrade all dry run",
+			args: []string{"upgrade", "--all", "--dry-run"},
+			managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
+				em.EnableDryRunModeFunc = func() {}
+				em.UpgradeFunc = func(name string, force bool) error {
+					return nil
+				}
+				return func(t *testing.T) {
+					dryRunCalls := em.EnableDryRunModeCalls()
+					assert.Equal(t, 1, len(dryRunCalls))
+					upgradeCalls := em.UpgradeCalls()
+					assert.Equal(t, 1, len(upgradeCalls))
+					assert.Equal(t, "", upgradeCalls[0].Name)
+					assert.False(t, upgradeCalls[0].Force)
+				}
+			},
+			isTTY:      true,
+			wantStdout: "✓ Would have upgraded extensions\n",
+		},
 		{
 			name: "upgrade all none installed",
 			args: []string{"upgrade", "--all"},
@@ -313,16 +359,17 @@ func TestNewCmdExtension(t *testing.T) {
 			name: "list extensions",
 			args: []string{"list"},
 			managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
-				em.ListFunc = func(bool) []extensions.Extension {
-					ex1 := &Extension{path: "cli/gh-test", url: "https://github.com/cli/gh-test", currentVersion: "1", latestVersion: "1"}
-					ex2 := &Extension{path: "cli/gh-test2", url: "https://github.com/cli/gh-test2", currentVersion: "1", latestVersion: "2"}
+				em.ListFunc = func() []extensions.Extension {
+					ex1 := &Extension{path: "cli/gh-test", url: "https://github.com/cli/gh-test", currentVersion: "1"}
+					ex2 := &Extension{path: "cli/gh-test2", url: "https://github.com/cli/gh-test2", currentVersion: "1"}
 					return []extensions.Extension{ex1, ex2}
 				}
 				return func(t *testing.T) {
-					assert.Equal(t, 1, len(em.ListCalls()))
+					calls := em.ListCalls()
+					assert.Equal(t, 1, len(calls))
 				}
 			},
-			wantStdout: "gh test\tcli/gh-test\t1\t\ngh test2\tcli/gh-test2\t1\tUpgrade available\n",
+			wantStdout: "gh test\tcli/gh-test\t1\ngh test2\tcli/gh-test2\t1\n",
 		},
 		{
 			name: "create extension interactive",
@@ -533,7 +580,7 @@ func Test_checkValidExtension(t *testing.T) {
 	rootCmd.AddCommand(&cobra.Command{Use: "auth"})
 
 	m := &extensions.ExtensionManagerMock{
-		ListFunc: func(bool) []extensions.Extension {
+		ListFunc: func() []extensions.Extension {
 			return []extensions.Extension{
 				&extensions.ExtensionMock{
 					NameFunc: func() string { return "screensaver" },

pkg/cmd/extension/manager.go

@@ -37,6 +37,7 @@ type Manager struct {
 	client     *http.Client
 	config     config.Config
 	io         *iostreams.IOStreams
+	dryRunMode bool
 }
 
 func NewManager(io *iostreams.IOStreams) *Manager {
@@ -64,6 +65,10 @@ func (m *Manager) SetClient(client *http.Client) {
 	m.client = client
 }
 
+func (m *Manager) EnableDryRunMode() {
+	m.dryRunMode = true
+}
+
 func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) {
 	if len(args) == 0 {
 		return false, errors.New("too few arguments in list")
@@ -109,8 +114,8 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri
 	return true, externalCmd.Run()
 }
 
-func (m *Manager) List(includeMetadata bool) []extensions.Extension {
-	exts, _ := m.list(includeMetadata)
+func (m *Manager) List() []extensions.Extension {
+	exts, _ := m.list(false)
 	r := make([]extensions.Extension, len(exts))
 	for i, v := range exts {
 		val := v
@@ -384,17 +389,21 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error {
 	targetDir := filepath.Join(m.installDir(), name)
 
 	// TODO clean this up if function errs?
-	err = os.MkdirAll(targetDir, 0755)
-	if err != nil {
-		return fmt.Errorf("failed to create installation directory: %w", err)
+	if !m.dryRunMode {
+		err = os.MkdirAll(targetDir, 0755)
+		if err != nil {
+			return fmt.Errorf("failed to create installation directory: %w", err)
+		}
 	}
 
 	binPath := filepath.Join(targetDir, name)
 	binPath += ext
 
-	err = downloadAsset(m.client, *asset, binPath)
-	if err != nil {
-		return fmt.Errorf("failed to download asset %s: %w", asset.Name, err)
+	if !m.dryRunMode {
+		err = downloadAsset(m.client, *asset, binPath)
+		if err != nil {
+			return fmt.Errorf("failed to download asset %s: %w", asset.Name, err)
+		}
 	}
 
 	manifest := binManifest{
@@ -411,17 +420,19 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error {
 		return fmt.Errorf("failed to serialize manifest: %w", err)
 	}
 
-	manifestPath := filepath.Join(targetDir, manifestName)
+	if !m.dryRunMode {
+		manifestPath := filepath.Join(targetDir, manifestName)
 
-	f, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
-	if err != nil {
-		return fmt.Errorf("failed to open manifest for writing: %w", err)
-	}
-	defer f.Close()
+		f, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+		if err != nil {
+			return fmt.Errorf("failed to open manifest for writing: %w", err)
+		}
+		defer f.Close()
 
-	_, err = f.Write(bs)
-	if err != nil {
-		return fmt.Errorf("failed write manifest file: %w", err)
+		_, err = f.Write(bs)
+		if err != nil {
+			return fmt.Errorf("failed write manifest file: %w", err)
+		}
 	}
 
 	return nil
@@ -519,7 +530,13 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error {
 			fmt.Fprintf(m.io.Out, "%s\n", err)
 			continue
 		}
-		fmt.Fprintf(m.io.Out, "upgrade complete\n")
+		currentVersion := displayExtensionVersion(&f, f.currentVersion)
+		latestVersion := displayExtensionVersion(&f, f.latestVersion)
+		if m.dryRunMode {
+			fmt.Fprintf(m.io.Out, "would have upgraded from %s to %s\n", currentVersion, latestVersion)
+		} else {
+			fmt.Fprintf(m.io.Out, "upgraded from %s to %s\n", currentVersion, latestVersion)
+		}
 	}
 	if failed {
 		return errors.New("some extensions failed to upgrade")
@@ -548,8 +565,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error {
 			isBin, _ = isBinExtension(m.client, repo)
 		}
 		if isBin {
-			err = m.Remove(ext.Name())
-			if err != nil {
+			if err := m.Remove(ext.Name()); err != nil {
 				return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err)
 			}
 			return m.installBin(repo, "")
@@ -565,6 +581,9 @@ func (m *Manager) upgradeGitExtension(ext Extension, force bool) error {
 		return err
 	}
 	dir := filepath.Dir(ext.path)
+	if m.dryRunMode {
+		return nil
+	}
 	if force {
 		if err := m.newCommand(exe, "-C", dir, "fetch", "origin", "HEAD").Run(); err != nil {
 			return err
@@ -587,6 +606,9 @@ func (m *Manager) Remove(name string) error {
 	if _, err := os.Lstat(targetDir); os.IsNotExist(err) {
 		return fmt.Errorf("no extension found: %q", targetDir)
 	}
+	if m.dryRunMode {
+		return nil
+	}
 	return os.RemoveAll(targetDir)
 }