From 54b82dd0726b20eff43927b43b21b64b22b9e3f3 Mon Sep 17 00:00:00 2001 From: nate smith Date: Thu, 21 Oct 2021 14:19:36 -0500 Subject: [PATCH] Migrate to binary style extensions when detected --- git/git.go | 9 ++++ git/remote.go | 28 +++++++---- pkg/cmd/extension/manager.go | 41 +++++++++++++++- pkg/cmd/extension/manager_test.go | 78 +++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 9 deletions(-) diff --git a/git/git.go b/git/git.go index 97dafa1c7..3cf1f281f 100644 --- a/git/git.go +++ b/git/git.go @@ -84,6 +84,15 @@ func CurrentBranch() (string, error) { return "", fmt.Errorf("%sgit: %s", stderr.String(), err) } +func listRemotesForPath(path string) ([]string, error) { + remoteCmd, err := GitCommand("-C", path, "remote", "-v") + if err != nil { + return nil, err + } + output, err := run.PrepareCmd(remoteCmd).Output() + return outputLines(output), err +} + func listRemotes() ([]string, error) { remoteCmd, err := GitCommand("remote", "-v") if err != nil { diff --git a/git/remote.go b/git/remote.go index 91bbb6770..bea81da90 100644 --- a/git/remote.go +++ b/git/remote.go @@ -35,16 +35,11 @@ func (r *Remote) String() string { return r.Name } -// Remotes gets the git remotes set for the current repo -func Remotes() (RemoteSet, error) { - list, err := listRemotes() - if err != nil { - return nil, err - } - remotes := parseRemotes(list) +func remotes(path string, remoteList []string) (RemoteSet, error) { + remotes := parseRemotes(remoteList) // this is affected by SetRemoteResolution - remoteCmd, err := GitCommand("config", "--get-regexp", `^remote\..*\.gh-resolved$`) + remoteCmd, err := GitCommand("-C", path, "config", "--get-regexp", `^remote\..*\.gh-resolved$`) if err != nil { return nil, err } @@ -70,6 +65,23 @@ func Remotes() (RemoteSet, error) { return remotes, nil } +func RemotesForPath(path string) (RemoteSet, error) { + list, err := listRemotesForPath(path) + if err != nil { + return nil, err + } + return remotes(path, list) +} + +// Remotes gets the git remotes set for the current repo +func Remotes() (RemoteSet, error) { + list, err := listRemotes() + if err != nil { + return nil, err + } + return remotes(".", list) +} + func parseRemotes(gitRemotes []string) (remotes RemoteSet) { for _, r := range gitRemotes { match := remoteRE.FindStringSubmatch(r) diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 914f5f8be..5d6764a8f 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -18,6 +18,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/extensions" @@ -333,7 +334,6 @@ func (m *Manager) Install(repo ghrepo.Interface) error { return err } if !hs { - // TODO open an issue hint, here? return errors.New("extension is uninstallable: missing executable") } @@ -487,6 +487,19 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.IsBinary() { err = m.upgradeBinExtension(ext) } else { + // Check if git extension has changed to a binary extension + var isBin bool + repo, repoErr := repoFromPath(filepath.Join(ext.Path(), "..")) + if repoErr == nil { + isBin, _ = isBinExtension(m.client, repo) + } + if isBin { + err = m.Remove(ext.Name()) + if err != nil { + return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) + } + return m.installBin(repo) + } err = m.upgradeGitExtension(ext, force) } return err @@ -654,6 +667,32 @@ func isBinExtension(client *http.Client, repo ghrepo.Interface) (isBin bool, err return } +func repoFromPath(path string) (ghrepo.Interface, error) { + remotes, err := git.RemotesForPath(path) + if err != nil { + return nil, err + } + + if len(remotes) == 0 { + return nil, fmt.Errorf("no remotes configured for %s", path) + } + + var remote *git.Remote + + for _, r := range remotes { + if r.Name == "origin" { + remote = r + break + } + } + + if remote == nil { + remote = remotes[0] + } + + return ghrepo.FromURL(remote.FetchURL) +} + func possibleDists() []string { return []string{ "aix-ppc64", diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 2e13345ff..70bbfed42 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -14,6 +14,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/run" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" "github.com/stretchr/testify/assert" @@ -297,6 +298,83 @@ func TestManager_UpgradeExtension_GitExtension_Force(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_MigrateToBinaryExtension(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExtension(filepath.Join(tempDir, "extensions", "gh-remote", "gh-remote"))) + io, _, stdout, stderr := iostreams.Test() + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + m := newTestManager(tempDir, &client, io) + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + ext.currentVersion = "old version" + ext.latestVersion = "new version" + + rs, restoreRun := run.Stub() + defer restoreRun(t) + + rs.Register(`git -C.*?gh-remote remote -v`, 0, "origin git@github.com:owner/gh-remote.git (fetch)\norigin git@github.com:owner/gh-remote.git (push)") + rs.Register(`git -C.*?gh-remote config --get-regexp \^.*`, 0, "remote.origin.gh-resolve base") + + reg.Register( + httpmock.REST("GET", "repos/owner/gh-remote/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.2", + Assets: []releaseAsset{ + { + Name: "gh-remote-windows-amd64.exe", + APIURL: "/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-remote/releases/latest"), + httpmock.JSONResponse( + release{ + Tag: "v1.0.2", + Assets: []releaseAsset{ + { + Name: "gh-remote-windows-amd64.exe", + APIURL: "/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/cool"), + httpmock.StringResponse("FAKE UPGRADED BINARY")) + + err = m.upgradeExtension(ext, false) + assert.NoError(t, err) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + + manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-remote", manifestName)) + assert.NoError(t, err) + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + assert.NoError(t, err) + + assert.Equal(t, binManifest{ + Name: "gh-remote", + Owner: "owner", + Host: "github.com", + Tag: "v1.0.2", + Path: filepath.Join(tempDir, "extensions/gh-remote/gh-remote.exe"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-remote/gh-remote.exe")) + assert.NoError(t, err) + + assert.Equal(t, "FAKE UPGRADED BINARY", string(fakeBin)) +} + func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { tempDir := t.TempDir() io, _, _, _ := iostreams.Test()