Merge pull request #4588 from cli/bin-ext-migrate

binary extension migration

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 {

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)

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",

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: &reg}
+	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()