diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index cb76f422087..46991e726a1 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -111,7 +111,7 @@ func (e *em) InstallLocal(_ string) error { return nil } -func (e *em) Upgrade(_ string, _ bool) error { +func (e *em) Upgrade(_ string, _ extensions.UpgradeOptions) error { return nil } diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 6ec516533d0..ff283f82ab3 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -59,9 +59,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { Aliases: []string{"extensions", "ext"}, } - upgradeFunc := func(name string, flagForce bool) error { + upgradeFunc := func(name string, opts extensions.UpgradeOptions) error { cs := io.ColorScheme() - err := m.Upgrade(name, flagForce) + err := m.Upgrade(name, opts) if err != nil { if name != "" { fmt.Fprintf(io.ErrOut, "%s Failed upgrading extension %s: %s\n", cs.FailureIcon(), name, err) @@ -379,7 +379,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if ext, err := checkValidExtension(cmd.Root(), m, repo.RepoName(), repo.RepoOwner()); err != nil { // If an existing extension was found and --force was specified, attempt to upgrade. if forceFlag && ext != nil { - return upgradeFunc(ext.Name(), forceFlag) + return upgradeFunc(ext.Name(), extensions.UpgradeOptions{Force: forceFlag}) } if errors.Is(err, alreadyInstalledError) { @@ -426,6 +426,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { var flagAll bool var flagForce bool var flagDryRun bool + var flagLatestPreRelease bool + var flagPin string + cmd := &cobra.Command{ Use: "upgrade { | --all}", Short: "Upgrade installed extensions", @@ -439,6 +442,15 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if len(args) > 1 { return cmdutil.FlagErrorf("too many arguments") } + if flagLatestPreRelease && flagAll { + return cmdutil.FlagErrorf("cannot use `--latest-pre-release` with `--all`") + } + if flagPin != "" && flagAll { + return cmdutil.FlagErrorf("cannot use `--pin` with `--all`") + } + if flagPin != "" && flagLatestPreRelease { + return cmdutil.FlagErrorf("cannot use `--pin` with `--latest-pre-release`") + } return nil }, RunE: func(cmd *cobra.Command, args []string) error { @@ -449,12 +461,18 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if flagDryRun { m.EnableDryRunMode() } - return upgradeFunc(name, flagForce) + return upgradeFunc(name, extensions.UpgradeOptions{ + Force: flagForce, + LatestPreRelease: flagLatestPreRelease, + PinVersion: flagPin, + }) }, } 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") + cmd.Flags().BoolVar(&flagLatestPreRelease, "latest-pre-release", false, "Upgrade to the latest release, including pre-releases (binary extensions only)") + cmd.Flags().StringVar(&flagPin, "pin", "", "Upgrade to and pin a specific release tag (binary extensions only)") return cmd }(), &cobra.Command{ diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 7001c8f1a7a..4ffc2192e76 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -365,7 +365,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension", args: []string{"upgrade", "hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -382,7 +382,7 @@ func TestNewCmdExtension(t *testing.T) { 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 { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -391,7 +391,7 @@ func TestNewCmdExtension(t *testing.T) { upgradeCalls := em.UpgradeCalls() assert.Equal(t, 1, len(upgradeCalls)) assert.Equal(t, "hello", upgradeCalls[0].Name) - assert.False(t, upgradeCalls[0].Force) + assert.False(t, upgradeCalls[0].Opts.Force) } }, isTTY: true, @@ -401,7 +401,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension notty", args: []string{"upgrade", "hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -412,11 +412,64 @@ func TestNewCmdExtension(t *testing.T) { }, isTTY: false, }, + { + name: "upgrade an extension to the latest pre-release", + args: []string{"upgrade", "hello", "--latest-pre-release"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + assert.True(t, calls[0].Opts.LatestPreRelease) + assert.Equal(t, "", calls[0].Opts.PinVersion) + } + }, + isTTY: true, + wantStdout: "✓ Successfully checked extension upgrades\n", + }, + { + name: "upgrade an extension pinned to a version", + args: []string{"upgrade", "hello", "--pin", "v1.2.3-pre"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + assert.Equal(t, "v1.2.3-pre", calls[0].Opts.PinVersion) + } + }, + isTTY: true, + wantStdout: "✓ Successfully checked extension upgrades\n", + }, + { + name: "Upgrade an extension with --latest-pre-release and --all", + args: []string{"upgrade", "--all", "--latest-pre-release"}, + wantErr: true, + errMsg: "cannot use `--latest-pre-release` with `--all`", + }, + { + name: "upgrade an extension with --pin and --all", + args: []string{"upgrade", "--all", "--pin", "v1.2.3"}, + wantErr: true, + errMsg: "cannot use `--pin` with `--all`", + }, + { + name: "upgrade an extension with --pin and --latest-pre-release", + args: []string{"upgrade", "hello", "--pin", "v1.2.3", "--latest-pre-release"}, + wantErr: true, + errMsg: "cannot use `--pin` with `--latest-pre-release`", + }, { name: "upgrade an up-to-date extension", args: []string{"upgrade", "hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { // An already up to date extension returns the same response // as an one that has been upgraded. return nil @@ -434,7 +487,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade extension error", args: []string{"upgrade", "hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return errors.New("oh no") } return func(t *testing.T) { @@ -453,7 +506,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension gh-prefix", args: []string{"upgrade", "gh-hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -469,7 +522,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade an extension full name", args: []string{"upgrade", "monalisa/gh-hello"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -485,7 +538,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade all", args: []string{"upgrade", "--all"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -502,7 +555,7 @@ func TestNewCmdExtension(t *testing.T) { 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 { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -511,7 +564,7 @@ func TestNewCmdExtension(t *testing.T) { upgradeCalls := em.UpgradeCalls() assert.Equal(t, 1, len(upgradeCalls)) assert.Equal(t, "", upgradeCalls[0].Name) - assert.False(t, upgradeCalls[0].Force) + assert.False(t, upgradeCalls[0].Opts.Force) } }, isTTY: true, @@ -521,7 +574,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade all none installed", args: []string{"upgrade", "--all"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return noExtensionsInstalledError } return func(t *testing.T) { @@ -538,7 +591,7 @@ func TestNewCmdExtension(t *testing.T) { name: "upgrade all notty", args: []string{"upgrade", "--all"}, managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { @@ -882,7 +935,7 @@ func TestNewCmdExtension(t *testing.T) { em.InstallFunc = func(_ ghrepo.Interface, _ string) error { return nil } - em.UpgradeFunc = func(name string, force bool) error { + em.UpgradeFunc = func(name string, opts extensions.UpgradeOptions) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 4ff8fa65cfb..9165012e37e 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -3,14 +3,17 @@ package extension import ( "encoding/json" "errors" + "fmt" "io" "net/http" "os" + "time" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/safeurl" + "github.com/hashicorp/go-version" ) func repoExists(httpClient *http.Client, repo ghrepo.Interface) (bool, error) { @@ -73,8 +76,11 @@ type releaseAsset struct { } type release struct { - Tag string `json:"tag_name"` - Assets []releaseAsset + Tag string `json:"tag_name"` + IsPrerelease bool `json:"prerelease"` + IsDraft bool `json:"draft"` + PublishedAt time.Time `json:"published_at"` + Assets []releaseAsset } // downloadAsset downloads a single asset to the given file path. @@ -114,6 +120,7 @@ func downloadAsset(httpClient *http.Client, assetURL safeurl.SafeURL, destPath s var commitNotFoundErr = errors.New("commit not found") var releaseNotFoundErr = errors.New("release not found") var repositoryNotFoundErr = errors.New("repository not found") +var noPrereleasesFoundErr = errors.New("no pre-releases found") // fetchLatestRelease finds the latest published release for a repository. func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { @@ -153,6 +160,94 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } +// fetchLatestPrerelease finds the highest-versioned pre-release for a +// repository. It only considers releases marked as pre-releases, selecting the +// one with the highest version. If the repository has no pre-releases it +// returns noPrereleasesFoundErr. +// +// When a stable (non-pre-release) release beats the chosen pre-release, either +// by a higher version or by a more recent publish date, it is returned as +// newerStable so the caller can warn the user that a newer stable release is +// available. +// +// Note that if the latest pre-release is not on the first page of 100, it is +// possible that this will not find it; for performance reasons in busy +// repositories it is not safe or efficient to iterate over every page of +// releases. In those cases, the user should specify a tag with --pin. +func fetchLatestPrerelease(httpClient *http.Client, baseRepo ghrepo.Interface) (prerelease *release, newerStable *release, err error) { + path := fmt.Sprintf("repos/%s/%s/releases?per_page=100", baseRepo.RepoOwner(), baseRepo.RepoName()) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, nil, err + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, nil, err + } + defer resp.Body.Close() + + if resp.StatusCode == 404 { + return nil, nil, releaseNotFoundErr + } + if resp.StatusCode > 299 { + return nil, nil, api.HandleHTTPError(resp) + } + + b, err := io.ReadAll(resp.Body) + if err != nil { + return nil, nil, err + } + + var releases []release + if err := json.Unmarshal(b, &releases); err != nil { + return nil, nil, err + } + + var bestPre *release + var bestPreVersion *version.Version + var bestStable *release + var bestStableVersion *version.Version + for i := range releases { + r := &releases[i] + if r.IsDraft { + continue + } + // Tags that are not valid semver cannot be ordered against other + // releases, so they are skipped. This means a repository whose newest + // pre-release uses an unparseable tag (e.g. v1.0.0.beta.2) may resolve + // to an older pre-release; users can reach such a release with --pin. + v, verr := version.NewVersion(r.Tag) + if verr != nil { + continue + } + if r.IsPrerelease { + if bestPre == nil || v.GreaterThan(bestPreVersion) { + bestPre = r + bestPreVersion = v + } + continue + } + if bestStable == nil || v.GreaterThan(bestStableVersion) { + bestStable = r + bestStableVersion = v + } + } + + if bestPre == nil { + return nil, nil, noPrereleasesFoundErr + } + + if bestStable != nil { + if bestStableVersion.GreaterThan(bestPreVersion) || bestStable.PublishedAt.After(bestPre.PublishedAt) { + newerStable = bestStable + } + } + + return bestPre, newerStable, nil +} + // fetchReleaseFromTag finds release by tag name for a repository func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { url, err := safeurl.JoinPathWithHostPrefix(ghinstance.RESTPrefix(baseRepo.RepoHost()), "repos", baseRepo.RepoOwner(), baseRepo.RepoName(), "releases", "tags", tagName) diff --git a/pkg/cmd/extension/http_test.go b/pkg/cmd/extension/http_test.go new file mode 100644 index 00000000000..a8895492b32 --- /dev/null +++ b/pkg/cmd/extension/http_test.go @@ -0,0 +1,211 @@ +package extension + +import ( + "net/http" + "testing" + "time" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetchLatestPrerelease(t *testing.T) { + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + tests := []struct { + name string + releases []release + wantTag string + wantNewerStable string + wantErr error + }{ + { + name: "picks highest pre-release", + releases: []release{ + {Tag: "v1.1.0-pre", IsPrerelease: true}, + {Tag: "v1.0.0-pre", IsPrerelease: true}, + }, + wantTag: "v1.1.0-pre", + }, + { + name: "picks highest pre-release even when a lower stable exists", + releases: []release{ + {Tag: "v1.0.1"}, + {Tag: "v1.1.0-pre", IsPrerelease: true}, + {Tag: "v1.0.0-pre", IsPrerelease: true}, + }, + wantTag: "v1.1.0-pre", + }, + { + name: "warns when a stable release is newer by version", + releases: []release{ + {Tag: "v2.0.0"}, + {Tag: "v1.9.0-pre", IsPrerelease: true}, + }, + wantTag: "v1.9.0-pre", + wantNewerStable: "v2.0.0", + }, + { + name: "warns when a stable release is published more recently", + releases: []release{ + {Tag: "v1.0.0", PublishedAt: time.Date(2024, 2, 1, 0, 0, 0, 0, time.UTC)}, + {Tag: "v1.1.0-pre", IsPrerelease: true, PublishedAt: time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)}, + }, + wantTag: "v1.1.0-pre", + wantNewerStable: "v1.0.0", + }, + { + name: "ignores list order in favor of version order", + releases: []release{ + {Tag: "v1.0.0-pre", IsPrerelease: true}, + {Tag: "v3.0.0-pre", IsPrerelease: true}, + {Tag: "v2.0.0-pre", IsPrerelease: true}, + }, + wantTag: "v3.0.0-pre", + }, + { + name: "skips drafts", + releases: []release{ + {Tag: "v9.9.9-pre", IsPrerelease: true, IsDraft: true}, + {Tag: "v1.1.0-pre", IsPrerelease: true}, + }, + wantTag: "v1.1.0-pre", + }, + { + name: "errors when there are no pre-releases", + releases: []release{ + {Tag: "v1.0.0"}, + }, + wantErr: noPrereleasesFoundErr, + }, + { + name: "errors when there are no releases", + releases: []release{}, + wantErr: noPrereleasesFoundErr, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse(tt.releases), + ) + client := &http.Client{Transport: ®} + + r, newerStable, err := fetchLatestPrerelease(client, repo) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantTag, r.Tag) + if tt.wantNewerStable == "" { + assert.Nil(t, newerStable) + } else { + require.NotNil(t, newerStable) + assert.Equal(t, tt.wantNewerStable, newerStable.Tag) + } + }) + } +} + +// TestFetchLatestPrerelease_UnparseableTagSequences documents the behavior +// discussed in PR review around tags that are not valid semver (for example +// v1.0.0.beta.2 from cli/cli#13968). +// +// version.NewVersion cannot order an unparseable tag against the others, so the +// current implementation skips it and keeps the highest parseable pre-release. +// The consequence is that when the newest pre-release uses an unparseable tag we +// silently resolve to an older one instead of erroring and pointing the user at +// --pin. The cases below spell out the various sequences so the trade-off is +// explicit; they assert the CURRENT behavior and pass. The final case captures +// the reviewer's preferred behavior and is skipped rather than changed. +func TestFetchLatestPrerelease_UnparseableTagSequences(t *testing.T) { + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + tests := []struct { + name string + releases []release + wantTag string + wantErr error + note string + }{ + { + name: "unparseable tag alongside a parseable one keeps the parseable", + releases: []release{ + {Tag: "v1.0.0.beta.2", IsPrerelease: true}, + {Tag: "v1.0.0-beta.1", IsPrerelease: true}, + }, + wantTag: "v1.0.0-beta.1", + note: "v1.0.0.beta.2 is newer to a human but is skipped; the older, parseable pre-release wins", + }, + { + name: "only unparseable pre-release tags errors as if none exist", + releases: []release{ + {Tag: "v1.0.0.beta.2", IsPrerelease: true}, + {Tag: "v1.0.0.beta.1", IsPrerelease: true}, + }, + wantErr: noPrereleasesFoundErr, + note: "every pre-release is unparseable, so none is selectable and we cannot point at a specific one", + }, + { + name: "unparseable stable tag is ignored for the newer-stable warning", + releases: []release{ + {Tag: "1.0-latest"}, + {Tag: "v1.0.0-pre", IsPrerelease: true}, + }, + wantTag: "v1.0.0-pre", + note: "an unparseable stable tag cannot be compared, so no newer-stable warning is produced", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse(tt.releases), + ) + client := &http.Client{Transport: ®} + + r, _, err := fetchLatestPrerelease(client, repo) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr, tt.note) + return + } + require.NoError(t, err, tt.note) + assert.Equal(t, tt.wantTag, r.Tag, tt.note) + }) + } + + // Reviewer's preferred behavior: when the newest pre-release tag is + // unparseable we would rather error and point the user at --pin than + // silently install an older pre-release. Implementing this reliably is hard + // because "newest" is undefined for an unparseable tag (it is not ordered as + // a version), so we cannot know it is newest without trusting API ordering. + // This is left as a known limitation; the subtest documents the aspiration + // without changing current behavior. + t.Run("ideal: newest unparseable pre-release errors and points at --pin", func(t *testing.T) { + t.Skip("known limitation: unparseable newest tag is not ordered as a version; use --pin") + + reg := httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse([]release{ + {Tag: "v1.0.0.beta.2", IsPrerelease: true}, + {Tag: "v1.0.0-beta.1", IsPrerelease: true}, + }), + ) + client := &http.Client{Transport: ®} + + _, _, err := fetchLatestPrerelease(client, repo) + require.Error(t, err, "newest pre-release tag is unparseable; should error and suggest --pin") + }) +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index f1528743a39..c559309b3b7 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -288,6 +288,13 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { return err } + return m.installBinRelease(repo, r, isPinned) +} + +// installBinRelease downloads and installs the appropriate asset from an +// already-resolved release, writing a manifest that records whether the +// extension is pinned to that release. +func (m *Manager) installBinRelease(repo ghrepo.Interface, r *release, isPinned bool) error { platform, ext := m.platform() isMacARM := platform == "darwin-arm64" trueARMBinary := false @@ -340,14 +347,14 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { } targetDir := filepath.Join(m.installDir(), name) - if err = os.MkdirAll(targetDir, 0755); err != nil { + if err := os.MkdirAll(targetDir, 0755); err != nil { return fmt.Errorf("failed to create installation directory: %w", err) } binPath := filepath.Join(targetDir, name) binPath += ext - err = downloadAsset(m.client, safeurl.NewImmutableSafeURL(asset.APIURL), binPath) + err := downloadAsset(m.client, safeurl.NewImmutableSafeURL(asset.APIURL), binPath) if err != nil { return fmt.Errorf("failed to download asset %s: %w", asset.Name, err) } @@ -452,7 +459,7 @@ var localExtensionUpgradeError = errors.New("local extensions can not be upgrade var upToDateError = errors.New("already up to date") var noExtensionsInstalledError = errors.New("no extensions installed") -func (m *Manager) Upgrade(name string, force bool) error { +func (m *Manager) Upgrade(name string, opts extensions.UpgradeOptions) error { // Fetch metadata during list only when upgrading all extensions. // This is a performance improvement so that we don't make a // bunch of unnecessary network requests when trying to upgrade a single extension. @@ -462,7 +469,7 @@ func (m *Manager) Upgrade(name string, force bool) error { return noExtensionsInstalledError } if name == "" { - return m.upgradeExtensions(exts, force) + return m.upgradeExtensions(exts, opts) } for _, f := range exts { if f.Name() != name { @@ -472,15 +479,19 @@ func (m *Manager) Upgrade(name string, force bool) error { return localExtensionUpgradeError } // 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) + // When resolving a pin or the latest pre-release we defer version resolution to the upgrade + // itself, so the stable "latest" lookup is not required (and may not exist). + if opts.PinVersion == "" && !opts.LatestPreRelease { + if latestVersion := f.LatestVersion(); latestVersion == "" { + return fmt.Errorf("unable to retrieve latest version for extension %q", name) + } } - return m.upgradeExtensions([]*Extension{f}, force) + return m.upgradeExtensions([]*Extension{f}, opts) } return fmt.Errorf("no extension matched %q", name) } -func (m *Manager) upgradeExtensions(exts []*Extension, force bool) error { +func (m *Manager) upgradeExtensions(exts []*Extension, opts extensions.UpgradeOptions) error { var longestExt = slices.MaxFunc(exts, func(a, b *Extension) int { return len(a.Name()) - len(b.Name()) }) @@ -490,7 +501,7 @@ func (m *Manager) upgradeExtensions(exts []*Extension, force bool) error { for _, f := range exts { fmt.Fprintf(m.io.Out, "[%*s]: ", longestExtName, f.Name()) currentVersion := displayExtensionVersion(f, f.CurrentVersion()) - err := m.upgradeExtension(f, force) + err := m.upgradeExtension(f, opts) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && !errors.Is(err, upToDateError) && @@ -513,35 +524,42 @@ func (m *Manager) upgradeExtensions(exts []*Extension, force bool) error { return nil } -func (m *Manager) upgradeExtension(ext *Extension, force bool) error { +func (m *Manager) upgradeExtension(ext *Extension, opts extensions.UpgradeOptions) error { if ext.IsLocal() { return localExtensionUpgradeError } - if !force && ext.IsPinned() { + // Pinned extensions are only upgraded with --force or when explicitly + // re-pinned to a new version via --pin. + if !opts.Force && opts.PinVersion == "" && ext.IsPinned() { return pinnedExtensionUpgradeError } + + if ext.IsBinary() { + return m.upgradeBinExtension(ext, opts) + } + + // Release-based selection is only meaningful for binary extensions. + if opts.LatestPreRelease || opts.PinVersion != "" { + return errors.New("the --pin and --latest-pre-release flags are only supported for binary extensions") + } + if !ext.UpdateAvailable() { return upToDateError } - var err error - if ext.IsBinary() { - err = m.upgradeBinExtension(ext) - } else { - // Check if git extension has changed to a binary extension - var isBin bool - repo, repoErr := repoFromPath(m.gitClient, filepath.Join(ext.Path(), "..")) - if repoErr == nil { - isBin, _ = isBinExtension(m.client, repo) - } - if isBin { - 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, "") + + // Check if git extension has changed to a binary extension + var isBin bool + repo, repoErr := repoFromPath(m.gitClient, filepath.Join(ext.Path(), "..")) + if repoErr == nil { + isBin, _ = isBinExtension(m.client, repo) + } + if isBin { + if err := m.Remove(ext.Name()); err != nil { + return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) } - err = m.upgradeGitExtension(ext, force) + return m.installBin(repo, "") } - return err + return m.upgradeGitExtension(ext, opts.Force) } func (m *Manager) upgradeGitExtension(ext *Extension, force bool) error { @@ -563,12 +581,66 @@ func (m *Manager) upgradeGitExtension(ext *Extension, force bool) error { return scopedClient.Pull("", "") } -func (m *Manager) upgradeBinExtension(ext *Extension) error { +func (m *Manager) upgradeBinExtension(ext *Extension, opts extensions.UpgradeOptions) error { repo, err := ghrepo.FromFullName(ext.URL()) if err != nil { return fmt.Errorf("failed to parse URL %s: %w", ext.URL(), err) } - return m.installBin(repo, "") + + // Pinning installs the requested release regardless of the currently + // installed version, allowing intentional pinning to (or downgrading to) a + // specific release. + if opts.PinVersion != "" { + if ext.CurrentVersion() == opts.PinVersion && ext.IsPinned() { + return upToDateError + } + if err := m.installBin(repo, opts.PinVersion); err != nil { + return err + } + ext.latestVersion = opts.PinVersion + return nil + } + + var target *release + if opts.LatestPreRelease { + var newerStable *release + target, newerStable, err = fetchLatestPrerelease(m.client, repo) + if errors.Is(err, noPrereleasesFoundErr) { + return fmt.Errorf("no pre-releases found for %s", ext.Name()) + } + if err != nil { + return err + } + if newerStable != nil { + cs := m.io.ColorScheme() + fmt.Fprintf(m.io.ErrOut, "%s a newer stable release (%s) is available for %s; installing pre-release %s\n", + cs.WarningIcon(), newerStable.Tag, ext.Name(), target.Tag) + } + if ext.CurrentVersion() == target.Tag { + return upToDateError + } + } else { + // The latest stable version was already fetched (and cached) when the + // extension was listed, so rely on the cached comparison instead of + // making another network request. This also keeps an "already up to + // date" result from turning into a failure when the fetch errors. + if !ext.UpdateAvailable() { + return upToDateError + } + target, err = fetchLatestRelease(m.client, repo) + if err != nil { + return err + } + if ext.CurrentVersion() == target.Tag { + return upToDateError + } + } + + if err := m.installBinRelease(repo, target, false); err != nil { + return err + } + ext.latestVersion = target.Tag + return nil } func (m *Manager) Remove(name string) error { diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 1e8f8248324..cb9bed650ed 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -216,7 +216,7 @@ func TestManager_Upgrade_NoExtensions(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() m := newTestManager(dataDir, updateDir, nil, nil, ios) - err := m.Upgrade("", false) + err := m.Upgrade("", extensions.UpgradeOptions{}) assert.EqualError(t, err, "no extensions installed") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -232,7 +232,7 @@ func TestManager_Upgrade_NoMatchingExtension(t *testing.T) { gc.On("ForRepo", extDir).Return(gcOne).Once() m := newTestManager(dataDir, updateDir, nil, gc, ios) - err := m.Upgrade("invalid", false) + err := m.Upgrade("invalid", extensions.UpgradeOptions{}) assert.EqualError(t, err, `no extension matched "invalid"`) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -265,7 +265,7 @@ func TestManager_UpgradeExtensions(t *testing.T) { exts[i].currentVersion = "old version" exts[i].latestVersion = "new version" } - err = m.upgradeExtensions(exts, false) + err = m.upgradeExtensions(exts, extensions.UpgradeOptions{}) assert.NoError(t, err) assert.Equal(t, heredoc.Doc( ` @@ -304,7 +304,7 @@ func TestManager_UpgradeExtensions_DryRun(t *testing.T) { exts[i].currentVersion = fmt.Sprintf("%d", i) exts[i].latestVersion = fmt.Sprintf("%d", i+1) } - err = m.upgradeExtensions(exts, false) + err = m.upgradeExtensions(exts, extensions.UpgradeOptions{}) assert.NoError(t, err) assert.Equal(t, heredoc.Doc( ` @@ -329,7 +329,7 @@ func TestManager_UpgradeExtension_LocalExtension(t *testing.T) { exts, err := m.list(false) assert.NoError(t, err) assert.Equal(t, 1, len(exts)) - err = m.upgradeExtension(exts[0], false) + err = m.upgradeExtension(exts[0], extensions.UpgradeOptions{}) assert.EqualError(t, err, "local extensions can not be upgraded") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -346,7 +346,7 @@ func TestManager_UpgradeExtension_LocalExtension_DryRun(t *testing.T) { exts, err := m.list(false) assert.NoError(t, err) assert.Equal(t, 1, len(exts)) - err = m.upgradeExtension(exts[0], false) + err = m.upgradeExtension(exts[0], extensions.UpgradeOptions{}) assert.EqualError(t, err, "local extensions can not be upgraded") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -370,7 +370,7 @@ func TestManager_UpgradeExtension_GitExtension(t *testing.T) { ext := exts[0] ext.currentVersion = "old version" ext.latestVersion = "new version" - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -396,7 +396,7 @@ func TestManager_UpgradeExtension_GitExtension_DryRun(t *testing.T) { ext := exts[0] ext.currentVersion = "old version" ext.latestVersion = "new version" - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -423,7 +423,7 @@ func TestManager_UpgradeExtension_GitExtension_Force(t *testing.T) { ext := exts[0] ext.currentVersion = "old version" ext.latestVersion = "new version" - err = m.upgradeExtension(ext, true) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{Force: true}) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -484,7 +484,7 @@ func TestManager_MigrateToBinaryExtension(t *testing.T) { httpmock.REST("GET", "release/cool"), httpmock.StringResponse("FAKE UPGRADED BINARY")) - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NoError(t, err) assert.Equal(t, "", stdout.String()) @@ -550,7 +550,7 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, 1, len(exts)) ext := exts[0] ext.latestVersion = "v1.0.2" - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext", manifestName)) @@ -616,7 +616,7 @@ func TestManager_UpgradeExtension_BinaryExtension_Pinned_Force(t *testing.T) { assert.Equal(t, 1, len(exts)) ext := exts[0] ext.latestVersion = "v1.0.2" - err = m.upgradeExtension(ext, true) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{Force: true}) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext", manifestName)) @@ -642,6 +642,339 @@ func TestManager_UpgradeExtension_BinaryExtension_Pinned_Force(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_UpgradeExtension_BinaryExtension_LatestPreRelease(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.1", + })) + + ios, _, stdout, stderr := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + // The releases list is returned in reverse-chronological order; the highest + // version (a pre-release) should win over both the older stable release and + // an even older pre-release. + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse( + []release{ + { + Tag: "v1.1.0-pre", + IsPrerelease: true, + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/pre", + }, + }, + }, + { + Tag: "v1.0.1", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/stable", + }, + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/pre"), + httpmock.StringResponse("FAKE PRERELEASE BINARY")) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + err = m.upgradeExtension(ext, extensions.UpgradeOptions{LatestPreRelease: true}) + assert.NoError(t, err) + + manifest, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext", manifestName)) + assert.NoError(t, err) + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + assert.NoError(t, err) + + assert.Equal(t, binManifest{ + Name: "gh-bin-ext", + Owner: "owner", + Host: "example.com", + Tag: "v1.1.0-pre", + Path: filepath.Join(dataDir, "extensions/gh-bin-ext/gh-bin-ext.exe"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext/gh-bin-ext.exe")) + assert.NoError(t, err) + assert.Equal(t, "FAKE PRERELEASE BINARY", string(fakeBin)) + + assert.Equal(t, "v1.1.0-pre", ext.LatestVersion()) + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_UpgradeExtension_BinaryExtension_LatestPreRelease_UpToDate(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.1.0-pre", + })) + + ios, _, _, _ := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse( + []release{ + { + Tag: "v1.1.0-pre", + IsPrerelease: true, + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/pre", + }, + }, + }, + })) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + err = m.upgradeExtension(exts[0], extensions.UpgradeOptions{LatestPreRelease: true}) + assert.ErrorIs(t, err, upToDateError) +} + +func TestManager_UpgradeExtension_BinaryExtension_LatestPreRelease_WarnsNewerStable(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.0-pre", + })) + + ios, _, stdout, stderr := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + // The highest pre-release is older than the newest stable release, so the + // pre-release is still installed but the user is warned about the newer + // stable release. + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse( + []release{ + { + Tag: "v2.0.0", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/stable", + }, + }, + }, + { + Tag: "v1.1.0-pre", + IsPrerelease: true, + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/pre", + }, + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/pre"), + httpmock.StringResponse("FAKE PRERELEASE BINARY")) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + err = m.upgradeExtension(exts[0], extensions.UpgradeOptions{LatestPreRelease: true}) + assert.NoError(t, err) + + assert.Equal(t, "v1.1.0-pre", exts[0].LatestVersion()) + assert.Equal(t, "", stdout.String()) + assert.Contains(t, stderr.String(), "a newer stable release (v2.0.0) is available for bin-ext") + assert.Contains(t, stderr.String(), "installing pre-release v1.1.0-pre") +} + +func TestManager_UpgradeExtension_BinaryExtension_LatestPreRelease_NoPrereleases(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.0", + })) + + ios, _, _, _ := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases"), + httpmock.JSONResponse( + []release{ + {Tag: "v1.0.0"}, + })) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + err = m.upgradeExtension(exts[0], extensions.UpgradeOptions{LatestPreRelease: true}) + assert.EqualError(t, err, "no pre-releases found for bin-ext") +} + +// TestManager_UpgradeExtension_BinaryExtension_UpToDate_NoNetwork verifies that +// an up-to-date binary extension resolves to upToDateError using the cached +// latest version, without making a second network request. No HTTP responder is +// registered, so any network call would fail the test. +func TestManager_UpgradeExtension_BinaryExtension_UpToDate_NoNetwork(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.0", + })) + + ios, _, _, _ := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + // Simulate the cached latest version already resolved during list(), + // matching the installed version so no upgrade is available. + ext.latestVersion = "v1.0.0" + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) + assert.ErrorIs(t, err, upToDateError) +} + +func TestManager_UpgradeExtension_BinaryExtension_Pin(t *testing.T) { + dataDir := t.TempDir() + updateDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(dataDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.2", + })) + + ios, _, stdout, stderr := iostreams.Test() + m := newTestManager(dataDir, updateDir, &http.Client{Transport: ®}, nil, ios) + // Pinning to an older pre-release should install exactly that release even + // though it is a downgrade, and mark the extension as pinned. + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/tags/v1.0.1-pre"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.1-pre", + IsPrerelease: true, + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/pin", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/pin"), + httpmock.StringResponse("FAKE PINNED BINARY")) + + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + err = m.upgradeExtension(ext, extensions.UpgradeOptions{PinVersion: "v1.0.1-pre"}) + assert.NoError(t, err) + + manifest, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext", manifestName)) + assert.NoError(t, err) + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + assert.NoError(t, err) + + assert.Equal(t, binManifest{ + Name: "gh-bin-ext", + Owner: "owner", + Host: "example.com", + Tag: "v1.0.1-pre", + IsPinned: true, + Path: filepath.Join(dataDir, "extensions/gh-bin-ext/gh-bin-ext.exe"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext/gh-bin-ext.exe")) + assert.NoError(t, err) + assert.Equal(t, "FAKE PINNED BINARY", string(fakeBin)) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_UpgradeExtension_GitExtension_ReleaseFlagsUnsupported(t *testing.T) { + tempDir := t.TempDir() + ext := &Extension{ + path: filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"), + kind: GitKind, + // Set currentVersion so the pinned check does not reach the git client. + currentVersion: "old version", + } + m := newTestManager(tempDir, t.TempDir(), nil, nil, nil) + + err := m.upgradeExtension(ext, extensions.UpgradeOptions{LatestPreRelease: true}) + assert.EqualError(t, err, "the --pin and --latest-pre-release flags are only supported for binary extensions") + + err = m.upgradeExtension(ext, extensions.UpgradeOptions{PinVersion: "v1.0.0"}) + assert.EqualError(t, err, "the --pin and --latest-pre-release flags are only supported for binary extensions") +} + func TestManager_UpgradeExtension_BinaryExtension_DryRun(t *testing.T) { dataDir := t.TempDir() updateDir := t.TempDir() @@ -676,7 +1009,7 @@ func TestManager_UpgradeExtension_BinaryExtension_DryRun(t *testing.T) { assert.Equal(t, 1, len(exts)) ext := exts[0] ext.latestVersion = "v1.0.2" - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(dataDir, "extensions/gh-bin-ext", manifestName)) @@ -717,7 +1050,7 @@ func TestManager_UpgradeExtension_BinaryExtension_Pinned(t *testing.T) { assert.Equal(t, 1, len(exts)) ext := exts[0] - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NotNil(t, err) assert.Equal(t, err, pinnedExtensionUpgradeError) } @@ -744,7 +1077,7 @@ func TestManager_UpgradeExtension_GitExtension_Pinned(t *testing.T) { ext.isPinned = &pinnedTrue ext.latestVersion = "new version" - err = m.upgradeExtension(ext, false) + err = m.upgradeExtension(ext, extensions.UpgradeOptions{}) assert.NotNil(t, err) assert.Equal(t, err, pinnedExtensionUpgradeError) gc.AssertExpectations(t) diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 1b9c3461204..898258617e6 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -28,12 +28,24 @@ type Extension interface { Owner() string } +// UpgradeOptions configures how installed extensions are upgraded. +type UpgradeOptions struct { + // Force upgrades the extension even when it is pinned or already up to date. + Force bool + // LatestPreRelease upgrades to the most recent release, including pre-releases, + // selected by version order. Only supported for binary extensions. + LatestPreRelease bool + // PinVersion, when set, upgrades to a specific release tag and pins the + // extension to it. Only supported for a single named binary extension. + PinVersion string +} + //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List() []Extension Install(ghrepo.Interface, string) error InstallLocal(dir string) error - Upgrade(name string, force bool) error + Upgrade(name string, opts UpgradeOptions) error Remove(name string) error Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) Create(name string, tmplType ExtTemplateType) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 81f0ba3f7d6..0b436912be6 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -78,7 +78,7 @@ type ExtensionManagerMock struct { UpdateDirFunc func(name string) string // UpgradeFunc mocks the Upgrade method. - UpgradeFunc func(name string, force bool) error + UpgradeFunc func(name string, opts UpgradeOptions) error // calls tracks calls to the methods. calls struct { @@ -132,8 +132,8 @@ type ExtensionManagerMock struct { Upgrade []struct { // Name is the name argument value. Name string - // Force is the force argument value. - Force bool + // Opts is the opts argument value. + Opts UpgradeOptions } } lockCreate sync.RWMutex @@ -414,21 +414,21 @@ func (mock *ExtensionManagerMock) UpdateDirCalls() []struct { } // Upgrade calls UpgradeFunc. -func (mock *ExtensionManagerMock) Upgrade(name string, force bool) error { +func (mock *ExtensionManagerMock) Upgrade(name string, opts UpgradeOptions) error { if mock.UpgradeFunc == nil { panic("ExtensionManagerMock.UpgradeFunc: method is nil but ExtensionManager.Upgrade was just called") } callInfo := struct { - Name string - Force bool + Name string + Opts UpgradeOptions }{ - Name: name, - Force: force, + Name: name, + Opts: opts, } mock.lockUpgrade.Lock() mock.calls.Upgrade = append(mock.calls.Upgrade, callInfo) mock.lockUpgrade.Unlock() - return mock.UpgradeFunc(name, force) + return mock.UpgradeFunc(name, opts) } // UpgradeCalls gets all the calls that were made to Upgrade. @@ -436,12 +436,12 @@ func (mock *ExtensionManagerMock) Upgrade(name string, force bool) error { // // len(mockedExtensionManager.UpgradeCalls()) func (mock *ExtensionManagerMock) UpgradeCalls() []struct { - Name string - Force bool + Name string + Opts UpgradeOptions } { var calls []struct { - Name string - Force bool + Name string + Opts UpgradeOptions } mock.lockUpgrade.RLock() calls = mock.calls.Upgrade