From db5bbf799fde38097889f016bb5447fad2ab2724 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Sep 2021 16:11:12 -0500 Subject: [PATCH 01/11] use manager io in Upgrade --- pkg/cmd/extension/command.go | 2 +- pkg/cmd/extension/command_test.go | 9 ++++---- pkg/cmd/extension/manager.go | 8 +++---- pkg/cmd/extension/manager_test.go | 37 ++++++++++++++----------------- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 36 ++++++++++-------------------- 6 files changed, 39 insertions(+), 55 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ad92a8f14..264945026 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -132,7 +132,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if len(args) > 0 { name = normalizeExtensionSelector(args[0]) } - return m.Upgrade(name, flagForce, io.Out, io.ErrOut) + return m.Upgrade(name, flagForce) }, } cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions") diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index fac5d0e9a..789b57b91 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -1,7 +1,6 @@ package extension import ( - "io" "io/ioutil" "net/http" "os" @@ -94,7 +93,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, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -108,7 +107,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, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -122,7 +121,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, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { @@ -136,7 +135,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, out, errOut io.Writer) error { + em.UpgradeFunc = func(name string, force bool) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 7e1f403e2..d8ec1d94a 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -306,7 +306,7 @@ func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") -func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) error { +func (m *Manager) Upgrade(name string, force bool) error { exe, err := m.lookPath("git") if err != nil { return err @@ -320,14 +320,14 @@ func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) err someUpgraded := false for _, f := range exts { if name == "" { - fmt.Fprintf(stdout, "[%s]: ", f.Name()) + fmt.Fprintf(m.io.Out, "[%s]: ", f.Name()) } else if f.Name() != name { continue } if f.IsLocal() { if name == "" { - fmt.Fprintf(stdout, "%s\n", localExtensionUpgradeError) + fmt.Fprintf(m.io.Out, "%s\n", localExtensionUpgradeError) } else { err = localExtensionUpgradeError } @@ -344,7 +344,7 @@ func (m *Manager) Upgrade(name string, force bool, stdout, stderr io.Writer) err pullCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") cmds = []*exec.Cmd{pullCmd} } - if e := runCmds(cmds, stdout, stderr); e != nil { + if e := runCmds(cmds, m.io.Out, m.io.ErrOut); e != nil { err = e } someUpgraded = true diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index d78f4c2e6..19e9e1965 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -108,11 +108,11 @@ func TestManager_Upgrade_AllExtensions(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("", false, stdout, stderr) + m := newTestManager(tempDir, nil, io) + + err := m.Upgrade("", false) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( @@ -133,11 +133,11 @@ func TestManager_Upgrade_RemoteExtension(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("remote", false, stdout, stderr) + m := newTestManager(tempDir, nil, io) + + err := m.Upgrade("remote", false) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( ` @@ -153,11 +153,10 @@ func TestManager_Upgrade_LocalExtension(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubLocalExtension(tempDir, filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("local", false, stdout, stderr) + err := m.Upgrade("local", false) assert.EqualError(t, err, "local extensions can not be upgraded") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) @@ -170,11 +169,10 @@ func TestManager_Upgrade_Force(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("remote", true, stdout, stderr) + err := m.Upgrade("remote", true) assert.NoError(t, err) assert.Equal(t, heredoc.Docf( ` @@ -192,11 +190,10 @@ func TestManager_Upgrade_Force(t *testing.T) { func TestManager_Upgrade_NoExtensions(t *testing.T) { tempDir := t.TempDir() - m := newTestManager(tempDir, nil, nil) + io, _, stdout, stderr := iostreams.Test() + m := newTestManager(tempDir, nil, io) - stdout := &bytes.Buffer{} - stderr := &bytes.Buffer{} - err := m.Upgrade("", false, stdout, stderr) + err := m.Upgrade("", false) assert.EqualError(t, err, "no extensions installed") assert.Equal(t, "", stdout.String()) assert.Equal(t, "", stderr.String()) diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 4e9ce89b5..c9f1472f9 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -20,7 +20,7 @@ type ExtensionManager interface { List(includeMetadata bool) []Extension Install(ghrepo.Interface) error InstallLocal(dir string) error - Upgrade(name string, force bool, stdout, stderr io.Writer) error + Upgrade(name string, force bool) error Remove(name string) error Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) Create(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 96d76cdf7..c681efe7d 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -37,7 +37,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // RemoveFunc: func(name string) error { // panic("mock out the Remove method") // }, -// UpgradeFunc: func(name string, force bool, stdout io.Writer, stderr io.Writer) error { +// UpgradeFunc: func(name string, force bool) error { // panic("mock out the Upgrade method") // }, // } @@ -66,7 +66,7 @@ type ExtensionManagerMock struct { RemoveFunc func(name string) error // UpgradeFunc mocks the Upgrade method. - UpgradeFunc func(name string, force bool, stdout io.Writer, stderr io.Writer) error + UpgradeFunc func(name string, force bool) error // calls tracks calls to the methods. calls struct { @@ -112,10 +112,6 @@ type ExtensionManagerMock struct { Name string // Force is the force argument value. Force bool - // Stdout is the stdout argument value. - Stdout io.Writer - // Stderr is the stderr argument value. - Stderr io.Writer } } lockCreate sync.RWMutex @@ -326,41 +322,33 @@ func (mock *ExtensionManagerMock) RemoveCalls() []struct { } // Upgrade calls UpgradeFunc. -func (mock *ExtensionManagerMock) Upgrade(name string, force bool, stdout io.Writer, stderr io.Writer) error { +func (mock *ExtensionManagerMock) Upgrade(name string, force bool) error { if mock.UpgradeFunc == nil { panic("ExtensionManagerMock.UpgradeFunc: method is nil but ExtensionManager.Upgrade was just called") } callInfo := struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool }{ - Name: name, - Force: force, - Stdout: stdout, - Stderr: stderr, + Name: name, + Force: force, } mock.lockUpgrade.Lock() mock.calls.Upgrade = append(mock.calls.Upgrade, callInfo) mock.lockUpgrade.Unlock() - return mock.UpgradeFunc(name, force, stdout, stderr) + return mock.UpgradeFunc(name, force) } // UpgradeCalls gets all the calls that were made to Upgrade. // Check the length with: // len(mockedExtensionManager.UpgradeCalls()) func (mock *ExtensionManagerMock) UpgradeCalls() []struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool } { var calls []struct { - Name string - Force bool - Stdout io.Writer - Stderr io.Writer + Name string + Force bool } mock.lockUpgrade.RLock() calls = mock.calls.Upgrade From 54ec5329c5ad00e57151481182e210ac869176bd Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 22 Sep 2021 16:37:08 -0500 Subject: [PATCH 02/11] add ability to upgrade binary extensions --- pkg/cmd/extension/extension.go | 2 + pkg/cmd/extension/manager.go | 66 +++++++++++++++++++++++++------ pkg/cmd/extension/manager_test.go | 2 +- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index 502522efa..39d81c52a 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -5,6 +5,8 @@ import ( "strings" ) +const manifestName = "manifest.yml" + type Extension struct { path string url string diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index d8ec1d94a..d7c1cf670 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -105,6 +105,7 @@ func (m *Manager) List(includeMetadata bool) []extensions.Extension { } func (m *Manager) list(includeMetadata bool) ([]extensions.Extension, error) { + // TODO need to fix this to work with binary extensions before upgrade will work dir := m.installDir() entries, err := ioutil.ReadDir(dir) if err != nil { @@ -273,7 +274,7 @@ func (m *Manager) installBin(repo ghrepo.Interface) error { return fmt.Errorf("failed to serialize manifest: %w", err) } - manifestPath := filepath.Join(targetDir, "manifest.yml") + manifestPath := filepath.Join(targetDir, manifestName) f, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { @@ -334,27 +335,70 @@ func (m *Manager) Upgrade(name string, force bool) error { continue } - var cmds []*exec.Cmd - dir := filepath.Dir(f.Path()) - if force { - fetchCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "fetch", "origin", "HEAD") - resetCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "reset", "--hard", "origin/HEAD") - cmds = []*exec.Cmd{fetchCmd, resetCmd} - } else { - pullCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") - cmds = []*exec.Cmd{pullCmd} + _, err := os.Stat(filepath.Join(f.Path(), manifestName)) + if err == nil { + // TODO is there any need for a "forced" binary upgrade? + err = m.upgradeBin(f) + someUpgraded = true + continue } - if e := runCmds(cmds, m.io.Out, m.io.ErrOut); e != nil { + + if e := m.upgradeGit(f, exe, force); e != nil { err = e } someUpgraded = true } + if err == nil && !someUpgraded { err = fmt.Errorf("no extension matched %q", name) } + return err } +func (m *Manager) upgradeGit(ext extensions.Extension, exe string, force bool) error { + var cmds []*exec.Cmd + dir := filepath.Dir(ext.Path()) + if force { + fetchCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "fetch", "origin", "HEAD") + resetCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "reset", "--hard", "origin/HEAD") + cmds = []*exec.Cmd{fetchCmd, resetCmd} + } else { + pullCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") + cmds = []*exec.Cmd{pullCmd} + } + + return runCmds(cmds, m.io.Out, m.io.ErrOut) +} + +func (m *Manager) upgradeBin(ext extensions.Extension) error { + manifestPath := filepath.Join(ext.Path(), manifestName) + manifest, err := os.ReadFile(manifestPath) + if err != nil { + return fmt.Errorf("could not open %s for reading: %w", manifestPath, err) + } + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + if err != nil { + return fmt.Errorf("could not parse %s: %w", manifestPath, err) + } + repo := ghrepo.NewWithHost(bm.Owner, bm.Name, bm.Host) + var r *release + + r, err = fetchLatestRelease(m.client, repo) + if err != nil { + return fmt.Errorf("failed to get release info for %s: %w", ghrepo.FullName(repo), err) + } + + if bm.Tag == r.Tag { + return nil + } + + // TODO will this work? + return m.installBin(repo) +} + func (m *Manager) Remove(name string) error { targetDir := filepath.Join(m.installDir(), "gh-"+name) if _, err := os.Lstat(targetDir); os.IsNotExist(err) { diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 19e9e1965..a13933fb7 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -319,7 +319,7 @@ func TestManager_Install_binary(t *testing.T) { err := m.Install(repo) assert.NoError(t, err) - manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/manifest.yml")) + manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) assert.NoError(t, err) var bm binManifest From 1fe49fa77682090237f5f422d9ec4634f0109c19 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 27 Sep 2021 16:23:31 -0500 Subject: [PATCH 03/11] fix listing, cleanup --- pkg/cmd/extension/http.go | 7 +- pkg/cmd/extension/manager.go | 120 ++++++++++++++++++++++++++--------- pkg/extensions/extension.go | 4 +- 3 files changed, 97 insertions(+), 34 deletions(-) diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 6f93f2303..c5687a5f4 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -70,7 +70,12 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return api.HandleHTTPError(resp) } - f, err := os.OpenFile(destPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0755) + mode := os.O_WRONLY | os.O_CREATE | os.O_EXCL + if _, err := os.Stat(destPath); err == nil { + mode = os.O_WRONLY | os.O_EXCL + } + + f, err := os.OpenFile(destPath, mode, 0755) if err != nil { return err } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index d7c1cf670..8188b67ea 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "io/fs" "io/ioutil" "net/http" "os" @@ -105,7 +106,6 @@ func (m *Manager) List(includeMetadata bool) []extensions.Extension { } func (m *Manager) list(includeMetadata bool) ([]extensions.Extension, error) { - // TODO need to fix this to work with binary extensions before upgrade will work dir := m.installDir() entries, err := ioutil.ReadDir(dir) if err != nil { @@ -117,36 +117,96 @@ func (m *Manager) list(includeMetadata bool) ([]extensions.Extension, error) { if !strings.HasPrefix(f.Name(), "gh-") { continue } - var remoteUrl string - updateAvailable := false - isLocal := false - exePath := filepath.Join(dir, f.Name(), f.Name()) - if f.IsDir() { - if includeMetadata { - remoteUrl = m.getRemoteUrl(f.Name()) - updateAvailable = m.checkUpdateAvailable(f.Name()) - } - } else { - isLocal = true - if !isSymlink(f.Mode()) { - // if this is a regular file, its contents is the local directory of the extension - p, err := readPathFromFile(filepath.Join(dir, f.Name())) - if err != nil { - return nil, err - } - exePath = filepath.Join(p, f.Name()) - } + ext, err := m.parseExtensionDir(f, includeMetadata) + if err != nil { + return nil, err } - results = append(results, &Extension{ - path: exePath, - url: remoteUrl, - isLocal: isLocal, - updateAvailable: updateAvailable, - }) + results = append(results, ext) } + return results, nil } +func (m *Manager) parseExtensionDir(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { + id := m.installDir() + if _, err := os.Stat(filepath.Join(id, fi.Name(), manifestName)); err == nil { + return m.parseBinaryExtension(fi, includeMetadata) + } + + return m.parseGitExtension(fi, includeMetadata) +} + +func (m *Manager) parseBinaryExtension(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { + id := m.installDir() + exePath := filepath.Join(id, fi.Name(), fi.Name()) + manifestPath := filepath.Join(id, fi.Name(), manifestName) + manifest, err := os.ReadFile(manifestPath) + if err != nil { + return nil, fmt.Errorf("could not open %s for reading: %w", manifestPath, err) + } + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + if err != nil { + return nil, fmt.Errorf("could not parse %s: %w", manifestPath, err) + } + + repo := ghrepo.NewWithHost(bm.Owner, bm.Name, bm.Host) + + var remoteURL string + var updateAvailable bool + + if includeMetadata { + remoteURL = ghrepo.GenerateRepoURL(repo, "") + var r *release + r, err = fetchLatestRelease(m.client, repo) + if err != nil { + return nil, fmt.Errorf("failed to get release info for %s: %w", ghrepo.FullName(repo), err) + } + if bm.Tag != r.Tag { + updateAvailable = true + } + } + + return &Extension{ + path: exePath, + url: remoteURL, + updateAvailable: updateAvailable, + }, nil +} + +func (m *Manager) parseGitExtension(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { + // TODO untangle local from this since local might be binary or git + id := m.installDir() + var remoteUrl string + updateAvailable := false + isLocal := false + exePath := filepath.Join(id, fi.Name(), fi.Name()) + if fi.IsDir() { + if includeMetadata { + remoteUrl = m.getRemoteUrl(fi.Name()) + updateAvailable = m.checkUpdateAvailable(fi.Name()) + } + } else { + isLocal = true + if !isSymlink(fi.Mode()) { + // if this is a regular file, its contents is the local directory of the extension + p, err := readPathFromFile(filepath.Join(id, fi.Name())) + if err != nil { + return nil, err + } + exePath = filepath.Join(p, fi.Name()) + } + } + + return &Extension{ + path: exePath, + url: remoteUrl, + isLocal: isLocal, + updateAvailable: updateAvailable, + }, nil +} + func (m *Manager) getRemoteUrl(extension string) string { gitExe, err := m.lookPath("git") if err != nil { @@ -335,9 +395,8 @@ func (m *Manager) Upgrade(name string, force bool) error { continue } - _, err := os.Stat(filepath.Join(f.Path(), manifestName)) - if err == nil { - // TODO is there any need for a "forced" binary upgrade? + binManifestPath := filepath.Join(filepath.Dir(f.Path()), manifestName) + if _, e := os.Stat(binManifestPath); e == nil { err = m.upgradeBin(f) someUpgraded = true continue @@ -372,7 +431,7 @@ func (m *Manager) upgradeGit(ext extensions.Extension, exe string, force bool) e } func (m *Manager) upgradeBin(ext extensions.Extension) error { - manifestPath := filepath.Join(ext.Path(), manifestName) + manifestPath := filepath.Join(filepath.Dir(ext.Path()), manifestName) manifest, err := os.ReadFile(manifestPath) if err != nil { return fmt.Errorf("could not open %s for reading: %w", manifestPath, err) @@ -395,7 +454,6 @@ func (m *Manager) upgradeBin(ext extensions.Extension) error { return nil } - // TODO will this work? return m.installBin(repo) } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index c9f1472f9..f0962a87a 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -8,8 +8,8 @@ import ( //go:generate moq -rm -out extension_mock.go . Extension type Extension interface { - Name() string - Path() string + Name() string // Extension Name without gh- + Path() string // Path to executable URL() string IsLocal() bool UpdateAvailable() bool From 94778a9cb05013c7cb9813b536a5e57c283879a5 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 15:04:39 -0500 Subject: [PATCH 04/11] un-stupid the file mode --- pkg/cmd/extension/http.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index c5687a5f4..cfae2b738 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -70,12 +70,7 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return api.HandleHTTPError(resp) } - mode := os.O_WRONLY | os.O_CREATE | os.O_EXCL - if _, err := os.Stat(destPath); err == nil { - mode = os.O_WRONLY | os.O_EXCL - } - - f, err := os.OpenFile(destPath, mode, 0755) + f, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) if err != nil { return err } From 22c1778b9f1e443b107c8f342fc7d518d9c24a0e Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 15:04:45 -0500 Subject: [PATCH 05/11] TODOs --- pkg/cmd/extension/manager_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index a13933fb7..d742356bb 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -66,6 +66,10 @@ func TestManager_List(t *testing.T) { assert.Equal(t, "two", exts[1].Name()) } +func TestManager_List_Binary(t *testing.T) { + // TODO +} + func TestManager_Dispatch(t *testing.T) { tempDir := t.TempDir() extPath := filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello") @@ -199,6 +203,10 @@ func TestManager_Upgrade_NoExtensions(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_Upgrade_BinaryExtension(t *testing.T) { + // TODO +} + func TestManager_Install_git(t *testing.T) { tempDir := t.TempDir() From 541ed3ba6fb06ba6826c8c60d8f0ff81b99cd43d Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 15:33:11 -0500 Subject: [PATCH 06/11] test Upgrade with binary exts --- pkg/cmd/extension/manager_test.go | 96 ++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index d742356bb..64a90da3f 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -204,7 +204,101 @@ func TestManager_Upgrade_NoExtensions(t *testing.T) { } func TestManager_Upgrade_BinaryExtension(t *testing.T) { - // TODO + tempDir := t.TempDir() + + installReg := httpmock.Registry{} + defer installReg.Verify(t) + installClient := http.Client{Transport: &installReg} + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, &installClient, io) + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + installReg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64", + APIURL: "https://example.com/release/cool", + }, + }, + })) + installReg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.1", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64", + APIURL: "https://example.com/release/cool", + }, + }, + })) + installReg.Register( + httpmock.REST("GET", "release/cool"), + httpmock.StringResponse("FAKE BINARY")) + + err := m.Install(repo) + assert.NoError(t, err) + + upgradeReg := httpmock.Registry{} + defer upgradeReg.Verify(t) + upgradeClient := http.Client{Transport: &upgradeReg} + + um := newTestManager(tempDir, &upgradeClient, io) + upgradeReg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.2", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64", + APIURL: "https://example.com/release/cool2", + }, + }, + })) + upgradeReg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.2", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64", + APIURL: "https://example.com/release/cool2", + }, + }, + })) + upgradeReg.Register( + httpmock.REST("GET", "release/cool2"), + httpmock.StringResponse("FAKE UPGRADED BINARY")) + + err = um.Upgrade("bin-ext", false) + 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.0.2", + Path: filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext")) + assert.NoError(t, err) + + assert.Equal(t, "FAKE UPGRADED BINARY", string(fakeBin)) } func TestManager_Install_git(t *testing.T) { From 392460b81e4445962c57b988d5e9ffce8ffcf598 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 15:45:49 -0500 Subject: [PATCH 07/11] WIP switching to stubBinaryExtension --- pkg/cmd/extension/manager_test.go | 68 +++++++++++-------------------- 1 file changed, 23 insertions(+), 45 deletions(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 64a90da3f..9df4ce4b2 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -67,7 +67,9 @@ func TestManager_List(t *testing.T) { } func TestManager_List_Binary(t *testing.T) { + //tempDir := t.TempDir() // TODO + } func TestManager_Dispatch(t *testing.T) { @@ -206,50 +208,19 @@ func TestManager_Upgrade_NoExtensions(t *testing.T) { func TestManager_Upgrade_BinaryExtension(t *testing.T) { tempDir := t.TempDir() - installReg := httpmock.Registry{} - defer installReg.Verify(t) - installClient := http.Client{Transport: &installReg} - io, _, _, _ := iostreams.Test() - m := newTestManager(tempDir, &installClient, io) - repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + stubBinaryExtension(filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.1", + }) - installReg.Register( - httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), - httpmock.JSONResponse( - release{ - Assets: []releaseAsset{ - { - Name: "gh-bin-ext-windows-amd64", - APIURL: "https://example.com/release/cool", - }, - }, - })) - installReg.Register( - httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), - httpmock.JSONResponse( - release{ - Tag: "v1.0.1", - Assets: []releaseAsset{ - { - Name: "gh-bin-ext-windows-amd64", - APIURL: "https://example.com/release/cool", - }, - }, - })) - installReg.Register( - httpmock.REST("GET", "release/cool"), - httpmock.StringResponse("FAKE BINARY")) - - err := m.Install(repo) - assert.NoError(t, err) - - upgradeReg := httpmock.Registry{} - defer upgradeReg.Verify(t) - upgradeClient := http.Client{Transport: &upgradeReg} - - um := newTestManager(tempDir, &upgradeClient, io) - upgradeReg.Register( + m := newTestManager(tempDir, &client, io) + reg.Register( httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), httpmock.JSONResponse( release{ @@ -261,7 +232,7 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { }, }, })) - upgradeReg.Register( + reg.Register( httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), httpmock.JSONResponse( release{ @@ -273,11 +244,11 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { }, }, })) - upgradeReg.Register( + reg.Register( httpmock.REST("GET", "release/cool2"), httpmock.StringResponse("FAKE UPGRADED BINARY")) - err = um.Upgrade("bin-ext", false) + err := m.Upgrade("bin-ext", false) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -500,3 +471,10 @@ func stubLocalExtension(tempDir, path string) error { } return f.Close() } + +func stubBinaryExtension(path string, bm binManifest) error { + // TODO make directory + // TODO write manifest file + // TODO write fake binary file + return nil +} From 3971df4f93b6b7a9eefe6e27cf22c1e51bed8870 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 19:23:28 -0500 Subject: [PATCH 08/11] switch to stubBinaryExtension --- pkg/cmd/extension/manager_test.go | 42 +++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 9df4ce4b2..c5db207b1 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -212,12 +212,13 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { reg := httpmock.Registry{} defer reg.Verify(t) client := http.Client{Transport: ®} - stubBinaryExtension(filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ + err := stubBinaryExtension(filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ Owner: "owner", Name: "gh-bin-ext", Host: "example.com", Tag: "v1.0.1", }) + assert.NoError(t, err) m := newTestManager(tempDir, &client, io) reg.Register( @@ -248,7 +249,7 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { httpmock.REST("GET", "release/cool2"), httpmock.StringResponse("FAKE UPGRADED BINARY")) - err := m.Upgrade("bin-ext", false) + err = m.Upgrade("bin-ext", false) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -472,9 +473,36 @@ func stubLocalExtension(tempDir, path string) error { return f.Close() } -func stubBinaryExtension(path string, bm binManifest) error { - // TODO make directory - // TODO write manifest file - // TODO write fake binary file - return nil +// Given the path where an extension should be installed and a manifest struct, creates a fake binary extension on disk +func stubBinaryExtension(installPath string, bm binManifest) error { + if err := os.MkdirAll(installPath, 0755); err != nil { + return err + } + fakeBinaryPath := filepath.Join(installPath, filepath.Base(installPath)) + fb, err := os.OpenFile(fakeBinaryPath, os.O_CREATE, 0755) + if err != nil { + return err + } + err = fb.Close() + if err != nil { + return err + } + + bs, err := yaml.Marshal(bm) + if err != nil { + return fmt.Errorf("failed to serialize manifest: %w", err) + } + + manifestPath := filepath.Join(installPath, manifestName) + + fm, err := os.OpenFile(manifestPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("failed to open manifest for writing: %w", err) + } + _, err = fm.Write(bs) + if err != nil { + return fmt.Errorf("failed write manifest file: %w", err) + } + + return fm.Close() } From ef087123544b57251dcff77b5b7a760556af9433 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 28 Sep 2021 19:27:21 -0500 Subject: [PATCH 09/11] test list --- pkg/cmd/extension/manager_test.go | 40 ++++++++++++++++++------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index c5db207b1..cb9140177 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -59,17 +59,21 @@ func TestManager_List(t *testing.T) { assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.1", + })) + m := newTestManager(tempDir, nil, nil) exts := m.List(false) - assert.Equal(t, 2, len(exts)) - assert.Equal(t, "hello", exts[0].Name()) - assert.Equal(t, "two", exts[1].Name()) -} - -func TestManager_List_Binary(t *testing.T) { - //tempDir := t.TempDir() - // TODO - + assert.Equal(t, 3, len(exts)) + assert.Equal(t, "bin-ext", exts[0].Name()) + assert.Equal(t, "hello", exts[1].Name()) + assert.Equal(t, "two", exts[2].Name()) } func TestManager_Dispatch(t *testing.T) { @@ -212,13 +216,15 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { reg := httpmock.Registry{} defer reg.Verify(t) client := http.Client{Transport: ®} - err := stubBinaryExtension(filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ - Owner: "owner", - Name: "gh-bin-ext", - Host: "example.com", - Tag: "v1.0.1", - }) - assert.NoError(t, err) + + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.0.1", + })) m := newTestManager(tempDir, &client, io) reg.Register( @@ -249,7 +255,7 @@ func TestManager_Upgrade_BinaryExtension(t *testing.T) { httpmock.REST("GET", "release/cool2"), httpmock.StringResponse("FAKE UPGRADED BINARY")) - err = m.Upgrade("bin-ext", false) + err := m.Upgrade("bin-ext", false) assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) From a2e7eaf80878e12f23edde21bfb054a44af23e52 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 29 Sep 2021 13:38:57 -0500 Subject: [PATCH 10/11] test update available for binary ext in list --- pkg/cmd/extension/manager_test.go | 38 +++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index cb9140177..3d613633b 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -76,6 +76,44 @@ func TestManager_List(t *testing.T) { assert.Equal(t, "two", exts[2].Name()) } +func TestManager_List_binary_update(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.0.1", + })) + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.2", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64", + APIURL: "https://example.com/release/cool2", + }, + }, + })) + + m := newTestManager(tempDir, &client, nil) + + exts := m.List(true) + assert.Equal(t, 1, len(exts)) + assert.Equal(t, "bin-ext", exts[0].Name()) + assert.True(t, exts[0].UpdateAvailable()) + assert.Equal(t, "https://example.com/owner/gh-bin-ext", exts[0].URL()) +} + func TestManager_Dispatch(t *testing.T) { tempDir := t.TempDir() extPath := filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello") From 7efd06b87dd5210cbb079c377fff08c0ae75c8aa Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 29 Sep 2021 13:43:37 -0500 Subject: [PATCH 11/11] rename function --- pkg/cmd/extension/manager.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 8188b67ea..f9ab502d1 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -130,13 +130,13 @@ func (m *Manager) list(includeMetadata bool) ([]extensions.Extension, error) { func (m *Manager) parseExtensionDir(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { id := m.installDir() if _, err := os.Stat(filepath.Join(id, fi.Name(), manifestName)); err == nil { - return m.parseBinaryExtension(fi, includeMetadata) + return m.parseBinaryExtensionDir(fi, includeMetadata) } - return m.parseGitExtension(fi, includeMetadata) + return m.parseGitExtensionDir(fi, includeMetadata) } -func (m *Manager) parseBinaryExtension(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { +func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { id := m.installDir() exePath := filepath.Join(id, fi.Name(), fi.Name()) manifestPath := filepath.Join(id, fi.Name(), manifestName) @@ -175,7 +175,7 @@ func (m *Manager) parseBinaryExtension(fi fs.FileInfo, includeMetadata bool) (*E }, nil } -func (m *Manager) parseGitExtension(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { +func (m *Manager) parseGitExtensionDir(fi fs.FileInfo, includeMetadata bool) (*Extension, error) { // TODO untangle local from this since local might be binary or git id := m.installDir() var remoteUrl string