diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index 9d1d61c21..188687634 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -1,8 +1,16 @@ package extension import ( + "bytes" + "fmt" + "net/http" + "os" "path/filepath" "strings" + "sync" + + "github.com/cli/cli/v2/internal/ghrepo" + "gopkg.in/yaml.v3" ) const manifestName = "manifest.yml" @@ -12,16 +20,22 @@ type ExtensionKind int const ( GitKind ExtensionKind = iota BinaryKind + LocalKind ) type Extension struct { - path string + path string + kind ExtensionKind + gitClient gitClient + httpClient *http.Client + + mu sync.RWMutex + + // These fields get resolved dynamically: url string - isLocal bool - isPinned bool + isPinned *bool currentVersion string latestVersion string - kind ExtensionKind } func (e *Extension) Name() string { @@ -32,32 +46,157 @@ func (e *Extension) Path() string { return e.path } -func (e *Extension) URL() string { - return e.url -} - func (e *Extension) IsLocal() bool { - return e.isLocal -} - -func (e *Extension) CurrentVersion() string { - return e.currentVersion -} - -func (e *Extension) IsPinned() bool { - return e.isPinned -} - -func (e *Extension) UpdateAvailable() bool { - if e.isLocal || - e.currentVersion == "" || - e.latestVersion == "" || - e.currentVersion == e.latestVersion { - return false - } - return true + return e.kind == LocalKind } func (e *Extension) IsBinary() bool { return e.kind == BinaryKind } + +func (e *Extension) URL() string { + e.mu.RLock() + if e.url != "" { + defer e.mu.RUnlock() + return e.url + } + e.mu.RUnlock() + + var url string + switch e.kind { + case LocalKind: + case BinaryKind: + if manifest, err := e.loadManifest(); err == nil { + repo := ghrepo.NewWithHost(manifest.Owner, manifest.Name, manifest.Host) + url = ghrepo.GenerateRepoURL(repo, "") + } + case GitKind: + if remoteURL, err := e.gitClient.Config("remote.origin.url"); err == nil { + url = strings.TrimSpace(string(remoteURL)) + } + } + + e.mu.Lock() + e.url = url + e.mu.Unlock() + + return e.url +} + +func (e *Extension) CurrentVersion() string { + e.mu.RLock() + if e.currentVersion != "" { + defer e.mu.RUnlock() + return e.currentVersion + } + e.mu.RUnlock() + + var currentVersion string + switch e.kind { + case LocalKind: + case BinaryKind: + if manifest, err := e.loadManifest(); err == nil { + currentVersion = manifest.Tag + } + case GitKind: + if sha, err := e.gitClient.CommandOutput([]string{"rev-parse", "HEAD"}); err == nil { + currentVersion = string(bytes.TrimSpace(sha)) + } + } + + e.mu.Lock() + e.currentVersion = currentVersion + e.mu.Unlock() + + return e.currentVersion +} + +func (e *Extension) LatestVersion() string { + e.mu.RLock() + if e.latestVersion != "" { + defer e.mu.RUnlock() + return e.latestVersion + } + e.mu.RUnlock() + + var latestVersion string + switch e.kind { + case LocalKind: + case BinaryKind: + repo, err := ghrepo.FromFullName(e.URL()) + if err != nil { + return "" + } + release, err := fetchLatestRelease(e.httpClient, repo) + if err != nil { + return "" + } + latestVersion = release.Tag + case GitKind: + if lsRemote, err := e.gitClient.CommandOutput([]string{"ls-remote", "origin", "HEAD"}); err == nil { + latestVersion = string(bytes.SplitN(lsRemote, []byte("\t"), 2)[0]) + } + } + + e.mu.Lock() + e.latestVersion = latestVersion + e.mu.Unlock() + + return e.latestVersion +} + +func (e *Extension) IsPinned() bool { + e.mu.RLock() + if e.isPinned != nil { + defer e.mu.RUnlock() + return *e.isPinned + } + e.mu.RUnlock() + + var isPinned bool + switch e.kind { + case LocalKind: + case BinaryKind: + if manifest, err := e.loadManifest(); err == nil { + isPinned = manifest.IsPinned + } + case GitKind: + pinPath := filepath.Join(e.Path(), fmt.Sprintf(".pin-%s", e.CurrentVersion())) + if _, err := os.Stat(pinPath); err == nil { + isPinned = true + } else { + isPinned = false + } + } + + e.mu.Lock() + e.isPinned = &isPinned + e.mu.Unlock() + + return *e.isPinned +} + +func (e *Extension) UpdateAvailable() bool { + if e.IsLocal() || + e.CurrentVersion() == "" || + e.LatestVersion() == "" || + e.CurrentVersion() == e.LatestVersion() { + return false + } + return true +} + +func (e *Extension) loadManifest() (binManifest, error) { + var bm binManifest + dir, _ := filepath.Split(e.Path()) + manifestPath := filepath.Join(dir, manifestName) + manifest, err := os.ReadFile(manifestPath) + if err != nil { + return bm, fmt.Errorf("could not open %s for reading: %w", manifestPath, err) + } + err = yaml.Unmarshal(manifest, &bm) + if err != nil { + return bm, fmt.Errorf("could not parse %s: %w", manifestPath, err) + } + return bm, nil +} diff --git a/pkg/cmd/extension/extension_test.go b/pkg/cmd/extension/extension_test.go index 0a50cbfe8..9b56e0bee 100644 --- a/pkg/cmd/extension/extension_test.go +++ b/pkg/cmd/extension/extension_test.go @@ -8,7 +8,7 @@ import ( func TestUpdateAvailable_IsLocal(t *testing.T) { e := &Extension{ - isLocal: true, + kind: LocalKind, } assert.False(t, e.UpdateAvailable()) @@ -16,7 +16,7 @@ func TestUpdateAvailable_IsLocal(t *testing.T) { func TestUpdateAvailable_NoCurrentVersion(t *testing.T) { e := &Extension{ - isLocal: false, + kind: LocalKind, } assert.False(t, e.UpdateAvailable()) @@ -24,7 +24,7 @@ func TestUpdateAvailable_NoCurrentVersion(t *testing.T) { func TestUpdateAvailable_NoLatestVersion(t *testing.T) { e := &Extension{ - isLocal: false, + kind: BinaryKind, currentVersion: "1.0.0", } @@ -33,7 +33,7 @@ func TestUpdateAvailable_NoLatestVersion(t *testing.T) { func TestUpdateAvailable_CurrentVersionIsLatestVersion(t *testing.T) { e := &Extension{ - isLocal: false, + kind: BinaryKind, currentVersion: "1.0.0", latestVersion: "1.0.0", } @@ -43,7 +43,7 @@ func TestUpdateAvailable_CurrentVersionIsLatestVersion(t *testing.T) { func TestUpdateAvailable(t *testing.T) { e := &Extension{ - isLocal: false, + kind: BinaryKind, currentVersion: "1.0.0", latestVersion: "1.1.0", } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 7a76e6a47..b19175880 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -1,12 +1,10 @@ package extension import ( - "bytes" _ "embed" "errors" "fmt" "io" - "io/fs" "net/http" "os" "os/exec" @@ -83,7 +81,7 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri forwardArgs := args[1:] exts, _ := m.list(false) - var ext Extension + var ext *Extension for _, e := range exts { if e.Name() == extName { ext = e @@ -121,39 +119,53 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri func (m *Manager) List() []extensions.Extension { exts, _ := m.list(false) r := make([]extensions.Extension, len(exts)) - for i, v := range exts { - val := v - r[i] = &val + for i, ext := range exts { + r[i] = ext } return r } -func (m *Manager) list(includeMetadata bool) ([]Extension, error) { +func (m *Manager) list(includeMetadata bool) ([]*Extension, error) { dir := m.installDir() entries, err := os.ReadDir(dir) if err != nil { return nil, err } - var results []Extension + results := make([]*Extension, 0, len(entries)) for _, f := range entries { if !strings.HasPrefix(f.Name(), "gh-") { continue } - var ext Extension - var err error if f.IsDir() { - ext, err = m.parseExtensionDir(f) - if err != nil { - return nil, err + if _, err := os.Stat(filepath.Join(dir, f.Name(), manifestName)); err == nil { + results = append(results, &Extension{ + path: filepath.Join(dir, f.Name(), f.Name()), + kind: BinaryKind, + httpClient: m.client, + }) + } else { + results = append(results, &Extension{ + path: filepath.Join(dir, f.Name(), f.Name()), + kind: GitKind, + gitClient: m.gitClient.ForRepo(filepath.Join(dir, f.Name())), + }) } - results = append(results, ext) + } else if isSymlink(f.Type()) { + results = append(results, &Extension{ + path: filepath.Join(dir, f.Name(), f.Name()), + kind: LocalKind, + }) } else { - ext, err = m.parseExtensionFile(f) + // the contents of a regular file point to a local extension on disk + p, err := readPathFromFile(filepath.Join(dir, f.Name())) if err != nil { return nil, err } - results = append(results, ext) + results = append(results, &Extension{ + path: filepath.Join(p, f.Name()), + kind: LocalKind, + }) } } @@ -164,145 +176,16 @@ func (m *Manager) list(includeMetadata bool) ([]Extension, error) { return results, nil } -func (m *Manager) parseExtensionFile(fi fs.DirEntry) (Extension, error) { - ext := Extension{isLocal: true} - id := m.installDir() - exePath := filepath.Join(id, fi.Name(), fi.Name()) - if !isSymlink(fi.Type()) { - // if this is a regular file, its contents is the local directory of the extension - p, err := readPathFromFile(filepath.Join(id, fi.Name())) - if err != nil { - return ext, err - } - exePath = filepath.Join(p, fi.Name()) - } - ext.path = exePath - return ext, nil -} - -func (m *Manager) parseExtensionDir(fi fs.DirEntry) (Extension, error) { - id := m.installDir() - if _, err := os.Stat(filepath.Join(id, fi.Name(), manifestName)); err == nil { - return m.parseBinaryExtensionDir(fi) - } - - return m.parseGitExtensionDir(fi) -} - -func (m *Manager) parseBinaryExtensionDir(fi fs.DirEntry) (Extension, error) { - id := m.installDir() - exePath := filepath.Join(id, fi.Name(), fi.Name()) - ext := Extension{path: exePath, kind: BinaryKind} - manifestPath := filepath.Join(id, fi.Name(), manifestName) - manifest, err := os.ReadFile(manifestPath) - if err != nil { - return ext, fmt.Errorf("could not open %s for reading: %w", manifestPath, err) - } - var bm binManifest - err = yaml.Unmarshal(manifest, &bm) - if err != nil { - return ext, fmt.Errorf("could not parse %s: %w", manifestPath, err) - } - repo := ghrepo.NewWithHost(bm.Owner, bm.Name, bm.Host) - remoteURL := ghrepo.GenerateRepoURL(repo, "") - ext.url = remoteURL - ext.currentVersion = bm.Tag - ext.isPinned = bm.IsPinned - return ext, nil -} - -func (m *Manager) parseGitExtensionDir(fi fs.DirEntry) (Extension, error) { - id := m.installDir() - exePath := filepath.Join(id, fi.Name(), fi.Name()) - remoteUrl := m.getRemoteUrl(fi.Name()) - currentVersion := m.getCurrentVersion(fi.Name()) - - var isPinned bool - pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) - if _, err := os.Stat(pinPath); err == nil { - isPinned = true - } - - return Extension{ - path: exePath, - url: remoteUrl, - isLocal: false, - currentVersion: currentVersion, - kind: GitKind, - isPinned: isPinned, - }, nil -} - -// getCurrentVersion determines the current version for non-local git extensions. -func (m *Manager) getCurrentVersion(extension string) string { - dir := filepath.Join(m.installDir(), extension) - scopedClient := m.gitClient.ForRepo(dir) - localSha, err := scopedClient.CommandOutput([]string{"rev-parse", "HEAD"}) - if err != nil { - return "" - } - return string(bytes.TrimSpace(localSha)) -} - -// getRemoteUrl determines the remote URL for non-local git extensions. -func (m *Manager) getRemoteUrl(extension string) string { - dir := filepath.Join(m.installDir(), extension) - scopedClient := m.gitClient.ForRepo(dir) - url, err := scopedClient.Config("remote.origin.url") - if err != nil { - return "" - } - return strings.TrimSpace(string(url)) -} - -func (m *Manager) populateLatestVersions(exts []Extension) { - size := len(exts) - type result struct { - index int - version string - } - ch := make(chan result, size) +func (m *Manager) populateLatestVersions(exts []*Extension) { var wg sync.WaitGroup - wg.Add(size) - for idx, ext := range exts { - go func(i int, e Extension) { + for _, ext := range exts { + wg.Add(1) + go func(e *Extension) { defer wg.Done() - version, _ := m.getLatestVersion(e) - ch <- result{index: i, version: version} - }(idx, ext) + e.LatestVersion() + }(ext) } wg.Wait() - close(ch) - for r := range ch { - ext := &exts[r.index] - ext.latestVersion = r.version - } -} - -func (m *Manager) getLatestVersion(ext Extension) (string, error) { - if ext.isLocal { - return "", localExtensionUpgradeError - } - if ext.IsBinary() { - repo, err := ghrepo.FromFullName(ext.url) - if err != nil { - return "", err - } - r, err := fetchLatestRelease(m.client, repo) - if err != nil { - return "", err - } - return r.Tag, nil - } else { - extDir := filepath.Dir(ext.path) - scopedClient := m.gitClient.ForRepo(extDir) - lsRemote, err := scopedClient.CommandOutput([]string{"ls-remote", "origin", "HEAD"}) - if err != nil { - return "", err - } - remoteSha := bytes.SplitN(lsRemote, []byte("\t"), 2)[0] - return string(remoteSha), nil - } } func (m *Manager) InstallLocal(dir string) error { @@ -521,22 +404,23 @@ func (m *Manager) Upgrade(name string, force bool) error { if f.Name() != name { continue } - var err error - // For single extensions manually retrieve latest version since we forgo - // doing it during list. - f.latestVersion, err = m.getLatestVersion(f) - if err != nil { - return err + if f.IsLocal() { + return localExtensionUpgradeError } - return m.upgradeExtensions([]Extension{f}, force) + // For single extensions manually retrieve latest version since we forgo doing it during list. + if latestVersion := f.LatestVersion(); latestVersion == "" { + return fmt.Errorf("unable to retrieve latest version for extension %q", name) + } + return m.upgradeExtensions([]*Extension{f}, force) } return fmt.Errorf("no extension matched %q", name) } -func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { +func (m *Manager) upgradeExtensions(exts []*Extension, force bool) error { var failed bool for _, f := range exts { fmt.Fprintf(m.io.Out, "[%s]: ", f.Name()) + currentVersion := displayExtensionVersion(f, f.CurrentVersion()) err := m.upgradeExtension(f, force) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && @@ -547,8 +431,7 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { fmt.Fprintf(m.io.Out, "%s\n", err) continue } - currentVersion := displayExtensionVersion(&f, f.currentVersion) - latestVersion := displayExtensionVersion(&f, f.latestVersion) + latestVersion := displayExtensionVersion(f, f.LatestVersion()) if m.dryRunMode { fmt.Fprintf(m.io.Out, "would have upgraded from %s to %s\n", currentVersion, latestVersion) } else { @@ -561,8 +444,8 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { return nil } -func (m *Manager) upgradeExtension(ext Extension, force bool) error { - if ext.isLocal { +func (m *Manager) upgradeExtension(ext *Extension, force bool) error { + if ext.IsLocal() { return localExtensionUpgradeError } if !force && ext.IsPinned() { @@ -592,7 +475,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { return err } -func (m *Manager) upgradeGitExtension(ext Extension, force bool) error { +func (m *Manager) upgradeGitExtension(ext *Extension, force bool) error { if m.dryRunMode { return nil } @@ -611,10 +494,10 @@ func (m *Manager) upgradeGitExtension(ext Extension, force bool) error { return scopedClient.Pull("", "") } -func (m *Manager) upgradeBinExtension(ext Extension) error { - repo, err := ghrepo.FromFullName(ext.url) +func (m *Manager) upgradeBinExtension(ext *Extension) error { + repo, err := ghrepo.FromFullName(ext.URL()) if err != nil { - return fmt.Errorf("failed to parse URL %s: %w", ext.url, err) + return fmt.Errorf("failed to parse URL %s: %w", ext.URL(), err) } return m.installBin(repo, "") } diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index b3fe64567..6d9b63124 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -80,12 +80,8 @@ func TestManager_List(t *testing.T) { dirOne := filepath.Join(tempDir, "extensions", "gh-hello") dirTwo := filepath.Join(tempDir, "extensions", "gh-two") gc, gcOne, gcTwo := &mockGitClient{}, &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", dirOne).Return(gcOne).Twice() - gc.On("ForRepo", dirTwo).Return(gcTwo).Twice() - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcTwo.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() - gcTwo.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", dirOne).Return(gcOne).Once() + gc.On("ForRepo", dirTwo).Return(gcTwo).Once() m := newTestManager(tempDir, nil, gc, nil) exts := m.List() @@ -145,9 +141,7 @@ func TestManager_Dispatch(t *testing.T) { assert.NoError(t, stubExtension(extPath)) gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extDir).Return(gcOne).Twice() - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extDir).Return(gcOne).Once() m := newTestManager(tempDir, nil, gc, nil) @@ -223,9 +217,7 @@ func TestManager_Upgrade_NoMatchingExtension(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extDir).Return(gcOne).Twice() - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extDir).Return(gcOne).Once() m := newTestManager(tempDir, nil, gc, ios) err := m.Upgrade("invalid", false) assert.EqualError(t, err, `no extension matched "invalid"`) @@ -244,12 +236,8 @@ func TestManager_UpgradeExtensions(t *testing.T) { assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne, gcTwo := &mockGitClient{}, &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", dirOne).Return(gcOne).Times(4) - gc.On("ForRepo", dirTwo).Return(gcTwo).Times(4) - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcTwo.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() - gcTwo.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", dirOne).Return(gcOne).Times(3) + gc.On("ForRepo", dirTwo).Return(gcTwo).Times(3) gcOne.On("Remotes").Return(nil, nil).Once() gcTwo.On("Remotes").Return(nil, nil).Once() gcOne.On("Pull", "", "").Return(nil).Once() @@ -286,12 +274,8 @@ func TestManager_UpgradeExtensions_DryRun(t *testing.T) { assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne, gcTwo := &mockGitClient{}, &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", dirOne).Return(gcOne).Times(3) - gc.On("ForRepo", dirTwo).Return(gcTwo).Times(3) - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcTwo.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() - gcTwo.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", dirOne).Return(gcOne).Twice() + gc.On("ForRepo", dirTwo).Return(gcTwo).Twice() gcOne.On("Remotes").Return(nil, nil).Once() gcTwo.On("Remotes").Return(nil, nil).Once() m := newTestManager(tempDir, nil, gc, ios) @@ -355,9 +339,7 @@ func TestManager_UpgradeExtension_GitExtension(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extensionDir).Return(gcOne).Times(4) - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extensionDir).Return(gcOne).Times(3) gcOne.On("Remotes").Return(nil, nil).Once() gcOne.On("Pull", "", "").Return(nil).Once() m := newTestManager(tempDir, nil, gc, ios) @@ -381,9 +363,7 @@ func TestManager_UpgradeExtension_GitExtension_DryRun(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extDir).Return(gcOne).Times(3) - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extDir).Return(gcOne).Twice() gcOne.On("Remotes").Return(nil, nil).Once() m := newTestManager(tempDir, nil, gc, ios) m.EnableDryRunMode() @@ -407,9 +387,7 @@ func TestManager_UpgradeExtension_GitExtension_Force(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) ios, _, stdout, stderr := iostreams.Test() gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extensionDir).Return(gcOne).Times(4) - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extensionDir).Return(gcOne).Times(3) gcOne.On("Remotes").Return(nil, nil).Once() gcOne.On("Fetch", "origin", "HEAD").Return(nil).Once() gcOne.On("CommandOutput", []string{"reset", "--hard", "origin/HEAD"}).Return("", nil).Once() @@ -721,9 +699,7 @@ func TestManager_UpgradeExtension_GitExtension_Pinned(t *testing.T) { ios, _, _, _ := iostreams.Test() gc, gcOne := &mockGitClient{}, &mockGitClient{} - gc.On("ForRepo", extDir).Return(gcOne).Twice() - gcOne.On("Config", "remote.origin.url").Return("", nil).Once() - gcOne.On("CommandOutput", []string{"rev-parse", "HEAD"}).Return("", nil).Once() + gc.On("ForRepo", extDir).Return(gcOne).Once() m := newTestManager(tempDir, nil, gc, ios) @@ -732,7 +708,8 @@ func TestManager_UpgradeExtension_GitExtension_Pinned(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(exts)) ext := exts[0] - ext.isPinned = true + pinnedTrue := true + ext.isPinned = &pinnedTrue ext.latestVersion = "new version" err = m.upgradeExtension(ext, false) diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index 52ef6b0a4..dea637619 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -3,29 +3,15 @@ package root import ( "fmt" - "github.com/cli/cli/v2/git" - "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" ) func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command { - var short string - if ext.IsLocal() { - short = fmt.Sprintf("Local extension gh-%s", ext.Name()) - } else { - path := ext.URL() - if u, err := git.ParseURL(ext.URL()); err == nil { - if r, err := ghrepo.FromURL(u); err == nil { - path = ghrepo.FullName(r) - } - } - short = fmt.Sprintf("Extension %s", path) - } return &cobra.Command{ Use: ext.Name(), - Short: short, + Short: fmt.Sprintf("Extension %s", ext.Name()), RunE: func(c *cobra.Command, args []string) error { args = append([]string{ext.Name()}, args...) if _, err := em.Dispatch(args, io.In, io.Out, io.ErrOut); err != nil { diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 95af0ccea..2583472c0 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -20,6 +20,7 @@ type Extension interface { Path() string // Path to executable URL() string CurrentVersion() string + LatestVersion() string IsPinned() bool UpdateAvailable() bool IsBinary() bool diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index a97de8f62..500603661 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -29,6 +29,9 @@ var _ Extension = &ExtensionMock{} // IsPinnedFunc: func() bool { // panic("mock out the IsPinned method") // }, +// LatestVersionFunc: func() string { +// panic("mock out the LatestVersion method") +// }, // NameFunc: func() string { // panic("mock out the Name method") // }, @@ -60,6 +63,9 @@ type ExtensionMock struct { // IsPinnedFunc mocks the IsPinned method. IsPinnedFunc func() bool + // LatestVersionFunc mocks the LatestVersion method. + LatestVersionFunc func() string + // NameFunc mocks the Name method. NameFunc func() string @@ -86,6 +92,9 @@ type ExtensionMock struct { // IsPinned holds details about calls to the IsPinned method. IsPinned []struct { } + // LatestVersion holds details about calls to the LatestVersion method. + LatestVersion []struct { + } // Name holds details about calls to the Name method. Name []struct { } @@ -103,6 +112,7 @@ type ExtensionMock struct { lockIsBinary sync.RWMutex lockIsLocal sync.RWMutex lockIsPinned sync.RWMutex + lockLatestVersion sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex lockURL sync.RWMutex @@ -217,6 +227,33 @@ func (mock *ExtensionMock) IsPinnedCalls() []struct { return calls } +// LatestVersion calls LatestVersionFunc. +func (mock *ExtensionMock) LatestVersion() string { + if mock.LatestVersionFunc == nil { + panic("ExtensionMock.LatestVersionFunc: method is nil but Extension.LatestVersion was just called") + } + callInfo := struct { + }{} + mock.lockLatestVersion.Lock() + mock.calls.LatestVersion = append(mock.calls.LatestVersion, callInfo) + mock.lockLatestVersion.Unlock() + return mock.LatestVersionFunc() +} + +// LatestVersionCalls gets all the calls that were made to LatestVersion. +// Check the length with: +// +// len(mockedExtension.LatestVersionCalls()) +func (mock *ExtensionMock) LatestVersionCalls() []struct { +} { + var calls []struct { + } + mock.lockLatestVersion.RLock() + calls = mock.calls.LatestVersion + mock.lockLatestVersion.RUnlock() + return calls +} + // Name calls NameFunc. func (mock *ExtensionMock) Name() string { if mock.NameFunc == nil {