From fe5eca1dd5da2625abee56aaebed26ae1ecde7b4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 1 Mar 2022 15:01:04 -0800 Subject: [PATCH 01/11] pinning binary exts --- pkg/cmd/extension/command.go | 63 ++++++++++++++++++---------------- pkg/cmd/extension/http.go | 34 ++++++++++++++++++ pkg/cmd/extension/manager.go | 17 +++++---- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 14 +++++--- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index de39d0287..c4a08152b 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -70,10 +70,12 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return t.Render() }, }, - &cobra.Command{ - Use: "install ", - Short: "Install a gh extension from a repository", - Long: heredoc.Doc(` + func() *cobra.Command { + var pinFlag string + cmd := &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Long: heredoc.Doc(` Install a GitHub repository locally as a GitHub CLI extension. The repository argument can be specified in "owner/repo" format as well as a full URL. @@ -84,41 +86,44 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { See the list of available extensions at `), - Example: heredoc.Doc(` + Example: heredoc.Doc(` $ gh extension install owner/gh-extension $ gh extension install https://git.example.com/owner/gh-extension $ gh extension install . `), - Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), - RunE: func(cmd *cobra.Command, args []string) error { - if args[0] == "." { - wd, err := os.Getwd() + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + if args[0] == "." { + wd, err := os.Getwd() + if err != nil { + return err + } + return m.InstallLocal(wd) + } + + repo, err := ghrepo.FromFullName(args[0]) if err != nil { return err } - return m.InstallLocal(wd) - } - repo, err := ghrepo.FromFullName(args[0]) - if err != nil { - return err - } + if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + return err + } - if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { - return err - } + if err := m.Install(repo, pinFlag); err != nil { + return err + } - if err := m.Install(repo); err != nil { - return err - } - - if io.IsStdoutTTY() { - cs := io.ColorScheme() - fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) - } - return nil - }, - }, + if io.IsStdoutTTY() { + cs := io.ColorScheme() + fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + } + return nil + }, + } + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + return cmd + }(), func() *cobra.Command { var flagAll bool var flagForce bool diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index cfae2b738..d1bbe84e7 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -112,3 +112,37 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } + +// fetchRelease finds release by tag name for a repository +func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { + fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) + path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return nil, err + } + + if resp.StatusCode > 299 { + return nil, api.HandleHTTPError(resp) + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + var r release + err = json.Unmarshal(b, &r) + if err != nil { + return nil, err + } + + return &r, nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 691110656..6ff5f461a 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -320,13 +320,13 @@ type binManifest struct { Path string } -func (m *Manager) Install(repo ghrepo.Interface) error { +func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo) + return m.installBin(repo, targetCommitish) } hs, err := hasScript(m.client, repo) @@ -341,9 +341,14 @@ func (m *Manager) Install(repo ghrepo.Interface) error { return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface) error { +func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { var r *release - r, err := fetchLatestRelease(m.client, repo) + var err error + if targetCommitish == "" { + r, err = fetchLatestRelease(m.client, repo) + } else { + r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } if err != nil { return err } @@ -498,7 +503,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if err != nil { return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) } - return m.installBin(repo) + return m.installBin(repo, "") } err = m.upgradeGitExtension(ext, force) } @@ -525,7 +530,7 @@ func (m *Manager) upgradeBinExtension(ext Extension) error { if err != nil { return fmt.Errorf("failed to parse URL %s: %w", ext.url, err) } - return m.installBin(repo) + return m.installBin(repo, "") } func (m *Manager) Remove(name string) error { diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 5826f4cd6..6b57a1cc5 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -27,7 +27,7 @@ type Extension interface { //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List(includeMetadata bool) []Extension - Install(ghrepo.Interface) error + Install(ghrepo.Interface, string) error InstallLocal(dir string) error Upgrade(name string, force bool) error Remove(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 80e1efd57..01eac92bb 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -25,7 +25,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // panic("mock out the Dispatch method") // }, -// InstallFunc: func(interfaceMoqParam ghrepo.Interface) error { +// InstallFunc: func(interfaceMoqParam ghrepo.Interface, s string) error { // panic("mock out the Install method") // }, // InstallLocalFunc: func(dir string) error { @@ -54,7 +54,7 @@ type ExtensionManagerMock struct { DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) // InstallFunc mocks the Install method. - InstallFunc func(interfaceMoqParam ghrepo.Interface) error + InstallFunc func(interfaceMoqParam ghrepo.Interface, s string) error // InstallLocalFunc mocks the InstallLocal method. InstallLocalFunc func(dir string) error @@ -92,6 +92,8 @@ type ExtensionManagerMock struct { Install []struct { // InterfaceMoqParam is the interfaceMoqParam argument value. InterfaceMoqParam ghrepo.Interface + // S is the s argument value. + S string } // InstallLocal holds details about calls to the InstallLocal method. InstallLocal []struct { @@ -204,19 +206,21 @@ func (mock *ExtensionManagerMock) DispatchCalls() []struct { } // Install calls InstallFunc. -func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) error { +func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface, s string) error { if mock.InstallFunc == nil { panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") } callInfo := struct { InterfaceMoqParam ghrepo.Interface + S string }{ InterfaceMoqParam: interfaceMoqParam, + S: s, } mock.lockInstall.Lock() mock.calls.Install = append(mock.calls.Install, callInfo) mock.lockInstall.Unlock() - return mock.InstallFunc(interfaceMoqParam) + return mock.InstallFunc(interfaceMoqParam, s) } // InstallCalls gets all the calls that were made to Install. @@ -224,9 +228,11 @@ func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) er // len(mockedExtensionManager.InstallCalls()) func (mock *ExtensionManagerMock) InstallCalls() []struct { InterfaceMoqParam ghrepo.Interface + S string } { var calls []struct { InterfaceMoqParam ghrepo.Interface + S string } mock.lockInstall.RLock() calls = mock.calls.Install From 44334bbec65bea8b09852640e1a812c9704c6ab4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 19:25:25 -0800 Subject: [PATCH 02/11] pinning script exts --- pkg/cmd/extension/command.go | 14 +++++++++++-- pkg/cmd/extension/http.go | 40 +++++++++++++++++++++++++++++++++++- pkg/cmd/extension/manager.go | 31 ++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 916940749..63f560710 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -111,18 +111,28 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } + cs := io.ColorScheme() if err := m.Install(repo, pinFlag); err != nil { + if errors.Is(err, releaseNotFoundErr) { + return fmt.Errorf("%s Could not find a release of %s for %s", + cs.FailureIcon(), args[0], cs.Cyan(pinFlag)) + } else if errors.Is(err, commitNotFoundErr) { + return fmt.Errorf("%s %s does not exist in %s", + cs.FailureIcon(), cs.Cyan(pinFlag), args[0]) + } return err } if io.IsStdoutTTY() { - cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + if pinFlag != "" { + fmt.Fprintf(io.Out, "%s Pinned extension at %s\n", cs.SuccessIcon(), cs.Cyan(pinFlag)) + } } return nil }, } - cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit ref") return cmd }(), func() *cobra.Command { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index d1bbe84e7..38ba9c5e5 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -2,6 +2,7 @@ package extension import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -80,6 +81,9 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return err } +var releaseNotFoundErr = errors.New("release not found") +var commitNotFoundErr = errors.New("commit not found") + // fetchLatestRelease finds the latest published release for a repository. func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName()) @@ -113,7 +117,7 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } -// fetchRelease finds release by tag name for a repository +// fetchReleaseFromTag finds release by tag name for a repository func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) @@ -129,6 +133,9 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return nil, err } + if resp.StatusCode == 404 { + return nil, releaseNotFoundErr + } if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } @@ -146,3 +153,34 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return &r, nil } + +// fetchCommitSHA finds full commit SHA from a target ref in a repo +func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRef string) (string, error) { + path := fmt.Sprintf("repos/%s/%s/commits/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), targetRef) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Accept", "application/vnd.github.VERSION.sha") + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return "", err + } + + if resp.StatusCode == 422 { + return "", commitNotFoundErr + } + if resp.StatusCode > 299 { + return "", api.HandleHTTPError(resp) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + return string(body), nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 41dab528d..aa45ee48c 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "io/ioutil" + "log" "net/http" "os" "os/exec" @@ -320,6 +321,7 @@ type binManifest struct { Path string } +// Install an extension from repo, and pin to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -337,11 +339,11 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") - return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) + return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { + log.Println("Installing binary extension") var r *release var err error if targetCommitish == "" { @@ -413,19 +415,40 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { + protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") + cloneURL := ghrepo.FormatRemoteURL(repo, protocol) + exe, err := m.lookPath("git") if err != nil { return err } + var commitSHA string + if targetCommitish != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if err != nil { + return err + } + } + name := strings.TrimSuffix(path.Base(cloneURL), ".git") targetDir := filepath.Join(m.installDir(), name) externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) externalCmd.Stdout = stdout externalCmd.Stderr = stderr - return externalCmd.Run() + if err := externalCmd.Run(); err != nil { + return err + } + if commitSHA == "" { + return nil + } + + checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) + checkoutCmd.Stdout = stdout + checkoutCmd.Stderr = stderr + return checkoutCmd.Run() } var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") From bdab7de1d2b79641eb873cbb9b0b1b5c08017478 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 21:06:23 -0800 Subject: [PATCH 03/11] list pinned exts --- pkg/cmd/extension/command.go | 11 ++++--- pkg/cmd/extension/extension.go | 8 ++++- pkg/cmd/extension/manager.go | 55 ++++++++++++++++++++++++-------- pkg/extensions/extension.go | 3 +- pkg/extensions/extension_mock.go | 36 +++++++++++++++++++++ 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 63f560710..eb3a4306a 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -61,11 +61,14 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil) t.AddField(repo, nil, nil) - var updateAvailable string - if c.UpdateAvailable() { - updateAvailable = "Upgrade available" + + if c.Pin() != "" { + t.AddField(c.Pin(), nil, cs.Cyan) + } else if c.UpdateAvailable() { + t.AddField("Upgrade available", nil, cs.Green) + } else { + t.AddField("", nil, nil) } - t.AddField(updateAvailable, nil, cs.Green) t.EndRow() } return t.Render() diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index b4106228f..87cd20c78 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -18,6 +18,7 @@ type Extension struct { path string url string isLocal bool + pin string currentVersion string latestVersion string kind ExtensionKind @@ -39,8 +40,13 @@ func (e *Extension) IsLocal() bool { return e.isLocal } +func (e *Extension) Pin() string { + return e.pin +} + func (e *Extension) UpdateAvailable() bool { - if e.isLocal || + if e.pin != "" || + e.isLocal || e.currentVersion == "" || e.latestVersion == "" || e.currentVersion == e.latestVersion { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index aa45ee48c..34db89a10 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,7 +8,6 @@ import ( "io" "io/fs" "io/ioutil" - "log" "net/http" "os" "os/exec" @@ -199,6 +198,9 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag + if bm.Pinned { + ext.pin = bm.Tag + } return ext, nil } @@ -207,12 +209,20 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { exePath := filepath.Join(id, fi.Name(), fi.Name()) remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) + + var pinnedVersion string + pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) + if _, err := os.Stat(pinPath); err == nil { + pinnedVersion = currentVersion[:7] + } + return Extension{ path: exePath, url: remoteUrl, isLocal: false, currentVersion: currentVersion, kind: GitKind, + pin: pinnedVersion, }, nil } @@ -313,15 +323,16 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { - Owner string - Name string - Host string - Tag string + Owner string + Name string + Host string + Tag string + Pinned bool // TODO I may end up not using this; just thinking ahead to local installs Path string } -// Install an extension from repo, and pin to commitish if provided +// Install installs an extension from repo, and pins to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -343,10 +354,10 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { - log.Println("Installing binary extension") var r *release var err error - if targetCommitish == "" { + isPinned := targetCommitish != "" + if isPinned { r, err = fetchLatestRelease(m.client, repo) } else { r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) @@ -372,6 +383,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro name := repo.RepoName() targetDir := filepath.Join(m.installDir(), name) + // TODO clean this up if function errs? err = os.MkdirAll(targetDir, 0755) if err != nil { @@ -387,11 +399,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro } manifest := binManifest{ - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + Tag: r.Tag, + Pinned: isPinned, } bs, err := yaml.Marshal(manifest) @@ -448,9 +461,20 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) checkoutCmd.Stdout = stdout checkoutCmd.Stderr = stderr - return checkoutCmd.Run() + if err := checkoutCmd.Run(); err != nil { + return err + } + + pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + if err != nil { + return fmt.Errorf("failed to create pin in directory: %w", err) + } + return nil } +var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") var upToDateError = errors.New("already up to date") var noExtensionsInstalledError = errors.New("no extensions installed") @@ -508,6 +532,9 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } + if ext.pin != "" { + return pinnedExtensionUpgradeError + } if !ext.UpdateAvailable() { return upToDateError } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 6b57a1cc5..2db8ab0fb 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -19,9 +19,10 @@ type Extension interface { Name() string // Extension Name without gh- Path() string // Path to executable URL() string - IsLocal() bool + Pin() string // Pinned version UpdateAvailable() bool IsBinary() bool + IsLocal() bool } //go:generate moq -rm -out manager_mock.go . ExtensionManager diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index b999ab955..9d47d1c4f 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -29,6 +29,9 @@ var _ Extension = &ExtensionMock{} // PathFunc: func() string { // panic("mock out the Path method") // }, +// PinFunc: func() string { +// panic("mock out the Pin method") +// }, // URLFunc: func() string { // panic("mock out the URL method") // }, @@ -54,6 +57,9 @@ type ExtensionMock struct { // PathFunc mocks the Path method. PathFunc func() string + // PinFunc mocks the Pin method. + PinFunc func() string + // URLFunc mocks the URL method. URLFunc func() string @@ -74,6 +80,9 @@ type ExtensionMock struct { // Path holds details about calls to the Path method. Path []struct { } + // Pin holds details about calls to the Pin method. + Pin []struct { + } // URL holds details about calls to the URL method. URL []struct { } @@ -85,6 +94,7 @@ type ExtensionMock struct { lockIsLocal sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex + lockPin sync.RWMutex lockURL sync.RWMutex lockUpdateAvailable sync.RWMutex } @@ -193,6 +203,32 @@ func (mock *ExtensionMock) PathCalls() []struct { return calls } +// Pin calls PinFunc. +func (mock *ExtensionMock) Pin() string { + if mock.PinFunc == nil { + panic("ExtensionMock.PinFunc: method is nil but Extension.Pin was just called") + } + callInfo := struct { + }{} + mock.lockPin.Lock() + mock.calls.Pin = append(mock.calls.Pin, callInfo) + mock.lockPin.Unlock() + return mock.PinFunc() +} + +// PinCalls gets all the calls that were made to Pin. +// Check the length with: +// len(mockedExtension.PinCalls()) +func (mock *ExtensionMock) PinCalls() []struct { +} { + var calls []struct { + } + mock.lockPin.RLock() + calls = mock.calls.Pin + mock.lockPin.RUnlock() + return calls +} + // URL calls URLFunc. func (mock *ExtensionMock) URL() string { if mock.URLFunc == nil { From f350b917e477280f89b8351331e2d55110399945 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Thu, 3 Mar 2022 22:10:35 -0800 Subject: [PATCH 04/11] fix tests --- pkg/cmd/extension/command_test.go | 2 +- pkg/cmd/extension/http.go | 4 +- pkg/cmd/extension/manager.go | 31 +++++++------- pkg/cmd/extension/manager_test.go | 68 +++++++++++++++++++++++++++++-- 4 files changed, 84 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 8f896eab0..1c7e4dd6d 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -44,7 +44,7 @@ func TestNewCmdExtension(t *testing.T) { em.ListFunc = func(bool) []extensions.Extension { return []extensions.Extension{} } - em.InstallFunc = func(_ ghrepo.Interface) error { + em.InstallFunc = func(_ ghrepo.Interface, _ string) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 38ba9c5e5..7208b1569 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -128,11 +128,11 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag } resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode == 404 { return nil, releaseNotFoundErr } @@ -165,11 +165,11 @@ func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRe req.Header.Set("Accept", "application/vnd.github.VERSION.sha") resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return "", err } + defer resp.Body.Close() if resp.StatusCode == 422 { return "", commitNotFoundErr } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 34db89a10..b6067fb7d 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -212,7 +212,7 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { var pinnedVersion string pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) - if _, err := os.Stat(pinPath); err == nil { + if _, err := os.Stat(pinPath); err == nil && len(currentVersion) > 6 { pinnedVersion = currentVersion[:7] } @@ -235,6 +235,7 @@ func (m *Manager) getCurrentVersion(extension string) string { dir := m.installDir() gitDir := "--git-dir=" + filepath.Join(dir, extension, ".git") cmd := m.newCommand(gitExe, gitDir, "rev-parse", "HEAD") + localSha, err := cmd.Output() if err != nil { return "" @@ -333,13 +334,13 @@ type binManifest struct { } // Install installs an extension from repo, and pins to commitish if provided -func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) Install(repo ghrepo.Interface, target string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo, targetCommitish) + return m.installBin(repo, target) } hs, err := hasScript(m.client, repo) @@ -350,17 +351,17 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) + return m.installGit(repo, target, m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) installBin(repo ghrepo.Interface, target string) error { var r *release var err error - isPinned := targetCommitish != "" + isPinned := target != "" if isPinned { - r, err = fetchLatestRelease(m.client, repo) + r, err = fetchReleaseFromTag(m.client, repo, target) } else { - r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + r, err = fetchLatestRelease(m.client, repo) } if err != nil { return err @@ -428,7 +429,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stderr io.Writer) error { protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") cloneURL := ghrepo.FormatRemoteURL(repo, protocol) @@ -438,8 +439,8 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } var commitSHA string - if targetCommitish != "" { - commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if target != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, target) if err != nil { return err } @@ -466,12 +467,11 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) - f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - defer f.Close() + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return fmt.Errorf("failed to create pin in directory: %w", err) } - return nil + return f.Close() } var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") @@ -514,7 +514,8 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { err := m.upgradeExtension(f, force) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && - !errors.Is(err, upToDateError) { + !errors.Is(err, upToDateError) && + !errors.Is(err, pinnedExtensionUpgradeError) { failed = true } fmt.Fprintf(m.io.Out, "%s\n", err) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 47f11d348..e1fc45f49 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,6 +445,50 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { + tempDir := t.TempDir() + + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + Pinned: true, + })) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.Nil(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + +func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { + tempDir := t.TempDir() + extDir := filepath.Join(tempDir, "extensions", "gh-remote") + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + ext.latestVersion = "new version" + ext.pin = "abc1234" + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + func TestManager_Install_git(t *testing.T) { tempDir := t.TempDir() @@ -472,7 +516,7 @@ func TestManager_Install_git(t *testing.T) { repo := ghrepo.New("owner", "gh-some-ext") - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) assert.Equal(t, fmt.Sprintf("[git clone https://github.com/owner/gh-some-ext.git %s]\n", filepath.Join(tempDir, "extensions", "gh-some-ext")), stdout.String()) assert.Equal(t, "", stderr.String()) @@ -514,7 +558,7 @@ func TestManager_Install_binary_unsupported(t *testing.T) { m := newTestManager(tempDir, &client, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.EqualError(t, err, "gh-bin-ext unsupported for windows-amd64. Open an issue: `gh issue create -R owner/gh-bin-ext -t'Support windows-amd64'`") assert.Equal(t, "", stdout.String()) @@ -559,7 +603,7 @@ func TestManager_Install_binary(t *testing.T) { m := newTestManager(tempDir, &http.Client{Transport: ®}, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -702,6 +746,24 @@ func stubExtension(path string) error { return f.Close() } +func stubPinnedExtension(path string, pinnedVersion string) error { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + f.Close() + + pinPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".pin-%s", pinnedVersion)) + f, err = os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) + if err != nil { + return err + } + return f.Close() +} + func stubLocalExtension(tempDir, path string) error { extDir, err := ioutil.TempDir(tempDir, "local-ext") if err != nil { From d7277e396cfc31cd7bf9bce5c4e82f17c7ef9f25 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 1 Mar 2022 15:01:04 -0800 Subject: [PATCH 05/11] pinning binary exts --- pkg/cmd/extension/command.go | 63 ++++++++++++++++++---------------- pkg/cmd/extension/http.go | 34 ++++++++++++++++++ pkg/cmd/extension/manager.go | 17 +++++---- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 14 +++++--- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 1c929482f..50aad7a40 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -76,10 +76,12 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return t.Render() }, }, - &cobra.Command{ - Use: "install ", - Short: "Install a gh extension from a repository", - Long: heredoc.Doc(` + func() *cobra.Command { + var pinFlag string + cmd := &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Long: heredoc.Doc(` Install a GitHub repository locally as a GitHub CLI extension. The repository argument can be specified in "owner/repo" format as well as a full URL. @@ -90,41 +92,44 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { See the list of available extensions at . `), - Example: heredoc.Doc(` + Example: heredoc.Doc(` $ gh extension install owner/gh-extension $ gh extension install https://git.example.com/owner/gh-extension $ gh extension install . `), - Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), - RunE: func(cmd *cobra.Command, args []string) error { - if args[0] == "." { - wd, err := os.Getwd() + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + if args[0] == "." { + wd, err := os.Getwd() + if err != nil { + return err + } + return m.InstallLocal(wd) + } + + repo, err := ghrepo.FromFullName(args[0]) if err != nil { return err } - return m.InstallLocal(wd) - } - repo, err := ghrepo.FromFullName(args[0]) - if err != nil { - return err - } + if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + return err + } - if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { - return err - } + if err := m.Install(repo, pinFlag); err != nil { + return err + } - if err := m.Install(repo); err != nil { - return err - } - - if io.IsStdoutTTY() { - cs := io.ColorScheme() - fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) - } - return nil - }, - }, + if io.IsStdoutTTY() { + cs := io.ColorScheme() + fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + } + return nil + }, + } + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + return cmd + }(), func() *cobra.Command { var flagAll bool var flagForce bool diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index cfae2b738..d1bbe84e7 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -112,3 +112,37 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } + +// fetchRelease finds release by tag name for a repository +func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { + fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) + path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return nil, err + } + + if resp.StatusCode > 299 { + return nil, api.HandleHTTPError(resp) + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + var r release + err = json.Unmarshal(b, &r) + if err != nil { + return nil, err + } + + return &r, nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 3449092cc..41dab528d 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -320,13 +320,13 @@ type binManifest struct { Path string } -func (m *Manager) Install(repo ghrepo.Interface) error { +func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo) + return m.installBin(repo, targetCommitish) } hs, err := hasScript(m.client, repo) @@ -341,9 +341,14 @@ func (m *Manager) Install(repo ghrepo.Interface) error { return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface) error { +func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { var r *release - r, err := fetchLatestRelease(m.client, repo) + var err error + if targetCommitish == "" { + r, err = fetchLatestRelease(m.client, repo) + } else { + r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } if err != nil { return err } @@ -498,7 +503,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if err != nil { return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) } - return m.installBin(repo) + return m.installBin(repo, "") } err = m.upgradeGitExtension(ext, force) } @@ -525,7 +530,7 @@ func (m *Manager) upgradeBinExtension(ext Extension) error { if err != nil { return fmt.Errorf("failed to parse URL %s: %w", ext.url, err) } - return m.installBin(repo) + return m.installBin(repo, "") } func (m *Manager) Remove(name string) error { diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index a9e120dc9..fa1959ab7 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -28,7 +28,7 @@ type Extension interface { //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List(includeMetadata bool) []Extension - Install(ghrepo.Interface) error + Install(ghrepo.Interface, string) error InstallLocal(dir string) error Upgrade(name string, force bool) error Remove(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 80e1efd57..01eac92bb 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -25,7 +25,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // panic("mock out the Dispatch method") // }, -// InstallFunc: func(interfaceMoqParam ghrepo.Interface) error { +// InstallFunc: func(interfaceMoqParam ghrepo.Interface, s string) error { // panic("mock out the Install method") // }, // InstallLocalFunc: func(dir string) error { @@ -54,7 +54,7 @@ type ExtensionManagerMock struct { DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) // InstallFunc mocks the Install method. - InstallFunc func(interfaceMoqParam ghrepo.Interface) error + InstallFunc func(interfaceMoqParam ghrepo.Interface, s string) error // InstallLocalFunc mocks the InstallLocal method. InstallLocalFunc func(dir string) error @@ -92,6 +92,8 @@ type ExtensionManagerMock struct { Install []struct { // InterfaceMoqParam is the interfaceMoqParam argument value. InterfaceMoqParam ghrepo.Interface + // S is the s argument value. + S string } // InstallLocal holds details about calls to the InstallLocal method. InstallLocal []struct { @@ -204,19 +206,21 @@ func (mock *ExtensionManagerMock) DispatchCalls() []struct { } // Install calls InstallFunc. -func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) error { +func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface, s string) error { if mock.InstallFunc == nil { panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") } callInfo := struct { InterfaceMoqParam ghrepo.Interface + S string }{ InterfaceMoqParam: interfaceMoqParam, + S: s, } mock.lockInstall.Lock() mock.calls.Install = append(mock.calls.Install, callInfo) mock.lockInstall.Unlock() - return mock.InstallFunc(interfaceMoqParam) + return mock.InstallFunc(interfaceMoqParam, s) } // InstallCalls gets all the calls that were made to Install. @@ -224,9 +228,11 @@ func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) er // len(mockedExtensionManager.InstallCalls()) func (mock *ExtensionManagerMock) InstallCalls() []struct { InterfaceMoqParam ghrepo.Interface + S string } { var calls []struct { InterfaceMoqParam ghrepo.Interface + S string } mock.lockInstall.RLock() calls = mock.calls.Install From db53df102c57ccea9f46bfa88cfc9456afa771e4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 19:25:25 -0800 Subject: [PATCH 06/11] pinning script exts --- pkg/cmd/extension/command.go | 14 +++++++++++-- pkg/cmd/extension/http.go | 40 +++++++++++++++++++++++++++++++++++- pkg/cmd/extension/manager.go | 31 ++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 50aad7a40..ee0dbd512 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -116,18 +116,28 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } + cs := io.ColorScheme() if err := m.Install(repo, pinFlag); err != nil { + if errors.Is(err, releaseNotFoundErr) { + return fmt.Errorf("%s Could not find a release of %s for %s", + cs.FailureIcon(), args[0], cs.Cyan(pinFlag)) + } else if errors.Is(err, commitNotFoundErr) { + return fmt.Errorf("%s %s does not exist in %s", + cs.FailureIcon(), cs.Cyan(pinFlag), args[0]) + } return err } if io.IsStdoutTTY() { - cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + if pinFlag != "" { + fmt.Fprintf(io.Out, "%s Pinned extension at %s\n", cs.SuccessIcon(), cs.Cyan(pinFlag)) + } } return nil }, } - cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit ref") return cmd }(), func() *cobra.Command { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index d1bbe84e7..38ba9c5e5 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -2,6 +2,7 @@ package extension import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -80,6 +81,9 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return err } +var releaseNotFoundErr = errors.New("release not found") +var commitNotFoundErr = errors.New("commit not found") + // fetchLatestRelease finds the latest published release for a repository. func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName()) @@ -113,7 +117,7 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } -// fetchRelease finds release by tag name for a repository +// fetchReleaseFromTag finds release by tag name for a repository func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) @@ -129,6 +133,9 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return nil, err } + if resp.StatusCode == 404 { + return nil, releaseNotFoundErr + } if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } @@ -146,3 +153,34 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return &r, nil } + +// fetchCommitSHA finds full commit SHA from a target ref in a repo +func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRef string) (string, error) { + path := fmt.Sprintf("repos/%s/%s/commits/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), targetRef) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Accept", "application/vnd.github.VERSION.sha") + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return "", err + } + + if resp.StatusCode == 422 { + return "", commitNotFoundErr + } + if resp.StatusCode > 299 { + return "", api.HandleHTTPError(resp) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + return string(body), nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 41dab528d..aa45ee48c 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "io/ioutil" + "log" "net/http" "os" "os/exec" @@ -320,6 +321,7 @@ type binManifest struct { Path string } +// Install an extension from repo, and pin to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -337,11 +339,11 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") - return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) + return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { + log.Println("Installing binary extension") var r *release var err error if targetCommitish == "" { @@ -413,19 +415,40 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { + protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") + cloneURL := ghrepo.FormatRemoteURL(repo, protocol) + exe, err := m.lookPath("git") if err != nil { return err } + var commitSHA string + if targetCommitish != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if err != nil { + return err + } + } + name := strings.TrimSuffix(path.Base(cloneURL), ".git") targetDir := filepath.Join(m.installDir(), name) externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) externalCmd.Stdout = stdout externalCmd.Stderr = stderr - return externalCmd.Run() + if err := externalCmd.Run(); err != nil { + return err + } + if commitSHA == "" { + return nil + } + + checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) + checkoutCmd.Stdout = stdout + checkoutCmd.Stderr = stderr + return checkoutCmd.Run() } var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") From bed630452bb2c29fbef3134479070f2e6d724b7b Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 21:06:23 -0800 Subject: [PATCH 07/11] list pinned exts --- pkg/cmd/extension/command.go | 8 ++++- pkg/cmd/extension/extension.go | 8 ++++- pkg/cmd/extension/manager.go | 57 +++++++++++++++++++++++--------- pkg/extensions/extension.go | 3 +- pkg/extensions/extension_mock.go | 36 ++++++++++++++++++++ 5 files changed, 93 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ee0dbd512..ec756edfa 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -65,7 +65,13 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if !c.IsBinary() && len(version) > 8 { version = version[:8] } - t.AddField(version, nil, nil) + + if c.IsPinned() { + t.AddField(version, nil, cs.Cyan) + } else { + t.AddField(version, nil, nil) + } + var updateAvailable string if c.UpdateAvailable() { updateAvailable = "Upgrade available" diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index d1b814c57..e4f109d83 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -18,6 +18,7 @@ type Extension struct { path string url string isLocal bool + isPinned bool currentVersion string latestVersion string kind ExtensionKind @@ -43,8 +44,13 @@ func (e *Extension) CurrentVersion() string { return e.currentVersion } +func (e *Extension) IsPinned() bool { + return e.isPinned +} + func (e *Extension) UpdateAvailable() bool { - if e.isLocal || + if e.isPinned || + e.isLocal || e.currentVersion == "" || e.latestVersion == "" || e.currentVersion == e.latestVersion { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index aa45ee48c..031e6d3a7 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,7 +8,6 @@ import ( "io" "io/fs" "io/ioutil" - "log" "net/http" "os" "os/exec" @@ -199,6 +198,7 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag + ext.isPinned = bm.IsPinned return ext, nil } @@ -207,12 +207,20 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { 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 } @@ -313,15 +321,16 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { - Owner string - Name string - Host string - Tag string + Owner string + Name string + Host string + Tag string + IsPinned bool // TODO I may end up not using this; just thinking ahead to local installs Path string } -// Install an extension from repo, and pin to commitish if provided +// Install installs an extension from repo, and pins to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -343,13 +352,13 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { - log.Println("Installing binary extension") var r *release var err error - if targetCommitish == "" { - r, err = fetchLatestRelease(m.client, repo) - } else { + isPinned := targetCommitish != "" + if isPinned { r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } else { + r, err = fetchLatestRelease(m.client, repo) } if err != nil { return err @@ -372,6 +381,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro name := repo.RepoName() targetDir := filepath.Join(m.installDir(), name) + // TODO clean this up if function errs? err = os.MkdirAll(targetDir, 0755) if err != nil { @@ -387,11 +397,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro } manifest := binManifest{ - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + Tag: r.Tag, + IsPinned: isPinned, } bs, err := yaml.Marshal(manifest) @@ -448,9 +459,20 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) checkoutCmd.Stdout = stdout checkoutCmd.Stderr = stderr - return checkoutCmd.Run() + if err := checkoutCmd.Run(); err != nil { + return err + } + + pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + if err != nil { + return fmt.Errorf("failed to create pin file in directory: %w", err) + } + return nil } +var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") var upToDateError = errors.New("already up to date") var noExtensionsInstalledError = errors.New("no extensions installed") @@ -508,6 +530,9 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } + if ext.IsPinned() { + return pinnedExtensionUpgradeError + } if !ext.UpdateAvailable() { return upToDateError } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index fa1959ab7..aa1019a70 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -19,10 +19,11 @@ type Extension interface { Name() string // Extension Name without gh- Path() string // Path to executable URL() string - IsLocal() bool CurrentVersion() string + IsPinned() bool UpdateAvailable() bool IsBinary() bool + IsLocal() bool } //go:generate moq -rm -out manager_mock.go . ExtensionManager diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index 5b418c3f1..e68c376f2 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -26,6 +26,9 @@ var _ Extension = &ExtensionMock{} // IsLocalFunc: func() bool { // panic("mock out the IsLocal method") // }, +// IsPinnedFunc: func() bool { +// panic("mock out the IsPinned method") +// }, // NameFunc: func() string { // panic("mock out the Name method") // }, @@ -54,6 +57,9 @@ type ExtensionMock struct { // IsLocalFunc mocks the IsLocal method. IsLocalFunc func() bool + // IsPinnedFunc mocks the IsPinned method. + IsPinnedFunc func() bool + // NameFunc mocks the Name method. NameFunc func() string @@ -77,6 +83,9 @@ type ExtensionMock struct { // IsLocal holds details about calls to the IsLocal method. IsLocal []struct { } + // IsPinned holds details about calls to the IsPinned method. + IsPinned []struct { + } // Name holds details about calls to the Name method. Name []struct { } @@ -93,6 +102,7 @@ type ExtensionMock struct { lockCurrentVersion sync.RWMutex lockIsBinary sync.RWMutex lockIsLocal sync.RWMutex + lockIsPinned sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex lockURL sync.RWMutex @@ -177,6 +187,32 @@ func (mock *ExtensionMock) IsLocalCalls() []struct { return calls } +// IsPinned calls IsPinnedFunc. +func (mock *ExtensionMock) IsPinned() bool { + if mock.IsPinnedFunc == nil { + panic("ExtensionMock.IsPinnedFunc: method is nil but Extension.IsPinned was just called") + } + callInfo := struct { + }{} + mock.lockIsPinned.Lock() + mock.calls.IsPinned = append(mock.calls.IsPinned, callInfo) + mock.lockIsPinned.Unlock() + return mock.IsPinnedFunc() +} + +// IsPinnedCalls gets all the calls that were made to IsPinned. +// Check the length with: +// len(mockedExtension.IsPinnedCalls()) +func (mock *ExtensionMock) IsPinnedCalls() []struct { +} { + var calls []struct { + } + mock.lockIsPinned.RLock() + calls = mock.calls.IsPinned + mock.lockIsPinned.RUnlock() + return calls +} + // Name calls NameFunc. func (mock *ExtensionMock) Name() string { if mock.NameFunc == nil { From 619774d460e192774b806189890ead7f890e0626 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Thu, 3 Mar 2022 22:10:35 -0800 Subject: [PATCH 08/11] fix tests --- pkg/cmd/extension/command_test.go | 2 +- pkg/cmd/extension/http.go | 4 +- pkg/cmd/extension/manager.go | 27 ++++++------ pkg/cmd/extension/manager_test.go | 68 +++++++++++++++++++++++++++++-- 4 files changed, 82 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 7e5520fa4..541b1b137 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -44,7 +44,7 @@ func TestNewCmdExtension(t *testing.T) { em.ListFunc = func(bool) []extensions.Extension { return []extensions.Extension{} } - em.InstallFunc = func(_ ghrepo.Interface) error { + em.InstallFunc = func(_ ghrepo.Interface, _ string) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 38ba9c5e5..7208b1569 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -128,11 +128,11 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag } resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode == 404 { return nil, releaseNotFoundErr } @@ -165,11 +165,11 @@ func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRe req.Header.Set("Accept", "application/vnd.github.VERSION.sha") resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return "", err } + defer resp.Body.Close() if resp.StatusCode == 422 { return "", commitNotFoundErr } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 031e6d3a7..156212909 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -233,6 +233,7 @@ func (m *Manager) getCurrentVersion(extension string) string { dir := m.installDir() gitDir := "--git-dir=" + filepath.Join(dir, extension, ".git") cmd := m.newCommand(gitExe, gitDir, "rev-parse", "HEAD") + localSha, err := cmd.Output() if err != nil { return "" @@ -331,13 +332,13 @@ type binManifest struct { } // Install installs an extension from repo, and pins to commitish if provided -func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) Install(repo ghrepo.Interface, target string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo, targetCommitish) + return m.installBin(repo, target) } hs, err := hasScript(m.client, repo) @@ -348,15 +349,15 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) + return m.installGit(repo, target, m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) installBin(repo ghrepo.Interface, target string) error { var r *release var err error - isPinned := targetCommitish != "" + isPinned := target != "" if isPinned { - r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + r, err = fetchReleaseFromTag(m.client, repo, target) } else { r, err = fetchLatestRelease(m.client, repo) } @@ -426,7 +427,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stderr io.Writer) error { protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") cloneURL := ghrepo.FormatRemoteURL(repo, protocol) @@ -436,8 +437,8 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } var commitSHA string - if targetCommitish != "" { - commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if target != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, target) if err != nil { return err } @@ -464,12 +465,11 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) - f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - defer f.Close() + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return fmt.Errorf("failed to create pin file in directory: %w", err) } - return nil + return f.Close() } var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") @@ -512,7 +512,8 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { err := m.upgradeExtension(f, force) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && - !errors.Is(err, upToDateError) { + !errors.Is(err, upToDateError) && + !errors.Is(err, pinnedExtensionUpgradeError) { failed = true } fmt.Fprintf(m.io.Out, "%s\n", err) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 47f11d348..e1fc45f49 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,6 +445,50 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { + tempDir := t.TempDir() + + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + Pinned: true, + })) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.Nil(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + +func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { + tempDir := t.TempDir() + extDir := filepath.Join(tempDir, "extensions", "gh-remote") + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + ext.latestVersion = "new version" + ext.pin = "abc1234" + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + func TestManager_Install_git(t *testing.T) { tempDir := t.TempDir() @@ -472,7 +516,7 @@ func TestManager_Install_git(t *testing.T) { repo := ghrepo.New("owner", "gh-some-ext") - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) assert.Equal(t, fmt.Sprintf("[git clone https://github.com/owner/gh-some-ext.git %s]\n", filepath.Join(tempDir, "extensions", "gh-some-ext")), stdout.String()) assert.Equal(t, "", stderr.String()) @@ -514,7 +558,7 @@ func TestManager_Install_binary_unsupported(t *testing.T) { m := newTestManager(tempDir, &client, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.EqualError(t, err, "gh-bin-ext unsupported for windows-amd64. Open an issue: `gh issue create -R owner/gh-bin-ext -t'Support windows-amd64'`") assert.Equal(t, "", stdout.String()) @@ -559,7 +603,7 @@ func TestManager_Install_binary(t *testing.T) { m := newTestManager(tempDir, &http.Client{Transport: ®}, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -702,6 +746,24 @@ func stubExtension(path string) error { return f.Close() } +func stubPinnedExtension(path string, pinnedVersion string) error { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + f.Close() + + pinPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".pin-%s", pinnedVersion)) + f, err = os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) + if err != nil { + return err + } + return f.Close() +} + func stubLocalExtension(tempDir, path string) error { extDir, err := ioutil.TempDir(tempDir, "local-ext") if err != nil { From 65bc3acaa54c0b02fae7aab28cc6c650f886ad27 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Mon, 7 Mar 2022 17:58:48 -0800 Subject: [PATCH 09/11] merge upstream --- pkg/cmd/extension/command.go | 1 + pkg/cmd/extension/manager.go | 42 ------------------------------------ 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 66370305c..ec756edfa 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -76,6 +76,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if c.UpdateAvailable() { updateAvailable = "Upgrade available" } + t.AddField(updateAvailable, nil, cs.Green) t.EndRow() } return t.Render() diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 9452c2b53..156212909 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -198,13 +198,7 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag -<<<<<<< HEAD - if bm.Pinned { - ext.pin = bm.Tag - } -======= ext.isPinned = bm.IsPinned ->>>>>>> tmp return ext, nil } @@ -214,17 +208,10 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) -<<<<<<< HEAD - var pinnedVersion string - pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) - if _, err := os.Stat(pinPath); err == nil && len(currentVersion) > 6 { - pinnedVersion = currentVersion[:7] -======= var isPinned bool pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) if _, err := os.Stat(pinPath); err == nil { isPinned = true ->>>>>>> tmp } return Extension{ @@ -233,11 +220,7 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { isLocal: false, currentVersion: currentVersion, kind: GitKind, -<<<<<<< HEAD - pin: pinnedVersion, -======= isPinned: isPinned, ->>>>>>> tmp }, nil } @@ -339,19 +322,11 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { -<<<<<<< HEAD - Owner string - Name string - Host string - Tag string - Pinned bool -======= Owner string Name string Host string Tag string IsPinned bool ->>>>>>> tmp // TODO I may end up not using this; just thinking ahead to local installs Path string } @@ -423,21 +398,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { } manifest := binManifest{ -<<<<<<< HEAD - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, - Pinned: isPinned, -======= Name: name, Owner: repo.RepoOwner(), Host: repo.RepoHost(), Path: binPath, Tag: r.Tag, IsPinned: isPinned, ->>>>>>> tmp } bs, err := yaml.Marshal(manifest) @@ -501,11 +467,7 @@ func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stder pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { -<<<<<<< HEAD - return fmt.Errorf("failed to create pin in directory: %w", err) -======= return fmt.Errorf("failed to create pin file in directory: %w", err) ->>>>>>> tmp } return f.Close() } @@ -569,11 +531,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } -<<<<<<< HEAD - if ext.pin != "" { -======= if ext.IsPinned() { ->>>>>>> tmp return pinnedExtensionUpgradeError } if !ext.UpdateAvailable() { From 8174640cda58b0f8858e0df2f55c1c2c59c87dfd Mon Sep 17 00:00:00 2001 From: meiji163 Date: Mon, 7 Mar 2022 19:47:21 -0800 Subject: [PATCH 10/11] install tests --- pkg/cmd/extension/manager_test.go | 119 +++++++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index e1fc45f49..717a0b906 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,17 +445,17 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } -func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { +func TestManager_UpgradeExtension_BinaryExtension_Pinned(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubBinaryExtension( filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ - Owner: "owner", - Name: "gh-bin-ext", - Host: "example.com", - Tag: "v1.6.3", - Pinned: true, + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + IsPinned: true, })) io, _, _, _ := iostreams.Test() @@ -470,10 +470,10 @@ func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { assert.Equal(t, err, pinnedExtensionUpgradeError) } -func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { +func TestManager_UpgradeExtenion_GitExtension_Pinned(t *testing.T) { tempDir := t.TempDir() extDir := filepath.Join(tempDir, "extensions", "gh-remote") - assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abcd1234")) io, _, _, _ := iostreams.Test() m := newTestManager(tempDir, nil, io) @@ -481,8 +481,8 @@ func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(exts)) ext := exts[0] + ext.isPinned = true ext.latestVersion = "new version" - ext.pin = "abc1234" err = m.upgradeExtension(ext, false) assert.NotNil(t, err) @@ -522,6 +522,107 @@ func TestManager_Install_git(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_Install_git_pinned(t *testing.T) { + tempDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + io, _, _, stderr := iostreams.Test() + m := newTestManager(tempDir, &client, io) + + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "not-a-binary", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/commits/some-ref"), + httpmock.StringResponse("abcd1234")) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/contents/gh-cool-ext"), + httpmock.StringResponse("script")) + + _ = os.MkdirAll(filepath.Join(m.installDir(), "gh-cool-ext"), 0700) + repo := ghrepo.New("owner", "gh-cool-ext") + err := m.Install(repo, "some-ref") + assert.NoError(t, err) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Install_binary_pinned(t *testing.T) { + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + reg := httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/tags/v1.6.3-pre"), + httpmock.JSONResponse( + release{ + Tag: "v1.6.3-pre", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/cool"), + httpmock.StringResponse("FAKE BINARY")) + + io, _, stdout, stderr := iostreams.Test() + tempDir := t.TempDir() + + m := newTestManager(tempDir, &http.Client{Transport: ®}, io) + + err := m.Install(repo, "v1.6.3-pre") + assert.NoError(t, err) + + manifest, err := os.ReadFile(filepath.Join(tempDir, "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.6.3-pre", + IsPinned: true, + Path: filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext.exe"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext.exe")) + assert.NoError(t, err) + assert.Equal(t, "FAKE BINARY", string(fakeBin)) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + +} + func TestManager_Install_binary_unsupported(t *testing.T) { repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") From 45845bc999c0bd17b9cb2b02e7c5c85c33c892d9 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 22 Mar 2022 11:02:16 -0700 Subject: [PATCH 11/11] cant pin local --- pkg/cmd/extension/command.go | 3 +++ pkg/cmd/extension/command_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ec756edfa..33306bb56 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -106,6 +106,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), RunE: func(cmd *cobra.Command, args []string) error { if args[0] == "." { + if pinFlag != "" { + return fmt.Errorf("local extensions cannot be pinned") + } wd, err := os.Getwd() if err != nil { return err diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 541b1b137..c84bf0664 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -86,6 +86,13 @@ func TestNewCmdExtension(t *testing.T) { } }, }, + { + name: "install local extension with pin", + args: []string{"install", ".", "--pin", "v1.0.0"}, + wantErr: true, + errMsg: "local extensions cannot be pinned", + isTTY: true, + }, { name: "upgrade argument error", args: []string{"upgrade"},