From e9a8b7f78125b3aebb927a404924f20cbdf5da7b Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 30 Jun 2021 12:36:52 -0700 Subject: [PATCH 1/4] Skip trying to upgrade local extensions --- pkg/cmd/extensions/manager.go | 26 ++++++++++ pkg/cmd/extensions/manager_test.go | 83 ++++++++++++++++++++++++++---- 2 files changed, 100 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go index 09520a930..182666d80 100644 --- a/pkg/cmd/extensions/manager.go +++ b/pkg/cmd/extensions/manager.go @@ -136,6 +136,8 @@ func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { return externalCmd.Run() } +var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") + func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { exe, err := m.lookPath("git") if err != nil { @@ -154,7 +156,31 @@ func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { } else if f.Name() != name { continue } + dir := filepath.Dir(f.Path()) + fileInfo, e := os.Lstat(dir) + if e != nil { + err = e + if name == "" { + fmt.Fprintf(stdout, "%s\n", err) + } + continue + } + if fileInfo.Mode().Type() == os.ModeSymlink { + err = localExtensionUpgradeError + if name == "" { + fmt.Fprintf(stdout, "%s\n", err) + } + continue + } + if _, err = os.Stat(filepath.Join(dir, ".git")); err != nil { + err = localExtensionUpgradeError + if name == "" { + fmt.Fprintf(stdout, "%s\n", err) + } + continue + } + externalCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") externalCmd.Stdout = stdout externalCmd.Stderr = stderr diff --git a/pkg/cmd/extensions/manager_test.go b/pkg/cmd/extensions/manager_test.go index d829b156a..693bfed89 100644 --- a/pkg/cmd/extensions/manager_test.go +++ b/pkg/cmd/extensions/manager_test.go @@ -44,8 +44,8 @@ func newTestManager(dir string) *Manager { func TestManager_List(t *testing.T) { tempDir := t.TempDir() - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) m := newTestManager(tempDir) exts := m.List() @@ -57,7 +57,7 @@ func TestManager_List(t *testing.T) { func TestManager_Dispatch(t *testing.T) { tempDir := t.TempDir() extPath := filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello") - assert.NoError(t, stubExecutable(extPath)) + assert.NoError(t, stubExtension(extPath)) m := newTestManager(tempDir) @@ -77,8 +77,8 @@ func TestManager_Dispatch(t *testing.T) { func TestManager_Remove(t *testing.T) { tempDir := t.TempDir() - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) m := newTestManager(tempDir) err := m.Remove("hello") @@ -90,10 +90,11 @@ func TestManager_Remove(t *testing.T) { assert.Equal(t, "gh-two", items[0].Name()) } -func TestManager_Upgrade(t *testing.T) { +func TestManager_Upgrade_AllExtensions(t *testing.T) { tempDir := t.TempDir() - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) - assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + 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, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) m := newTestManager(tempDir) @@ -105,6 +106,7 @@ func TestManager_Upgrade(t *testing.T) { assert.Equal(t, heredoc.Docf( ` [hello]: [git -C %s --git-dir=%s pull --ff-only] + [local]: local extensions can not be upgraded [two]: [git -C %s --git-dir=%s pull --ff-only] `, filepath.Join(tempDir, "extensions", "gh-hello"), @@ -115,6 +117,53 @@ func TestManager_Upgrade(t *testing.T) { assert.Equal(t, "", stderr.String()) } +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) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("remote", stdout, stderr) + assert.NoError(t, err) + assert.Equal(t, heredoc.Docf( + ` + [git -C %s --git-dir=%s pull --ff-only] + `, + filepath.Join(tempDir, "extensions", "gh-remote"), + filepath.Join(tempDir, "extensions", "gh-remote", ".git"), + ), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Upgrade_LocalExtension(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubLocalExtension(filepath.Join(tempDir, "extensions", "gh-local", "gh-local"))) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("local", stdout, stderr) + assert.EqualError(t, err, "local extensions can not be upgraded") + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Upgrade_NoExtensions(t *testing.T) { + tempDir := t.TempDir() + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("", stdout, stderr) + assert.EqualError(t, err, "no extensions installed") + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) +} + func TestManager_Install(t *testing.T) { tempDir := t.TempDir() m := newTestManager(tempDir) @@ -127,7 +176,23 @@ func TestManager_Install(t *testing.T) { assert.Equal(t, "", stderr.String()) } -func stubExecutable(path string) error { +func stubExtension(path string) error { + dir := filepath.Dir(path) + gitDir := filepath.Join(dir, ".git") + if err := os.MkdirAll(dir, 0755); err != nil { + return err + } + if err := os.Mkdir(gitDir, 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + return f.Close() +} + +func stubLocalExtension(path string) error { if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { return err } From d19c348d7af23528f5d2f7cc13746e8d5c61e233 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Thu, 1 Jul 2021 10:14:29 -0700 Subject: [PATCH 2/4] Rework code for go 1.15 --- pkg/cmd/extensions/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go index 182666d80..9563b74cc 100644 --- a/pkg/cmd/extensions/manager.go +++ b/pkg/cmd/extensions/manager.go @@ -166,7 +166,7 @@ func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { } continue } - if fileInfo.Mode().Type() == os.ModeSymlink { + if fileInfo.Mode()&os.ModeSymlink != 0 { err = localExtensionUpgradeError if name == "" { fmt.Fprintf(stdout, "%s\n", err) From bc7eaf8004c542a8740f5949334e2d06b107054c Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Thu, 1 Jul 2021 11:19:25 -0700 Subject: [PATCH 3/4] Add IsLocal helper func --- pkg/cmd/extensions/extension.go | 18 ++++++++++++++ pkg/cmd/extensions/manager.go | 24 ++++-------------- pkg/extensions/extension.go | 1 + pkg/extensions/extension_mock.go | 42 +++++++++++++++++++++++++++++--- 4 files changed, 63 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/extensions/extension.go b/pkg/cmd/extensions/extension.go index 33b2f8d67..d73bf4bc0 100644 --- a/pkg/cmd/extensions/extension.go +++ b/pkg/cmd/extensions/extension.go @@ -1,6 +1,7 @@ package extensions import ( + "os" "path/filepath" "strings" ) @@ -21,3 +22,20 @@ func (e *Extension) Path() string { func (e *Extension) URL() string { return e.url } + +func (e *Extension) IsLocal() bool { + dir := filepath.Dir(e.path) + fileInfo, err := os.Lstat(dir) + if err != nil { + return false + } + // Check if extension is a symlink + if fileInfo.Mode()&os.ModeSymlink != 0 { + return true + } + // Check if extension does not have a git directory + if _, err = os.Stat(filepath.Join(dir, ".git")); err != nil { + return true + } + return false +} diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go index 9563b74cc..ff6452657 100644 --- a/pkg/cmd/extensions/manager.go +++ b/pkg/cmd/extensions/manager.go @@ -157,30 +157,16 @@ func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { continue } - dir := filepath.Dir(f.Path()) - fileInfo, e := os.Lstat(dir) - if e != nil { - err = e + if f.IsLocal() { if name == "" { - fmt.Fprintf(stdout, "%s\n", err) - } - continue - } - if fileInfo.Mode()&os.ModeSymlink != 0 { - err = localExtensionUpgradeError - if name == "" { - fmt.Fprintf(stdout, "%s\n", err) - } - continue - } - if _, err = os.Stat(filepath.Join(dir, ".git")); err != nil { - err = localExtensionUpgradeError - if name == "" { - fmt.Fprintf(stdout, "%s\n", err) + fmt.Fprintf(stdout, "%s\n", localExtensionUpgradeError) + } else { + err = localExtensionUpgradeError } continue } + dir := filepath.Dir(f.Path()) externalCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") externalCmd.Stdout = stdout externalCmd.Stderr = stderr diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index c0d65e297..da915463a 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -9,6 +9,7 @@ type Extension interface { Name() string Path() string URL() string + IsLocal() bool } //go:generate moq -out manager_mock.go . ExtensionManager diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index fb32db5b1..8fe35c1f6 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -17,6 +17,9 @@ var _ Extension = &ExtensionMock{} // // // make and configure a mocked Extension // mockedExtension := &ExtensionMock{ +// IsLocalFunc: func() bool { +// panic("mock out the IsLocal method") +// }, // NameFunc: func() string { // panic("mock out the Name method") // }, @@ -33,6 +36,9 @@ var _ Extension = &ExtensionMock{} // // } type ExtensionMock struct { + // IsLocalFunc mocks the IsLocal method. + IsLocalFunc func() bool + // NameFunc mocks the Name method. NameFunc func() string @@ -44,6 +50,9 @@ type ExtensionMock struct { // calls tracks calls to the methods. calls struct { + // IsLocal holds details about calls to the IsLocal method. + IsLocal []struct { + } // Name holds details about calls to the Name method. Name []struct { } @@ -54,9 +63,36 @@ type ExtensionMock struct { URL []struct { } } - lockName sync.RWMutex - lockPath sync.RWMutex - lockURL sync.RWMutex + lockIsLocal sync.RWMutex + lockName sync.RWMutex + lockPath sync.RWMutex + lockURL sync.RWMutex +} + +// IsLocal calls IsLocalFunc. +func (mock *ExtensionMock) IsLocal() bool { + if mock.IsLocalFunc == nil { + panic("ExtensionMock.IsLocalFunc: method is nil but Extension.IsLocal was just called") + } + callInfo := struct { + }{} + mock.lockIsLocal.Lock() + mock.calls.IsLocal = append(mock.calls.IsLocal, callInfo) + mock.lockIsLocal.Unlock() + return mock.IsLocalFunc() +} + +// IsLocalCalls gets all the calls that were made to IsLocal. +// Check the length with: +// len(mockedExtension.IsLocalCalls()) +func (mock *ExtensionMock) IsLocalCalls() []struct { +} { + var calls []struct { + } + mock.lockIsLocal.RLock() + calls = mock.calls.IsLocal + mock.lockIsLocal.RUnlock() + return calls } // Name calls NameFunc. From b71735d0d773585a6451dd2c3a9705234c22edb4 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 19 Jul 2021 10:48:09 -0700 Subject: [PATCH 4/4] Address PR comments --- pkg/cmd/extensions/extension.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/extensions/extension.go b/pkg/cmd/extensions/extension.go index d73bf4bc0..8fa07e7cf 100644 --- a/pkg/cmd/extensions/extension.go +++ b/pkg/cmd/extensions/extension.go @@ -1,6 +1,7 @@ package extensions import ( + "errors" "os" "path/filepath" "strings" @@ -34,7 +35,7 @@ func (e *Extension) IsLocal() bool { return true } // Check if extension does not have a git directory - if _, err = os.Stat(filepath.Join(dir, ".git")); err != nil { + if _, err = os.Stat(filepath.Join(dir, ".git")); errors.Is(err, os.ErrNotExist) { return true } return false