diff --git a/.changes/unreleased/Fixed-20240601-151223.yaml b/.changes/unreleased/Fixed-20240601-151223.yaml new file mode 100644 index 00000000..22578a0b --- /dev/null +++ b/.changes/unreleased/Fixed-20240601-151223.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: GitHub URL detection now respects non-standard URLs like GHES set via `$GITHUB_URL`. +time: 2024-06-01T15:12:23.588802-07:00 diff --git a/branch_submit.go b/branch_submit.go index 00713b6a..b988b4ff 100644 --- a/branch_submit.go +++ b/branch_submit.go @@ -7,14 +7,13 @@ import ( "strings" "github.com/charmbracelet/log" - "github.com/google/go-github/v61/github" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/git" "go.abhg.dev/gs/internal/must" "go.abhg.dev/gs/internal/spice" "go.abhg.dev/gs/internal/spice/state" "go.abhg.dev/gs/internal/text" "go.abhg.dev/gs/internal/ui" - "golang.org/x/oauth2" ) type branchSubmitCmd struct { @@ -52,7 +51,7 @@ func (cmd *branchSubmitCmd) Run( ctx context.Context, log *log.Logger, opts *globalOptions, - tokenSource oauth2.TokenSource, + ghBuilder *github.Builder, ) error { repo, err := git.Open(ctx, ".", git.OpenOptions{ Log: log, @@ -108,25 +107,21 @@ func (cmd *branchSubmitCmd) Run( return err } - ghrepo, err := ensureGitHubRepo(ctx, log, repo, remote) + forge, err := ensureGitHubForge(ctx, log, ghBuilder, repo, remote) if err != nil { return err } - gh, err := newGitHubClient(ctx, tokenSource, opts) - if err != nil { - return fmt.Errorf("create GitHub client: %w", err) - } - pulls, _, err := gh.PullRequests.List(ctx, ghrepo.Owner, ghrepo.Name, &github.PullRequestListOptions{ - State: "open", - Head: ghrepo.Owner + ":" + upstreamBranch, - // Don't filter by base -- we may need to update it. + changes, err := forge.ListChanges(ctx, github.ListChangesOptions{ + State: "open", + Branch: upstreamBranch, + // We don't filter by base here as that may be out of date. }) if err != nil { - return fmt.Errorf("list pull requests: %w", err) + return fmt.Errorf("list changes: %w", err) } - switch len(pulls) { + switch len(changes) { case 0: if cmd.DryRun { log.Infof("WOULD create a pull request for %s", cmd.Name) @@ -251,48 +246,48 @@ func (cmd *branchSubmitCmd) Run( log.Warn("Could not set upstream", "branch", cmd.Name, "remote", remote, "error", err) } - pull, _, err := gh.PullRequests.Create(ctx, ghrepo.Owner, ghrepo.Name, &github.NewPullRequest{ - Title: &cmd.Title, - Body: &cmd.Body, - Head: &cmd.Name, - Base: &branch.Base, - Draft: &cmd.Draft, + result, err := forge.SubmitChange(ctx, github.SubmitChangeRequest{ + Subject: cmd.Title, + Body: cmd.Body, + Head: cmd.Name, + Base: branch.Base, + Draft: cmd.Draft, }) if err != nil { - return fmt.Errorf("create pull request: %w", err) + return fmt.Errorf("create change: %w", err) } - upsert.PR = pull.GetNumber() + upsert.PR = int(result.ID) - log.Infof("Created #%d: %s", pull.GetNumber(), pull.GetHTMLURL()) + log.Infof("Created %v: %s", result.ID, result.URL) case 1: // Check base and HEAD are up-to-date. - pull := pulls[0] + pull := changes[0] var updates []string - if pull.Head.GetSHA() != commitHash.String() { + if pull.HeadHash != commitHash { updates = append(updates, "push branch") } - if pull.Base.GetRef() != branch.Base { + if pull.BaseName != branch.Base { updates = append(updates, "set base to "+branch.Base) } - if pull.GetDraft() != cmd.Draft { + if pull.Draft != cmd.Draft { updates = append(updates, "set draft to "+fmt.Sprint(cmd.Draft)) } if len(updates) == 0 { - log.Infof("Pull request #%d is up-to-date", pull.GetNumber()) + log.Infof("Pull request %v is up-to-date", pull.ID) return nil } if cmd.DryRun { - log.Infof("WOULD update PR #%d:", pull.GetNumber()) + log.Infof("WOULD update PR %v:", pull.ID) for _, update := range updates { log.Infof(" - %s", update) } return nil } - if pull.Head.GetSHA() != commitHash.String() { + if pull.HeadHash != commitHash { err := repo.Push(ctx, git.PushOptions{ Remote: remote, Refspec: git.Refspec( @@ -300,7 +295,7 @@ func (cmd *branchSubmitCmd) Run( ), // Force push, but only if the ref is exactly // where we think it is. - ForceWithLease: cmd.Name + ":" + pull.Head.GetSHA(), + ForceWithLease: cmd.Name + ":" + pull.HeadHash.String(), }) if err != nil { log.Error("Branch may have been updated by someone else.") @@ -308,19 +303,20 @@ func (cmd *branchSubmitCmd) Run( } } - if pull.Base.GetRef() != branch.Base || pull.GetDraft() != cmd.Draft { - _, _, err := gh.PullRequests.Edit(ctx, ghrepo.Owner, ghrepo.Name, pull.GetNumber(), &github.PullRequest{ - Base: &github.PullRequestBranch{ - Ref: &branch.Base, - }, - Draft: &cmd.Draft, - }) - if err != nil { - return fmt.Errorf("update PR #%d: %w", pull.GetNumber(), err) + if pull.BaseName != branch.Base || pull.Draft != cmd.Draft { + opts := github.EditChangeOptions{ + Base: branch.Base, + } + if pull.Draft != cmd.Draft { + opts.Draft = &cmd.Draft + } + + if err := forge.EditChange(ctx, pull.ID, opts); err != nil { + return fmt.Errorf("edit PR %v: %w", pull.ID, err) } } - log.Infof("Updated #%d: %s", pull.GetNumber(), pull.GetHTMLURL()) + log.Infof("Updated %v: %s", pull.ID, pull.URL) default: // TODO: add a --pr flag to allow picking a PR? diff --git a/downstack_submit.go b/downstack_submit.go index 4b6b3005..641fee02 100644 --- a/downstack_submit.go +++ b/downstack_submit.go @@ -7,11 +7,11 @@ import ( "slices" "github.com/charmbracelet/log" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/git" "go.abhg.dev/gs/internal/must" "go.abhg.dev/gs/internal/spice" "go.abhg.dev/gs/internal/text" - "golang.org/x/oauth2" ) type downstackSubmitCmd struct { @@ -37,7 +37,7 @@ func (cmd *downstackSubmitCmd) Run( ctx context.Context, log *log.Logger, opts *globalOptions, - tokenSource oauth2.TokenSource, + ghBuilder *github.Builder, ) error { repo, err := git.Open(ctx, ".", git.OpenOptions{ Log: log, @@ -79,7 +79,7 @@ func (cmd *downstackSubmitCmd) Run( DryRun: cmd.DryRun, Fill: cmd.Fill, Name: downstack, - }).Run(ctx, log, opts, tokenSource) + }).Run(ctx, log, opts, ghBuilder) if err != nil { return fmt.Errorf("submit %v: %w", downstack, err) } diff --git a/gh.go b/gh.go index afa4c2ec..1e3a0474 100644 --- a/gh.go +++ b/gh.go @@ -2,47 +2,35 @@ package main import ( "context" + "errors" "fmt" "github.com/charmbracelet/log" - "github.com/google/go-github/v61/github" - "go.abhg.dev/gs/internal/gh" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/git" - "golang.org/x/oauth2" ) -func ensureGitHubRepo( +func ensureGitHubForge( ctx context.Context, log *log.Logger, + builder *github.Builder, repo *git.Repository, remote string, -) (gh.RepoInfo, error) { +) (*github.Forge, error) { remoteURL, err := repo.RemoteURL(ctx, remote) if err != nil { - return gh.RepoInfo{}, fmt.Errorf("get remote URL: %w", err) + return nil, fmt.Errorf("get remote URL: %w", err) } - // TODO: Take GITHUB_GIT_URL into account for ParseRepoInfo. - ghrepo, err := gh.ParseRepoInfo(remoteURL) + forge, err := builder.New(ctx, remoteURL) if err != nil { - log.Error("Could not guess GitHub repository from remote URL", "url", remoteURL) - log.Error("Are you sure the remote is a GitHub repository?") - return gh.RepoInfo{}, err - } - - return ghrepo, nil -} - -// TODO: move this into gh package -func newGitHubClient(ctx context.Context, tokenSource oauth2.TokenSource, opts *globalOptions) (*github.Client, error) { - gh := github.NewClient(oauth2.NewClient(ctx, tokenSource)) - if opts.GithubAPIURL != "" { - var err error - gh, err = gh.WithEnterpriseURLs(opts.GithubAPIURL, gh.UploadURL.String()) - if err != nil { - return nil, fmt.Errorf("set GitHub API URL: %w", err) + if errors.Is(err, github.ErrUnsupportedURL) { + log.Error("Could not guess GitHub repository from remote URL", "url", remoteURL) + log.Error("Are you sure the remote is a GitHub repository?") + return nil, err } + return nil, fmt.Errorf("build GitHub Forge: %w", err) } - return gh, nil + return forge, nil } diff --git a/internal/forge/github/build.go b/internal/forge/github/build.go new file mode 100644 index 00000000..9c1be47e --- /dev/null +++ b/internal/forge/github/build.go @@ -0,0 +1,147 @@ +package github + +import ( + "context" + "errors" + "fmt" + "io" + "net/url" + "strings" + + "github.com/charmbracelet/log" + "github.com/google/go-github/v61/github" + "golang.org/x/oauth2" +) + +const ( + // DefaultBaseURL is the default URL for GitHub. + DefaultBaseURL = "https://github.com" + + // DefaultAPIURL is the default URL for the GitHub API. + DefaultAPIURL = "https://api.github.com" +) + +// Builder builds a GitHub Forge. +type Builder struct { + // URL is the URL for GitHub. + // Override this for testing or GitHub Enterprise. + URL string + + // APIURL is the URL for the GitHub API. + // Override this for testing or GitHub Enterprise. + APIURL string + + // Token is the OAuth2 token source to use + // to authenticate with GitHub. + Token oauth2.TokenSource + + // Log specifies the logger to use. + Log *log.Logger +} + +// ErrUnsupportedURL is returned when the given URL is not a valid GitHub URL. +var ErrUnsupportedURL = errors.New("unsupported URL") + +// New builds a new GitHub Forge from the given remote URL. +// +// Returns [ErrUnsupportedURL] if the URL is not a valid GitHub URL. +func (b *Builder) New(ctx context.Context, remoteURL string) (*Forge, error) { + if b.URL == "" { + b.URL = DefaultBaseURL + // TODO: Use this to build API URL if not set. + } + if b.Log == nil { + b.Log = log.New(io.Discard) + } + + owner, repo, err := b.repoInfo(remoteURL) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrUnsupportedURL, err) + } + + ghc := github.NewClient(oauth2.NewClient(ctx, b.Token)) + if b.APIURL != "" { + var err error + // FIXME: If we need to ever use the UploadURL, + // we'll want to also fix that here. + ghc, err = ghc.WithEnterpriseURLs(b.APIURL, ghc.UploadURL.String()) + if err != nil { + return nil, fmt.Errorf("set GitHub API URL: %w", err) + } + } + + return &Forge{ + owner: owner, + repo: repo, + log: b.Log, + client: ghc, + }, nil +} + +func (b *Builder) repoInfo(remoteURL string) (owner, repo string, err error) { + baseURL, err := url.Parse(b.URL) + if err != nil { + return "", "", fmt.Errorf("bad base URL: %w", err) + } + + // We recognize the following GitHub remote URL formats: + // + // http(s)://github.com/OWNER/REPO.git + // git@github.com:OWNER/REPO.git + // + // We can parse these all with url.Parse + // if we normalize the latter to: + // + // ssh://git@github.com/OWNER/REPO.git + if !hasGitProtocol(remoteURL) && strings.Contains(remoteURL, ":") { + // $user@$host:$path => ssh://$user@$host/$path + remoteURL = "ssh://" + strings.Replace(remoteURL, ":", "/", 1) + } + + u, err := url.Parse(remoteURL) + if err != nil { + return "", "", fmt.Errorf("parse remote URL: %w", err) + } + + if u.Host != baseURL.Host { + return "", "", fmt.Errorf("%v is not a GitHub URL: expected host %q", u, baseURL.Host) + } + + s := u.Path // /OWNER/REPO.git + s = strings.TrimPrefix(s, "/") // OWNER/REPO.git + s = strings.TrimSuffix(s, ".git") // OWNER/REPO + + owner, repo, ok := strings.Cut(s, "/") + if !ok { + return "", "", fmt.Errorf("path %q does not contain a GitHub repo", s) + } + + return owner, repo, nil +} + +// _gitProtocols is a list of known git protocols +// including the :// suffix. +var _gitProtocols = []string{ + "ssh", + "git", + "git+ssh", + "git+https", + "git+http", + "https", + "http", +} + +func init() { + for i, proto := range _gitProtocols { + _gitProtocols[i] = proto + "://" + } +} + +func hasGitProtocol(url string) bool { + for _, proto := range _gitProtocols { + if strings.HasPrefix(url, proto) { + return true + } + } + return false +} diff --git a/internal/forge/github/change.go b/internal/forge/github/change.go new file mode 100644 index 00000000..53c8a32d --- /dev/null +++ b/internal/forge/github/change.go @@ -0,0 +1,19 @@ +package github + +import ( + "context" + "strconv" +) + +// ChangeID is a unique identifier for a change in a repository. +type ChangeID int + +func (id ChangeID) String() string { + return "#" + strconv.Itoa(int(id)) +} + +// IsMerged reports whether a change has been merged. +func (f *Forge) IsMerged(ctx context.Context, id ChangeID) (bool, error) { + merged, _, err := f.client.PullRequests.IsMerged(ctx, f.owner, f.repo, int(id)) + return merged, err +} diff --git a/internal/forge/github/edit.go b/internal/forge/github/edit.go new file mode 100644 index 00000000..d35b19b1 --- /dev/null +++ b/internal/forge/github/edit.go @@ -0,0 +1,41 @@ +package github + +import ( + "context" + "fmt" + + "github.com/google/go-github/v61/github" +) + +// EditChangeOptions specifies options for an operation to edit +// an existing change. +type EditChangeOptions struct { + // Base specifies the name of the base branch. + // + // If unset, the base branch is not changed. + Base string + + // Draft specifies whether the change should be marked as a draft. + // If unset, the draft status is not changed. + Draft *bool +} + +// EditChange edits an existing change in a repository. +func (f *Forge) EditChange(ctx context.Context, id ChangeID, opts EditChangeOptions) error { + var req github.PullRequest + if opts.Base != "" { + req.Base = &github.PullRequestBranch{ + Ref: &opts.Base, + } + } + if opts.Draft != nil { + req.Draft = opts.Draft + } + + _, _, err := f.client.PullRequests.Edit(ctx, f.owner, f.repo, int(id), &req) + if err != nil { + return fmt.Errorf("edit pull request: %w", err) + } + + return nil +} diff --git a/internal/forge/github/forge.go b/internal/forge/github/forge.go new file mode 100644 index 00000000..5908c6cf --- /dev/null +++ b/internal/forge/github/forge.go @@ -0,0 +1,15 @@ +// Package github defines a GitHub Forge. +package github + +import ( + "github.com/charmbracelet/log" + "github.com/google/go-github/v61/github" +) + +// Forge provides access to GitHub's API, +// while complying with the Forge interface. +type Forge struct { + owner, repo string + log *log.Logger + client *github.Client +} diff --git a/internal/gh/ghtest/shamhub.go b/internal/forge/github/githubtest/shamhub.go similarity index 99% rename from internal/gh/ghtest/shamhub.go rename to internal/forge/github/githubtest/shamhub.go index f764d82d..4f98aa55 100644 --- a/internal/gh/ghtest/shamhub.go +++ b/internal/forge/github/githubtest/shamhub.go @@ -1,5 +1,5 @@ -// Package ghtest provides tools to test code that interacts with GitHub. -package ghtest +// Package githubtest provides tools to test code that interacts with GitHub. +package githubtest import ( "encoding/json" diff --git a/internal/forge/github/list.go b/internal/forge/github/list.go new file mode 100644 index 00000000..9f0e6b7e --- /dev/null +++ b/internal/forge/github/list.go @@ -0,0 +1,73 @@ +package github + +import ( + "context" + "fmt" + + "github.com/google/go-github/v61/github" + "go.abhg.dev/gs/internal/git" +) + +// ListChangesOptions specifies options for listing changes in a repository. +type ListChangesOptions struct { + // State specifies the state of changes to list. + // This can be "open", "closed", or "all". + // Defaults to "open". + State string + + // Branch is the upstream branch to list changes for. + // If unset, changes for all branches are listed. + Branch string +} + +// ListChangesItem is a single item in a list of changes +// returned by ListChanges. +type ListChangesItem struct { + // ID is a unique identifier for the change. + ID ChangeID + // TODO: Type for ChangeID with String() that does "#num". + + // Note: This is always numeric for GitHub, + // but this API is trying to generalize across forges. + + // URL is the web URL at which the change can be viewed. + URL string + + // HeadHash is the hash of the commit at the top of the change. + HeadHash git.Hash + + // BaseName is the name of the base branch + // that this change is proposed against. + BaseName string + + // Draft is true if the change is not yet ready to be reviewed. + Draft bool +} + +// TODO: Reduce filtering options in favor of explicit queries, +// e.g. "FindChangesForBranch" or "ListOpenChanges", etc. + +// ListChanges lists changes in a repository, optionally filtered by options. +func (f *Forge) ListChanges(ctx context.Context, opts ListChangesOptions) ([]ListChangesItem, error) { + // TODO: pagination/iterator? + pulls, _, err := f.client.PullRequests.List(ctx, f.owner, f.repo, &github.PullRequestListOptions{ + State: opts.State, + Head: f.owner + ":" + opts.Branch, + }) + if err != nil { + return nil, fmt.Errorf("list pull requests: %w", err) + } + + items := make([]ListChangesItem, len(pulls)) + for i, pull := range pulls { + items[i] = ListChangesItem{ + ID: ChangeID(pull.GetNumber()), + URL: pull.GetHTMLURL(), + BaseName: pull.GetBase().GetRef(), + HeadHash: git.Hash(pull.GetHead().GetSHA()), + Draft: pull.GetDraft(), + } + } + + return items, nil +} diff --git a/internal/forge/github/submit.go b/internal/forge/github/submit.go new file mode 100644 index 00000000..d5141ff1 --- /dev/null +++ b/internal/forge/github/submit.go @@ -0,0 +1,55 @@ +package github + +import ( + "context" + "fmt" + + "github.com/google/go-github/v61/github" +) + +// SubmitChangeRequest is a request to submit a new change in a repository. +// The change must have already been pushed to the remote. +type SubmitChangeRequest struct { + // Subject is the title of the change. + Subject string // required + + // Body is the description of the change. + Body string + + // Base is the name of the base branch + // that this change is proposed against. + Base string // required + + // Head is the name of the branch containing the change. + // + // This must have already been pushed to the remote. + Head string // required + + // Draft specifies whether the change should be marked as a draft. + Draft bool +} + +// SubmitChangeResult is the result of creating a new change in a repository. +type SubmitChangeResult struct { + ID ChangeID + URL string +} + +// SubmitChange creates a new change in a repository. +func (f *Forge) SubmitChange(ctx context.Context, req SubmitChangeRequest) (SubmitChangeResult, error) { + pr, _, err := f.client.PullRequests.Create(ctx, f.owner, f.repo, &github.NewPullRequest{ + Title: &req.Subject, + Body: &req.Body, + Base: &req.Base, + Head: &req.Head, + Draft: &req.Draft, + }) + if err != nil { + return SubmitChangeResult{}, fmt.Errorf("create pull request: %w", err) + } + + return SubmitChangeResult{ + ID: ChangeID(pr.GetNumber()), + URL: pr.GetHTMLURL(), + }, nil +} diff --git a/internal/gh/token.go b/internal/forge/github/token.go similarity index 81% rename from internal/gh/token.go rename to internal/forge/github/token.go index 6a86ace0..0a3684c8 100644 --- a/internal/gh/token.go +++ b/internal/forge/github/token.go @@ -1,5 +1,4 @@ -// Package gh gates our access to GitHub's APIs. -package gh +package github import ( "errors" @@ -10,6 +9,10 @@ import ( "golang.org/x/oauth2" ) +// TODO: This is a massive hack, and we should not use this. +// Replace this with Device Flow auth +// https://github.com/abhinav/git-spice/issues/22. + // CLITokenSource is an oauth2 token source // that uses the GitHub CLI to get a token. // diff --git a/internal/gh/url.go b/internal/gh/url.go deleted file mode 100644 index 4f55a655..00000000 --- a/internal/gh/url.go +++ /dev/null @@ -1,81 +0,0 @@ -package gh - -import ( - "fmt" - "net/url" - "strings" -) - -// RepoInfo contains information about a GitHub repository. -type RepoInfo struct { - Owner string - Name string -} - -func (r RepoInfo) String() string { - return r.Owner + "/" + r.Name -} - -// ParseRepoInfo guesses the GitHub repository owner and name -// from a Git remote URL. -func ParseRepoInfo(remote string) (RepoInfo, error) { - // We recognize the following GitHub remote URL formats: - // - // http(s)://github.com/OWNER/REPO.git - // git@github.com:OWNER/REPO.git - // - // We can parse these all with url.Parse - // if we normalize the latter to: - // - // ssh://git@github.com/OWNER/REPO.git - if !hasGitProtocol(remote) && strings.Contains(remote, ":") { - // $user@$host:$path => ssh://$user@$host/$path - remote = "ssh://" + strings.Replace(remote, ":", "/", 1) - } - - u, err := url.Parse(remote) - if err != nil { - return RepoInfo{}, fmt.Errorf("parse remote URL: %w", err) - } - // TODO: We currently assume "github.com" is the host and don't check. - // In the future, we'll want to validate against the configured - // GitHub host (e.g. GitHub Enterprise). - - s := u.Path // /OWNER/REPO.git - s = strings.TrimPrefix(s, "/") // OWNER/REPO.git - s = strings.TrimSuffix(s, ".git") // OWNER/REPO - - owner, repo, ok := strings.Cut(s, "/") - if !ok { - return RepoInfo{}, fmt.Errorf("path %q does not contain a GitHub repo", s) - } - - return RepoInfo{Owner: owner, Name: repo}, nil -} - -// _gitProtocols is a list of known git protocols -// including the :// suffix. -var _gitProtocols = []string{ - "ssh", - "git", - "git+ssh", - "git+https", - "git+http", - "https", - "http", -} - -func init() { - for i, proto := range _gitProtocols { - _gitProtocols[i] = proto + "://" - } -} - -func hasGitProtocol(url string) bool { - for _, proto := range _gitProtocols { - if strings.HasPrefix(url, proto) { - return true - } - } - return false -} diff --git a/main.go b/main.go index f1cba6d8..aee7a430 100644 --- a/main.go +++ b/main.go @@ -15,7 +15,7 @@ import ( "github.com/charmbracelet/log" "github.com/mattn/go-isatty" "github.com/posener/complete" - "go.abhg.dev/gs/internal/gh" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/komplete" "golang.org/x/oauth2" ) @@ -180,7 +180,8 @@ type globalOptions struct { // GitHubToken will get replaced once we do Device Flow authentication. // GithubAPIURL will remain hidden. GitHubToken string `name:"github-token" placeholder:"TOKEN" hidden:"" env:"GITHUB_TOKEN" help:"GitHub API token"` - GithubAPIURL string `name:"github-api-url" placeholder:"URL" hidden:"" env:"GITHUB_API_URL" help:"Base URL for GitHub API requests"` + GitHubURL string `name:"github-url" placeholder:"URL" hidden:"" env:"GITHUB_URL" help:"Base URL for GitHub web requests"` + GitHubAPIURL string `name:"github-api-url" placeholder:"URL" hidden:"" env:"GITHUB_API_URL" help:"Base URL for GitHub API requests"` } type mainCmd struct { @@ -215,13 +216,18 @@ func (cmd *mainCmd) AfterApply(kctx *kong.Context, logger *log.Logger) error { logger.SetLevel(log.DebugLevel) } - var tokenSource oauth2.TokenSource = &gh.CLITokenSource{} + var tokenSource oauth2.TokenSource = &github.CLITokenSource{} if token := cmd.GitHubToken; token != "" { tokenSource = oauth2.StaticTokenSource( &oauth2.Token{AccessToken: token}, ) } - kctx.BindTo(tokenSource, (*oauth2.TokenSource)(nil)) + kctx.Bind(&github.Builder{ + URL: cmd.GitHubURL, + APIURL: cmd.GitHubAPIURL, + Token: tokenSource, + Log: logger, + }) return nil } diff --git a/repo_sync.go b/repo_sync.go index 3642ae3e..5e7f104e 100644 --- a/repo_sync.go +++ b/repo_sync.go @@ -9,10 +9,10 @@ import ( "sync" "github.com/charmbracelet/log" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/git" "go.abhg.dev/gs/internal/spice" "go.abhg.dev/gs/internal/text" - "golang.org/x/oauth2" ) type repoSyncCmd struct { @@ -32,7 +32,7 @@ func (*repoSyncCmd) Run( ctx context.Context, log *log.Logger, opts *globalOptions, - tokenSource oauth2.TokenSource, + ghBuilder *github.Builder, ) error { repo, err := git.Open(ctx, ".", git.OpenOptions{ Log: log, @@ -191,16 +191,11 @@ func (*repoSyncCmd) Run( } } - ghrepo, err := ensureGitHubRepo(ctx, log, repo, remote) + forge, err := ensureGitHubForge(ctx, log, ghBuilder, repo, remote) if err != nil { return err } - gh, err := newGitHubClient(ctx, tokenSource, opts) - if err != nil { - return fmt.Errorf("create GitHub client: %w", err) - } - // There are two options for detecting merged branches: // // 1. Query the PR status for each submitted branch. @@ -215,8 +210,8 @@ func (*repoSyncCmd) Run( // In the future, we may need a hybrid approach that switches to (2). var ( - branches []string - prs []int // prs[i] = PR for branches[i] + branches []string + changeIDs []github.ChangeID // changeIDs[i] = PR for branches[i] ) { tracked, err := svc.LoadBranches(ctx) @@ -227,7 +222,7 @@ func (*repoSyncCmd) Run( for _, b := range tracked { if b.PR != 0 { branches = append(branches, b.Name) - prs = append(prs, b.PR) + changeIDs = append(changeIDs, github.ChangeID(b.PR)) } } } @@ -237,7 +232,7 @@ func (*repoSyncCmd) Run( return nil } - prMerged := make([]bool, len(branches)) // whether prs[i] is merged + changesMerged := make([]bool, len(branches)) // whether changeIDs[i] is merged { idxc := make(chan int) // PRs to query @@ -249,13 +244,14 @@ func (*repoSyncCmd) Run( defer wg.Done() for idx := range idxc { - merged, _, err := gh.PullRequests.IsMerged(ctx, ghrepo.Owner, ghrepo.Name, prs[idx]) + id := changeIDs[idx] + merged, err := forge.IsMerged(ctx, id) if err != nil { - log.Error("Failed to query PR status", "pr", prs[idx], "error", err) + log.Error("Failed to query PR status", "pr", id, "error", err) continue } - prMerged[idx] = merged + changesMerged[idx] = merged } }() } @@ -273,11 +269,11 @@ func (*repoSyncCmd) Run( // Should the branches be deleted in any particular order? // (e.g. from the bottom of the stack up) for i, branch := range branches { - if !prMerged[i] { + if !changesMerged[i] { continue } - log.Infof("%v: #%v was merged: deleting...", branch, prs[i]) + log.Infof("%v: %v was merged: deleting...", branch, changeIDs[i]) err := (&branchDeleteCmd{ Name: branch, Force: true, diff --git a/script_test.go b/script_test.go index 9cabf574..6b9b82fd 100644 --- a/script_test.go +++ b/script_test.go @@ -15,7 +15,7 @@ import ( "github.com/google/go-github/v61/github" "github.com/rogpeppe/go-internal/diff" "github.com/rogpeppe/go-internal/testscript" - "go.abhg.dev/gs/internal/gh/ghtest" + "go.abhg.dev/gs/internal/forge/github/githubtest" "go.abhg.dev/gs/internal/git/gittest" "go.abhg.dev/gs/internal/logtest" "go.abhg.dev/gs/internal/mockedit" @@ -47,7 +47,7 @@ func TestScript(t *testing.T) { // because testscript does not allow adding the value afterwards. // We only set up the ShamHub on gh-init, though. type shamHubKey struct{} - type shamHubValue struct{ v *ghtest.ShamHub } + type shamHubValue struct{ v *githubtest.ShamHub } type testingTBKey struct{} @@ -83,7 +83,7 @@ func TestScript(t *testing.T) { // Sets up a fake GitHub server. "gh-init": func(ts *testscript.TestScript, neg bool, args []string) { t := ts.Value(testingTBKey{}).(testing.TB) - shamHub, err := ghtest.NewShamHub(ghtest.ShamHubConfig{ + shamHub, err := githubtest.NewShamHub(githubtest.ShamHubConfig{ Log: logtest.New(t), }) if err != nil { @@ -106,7 +106,7 @@ func TestScript(t *testing.T) { ) ts.Setenv("GITHUB_API_URL", shamHub.APIURL()) - ts.Setenv("GITHUB_GIT_URL", shamHub.GitURL()) + ts.Setenv("GITHUB_URL", shamHub.GitURL()) ts.Setenv("GITHUB_TOKEN", "test-token") }, @@ -154,7 +154,7 @@ func TestScript(t *testing.T) { ts.Fatalf("invalid PR number: %s", err) } - req := ghtest.MergePullRequest{ + req := githubtest.MergePullRequest{ Owner: owner, Repo: repo, Number: pr, diff --git a/stack_submit.go b/stack_submit.go index 8faea844..0710c673 100644 --- a/stack_submit.go +++ b/stack_submit.go @@ -5,9 +5,9 @@ import ( "fmt" "github.com/charmbracelet/log" + "go.abhg.dev/gs/internal/forge/github" "go.abhg.dev/gs/internal/git" "go.abhg.dev/gs/internal/spice" - "golang.org/x/oauth2" ) type stackSubmitCmd struct { @@ -19,7 +19,7 @@ func (cmd *stackSubmitCmd) Run( ctx context.Context, log *log.Logger, opts *globalOptions, - tokenSource oauth2.TokenSource, + ghBuilder *github.Builder, ) error { repo, err := git.Open(ctx, ".", git.OpenOptions{ Log: log, @@ -56,7 +56,7 @@ func (cmd *stackSubmitCmd) Run( DryRun: cmd.DryRun, Fill: cmd.Fill, Name: branch, - }).Run(ctx, log, opts, tokenSource) + }).Run(ctx, log, opts, ghBuilder) if err != nil { return fmt.Errorf("submit %v: %w", branch, err) } diff --git a/testdata/script/branch_submit_by_name.txt b/testdata/script/branch_submit_by_name.txt index 2f847d37..9af09673 100644 --- a/testdata/script/branch_submit_by_name.txt +++ b/testdata/script/branch_submit_by_name.txt @@ -36,7 +36,7 @@ Contents of feature1 "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "25596d0f65bb595aa658f9fa7fe164949cd2ea0b" diff --git a/testdata/script/branch_submit_create_update.txt b/testdata/script/branch_submit_create_update.txt index 2d816c69..66624952 100644 --- a/testdata/script/branch_submit_create_update.txt +++ b/testdata/script/branch_submit_create_update.txt @@ -46,7 +46,7 @@ New contents of feature1 "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "25596d0f65bb595aa658f9fa7fe164949cd2ea0b" @@ -65,7 +65,7 @@ New contents of feature1 "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "0ff00934c3e878a52bf90fda3643627921cc0aeb" diff --git a/testdata/script/branch_submit_multiple_commits.txt b/testdata/script/branch_submit_multiple_commits.txt index 67325841..8e5185a3 100644 --- a/testdata/script/branch_submit_multiple_commits.txt +++ b/testdata/script/branch_submit_multiple_commits.txt @@ -41,7 +41,7 @@ Part 2 of the feature "state": "open", "title": "Add feature", "body": "Add feature\n\nAdd feature part 2", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature", "sha": "c1a2497e4115fcbfc275d36b754a8ed4ca1169e4" diff --git a/testdata/script/branch_submit_remote_prompt.txt b/testdata/script/branch_submit_remote_prompt.txt index 663fb986..e7e14117 100644 --- a/testdata/script/branch_submit_remote_prompt.txt +++ b/testdata/script/branch_submit_remote_prompt.txt @@ -48,7 +48,7 @@ Changes will be pushed to this remote "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/bob/example-fork/pull/1", + "html_url": "$GITHUB_URL/bob/example-fork/pull/1", "head": { "ref": "feature1", "sha": "25596d0f65bb595aa658f9fa7fe164949cd2ea0b" diff --git a/testdata/script/branch_submit_rename.txt b/testdata/script/branch_submit_rename.txt index df19009f..73e1bcfa 100644 --- a/testdata/script/branch_submit_rename.txt +++ b/testdata/script/branch_submit_rename.txt @@ -50,7 +50,7 @@ New contents of feature1 "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "854f3268c794eb5ec30439658dfb43ac494d9074" @@ -69,7 +69,7 @@ New contents of feature1 "state": "open", "title": "Add feature1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "b805a8b9545d71929cc128fc81b0d86bb2def9ed" diff --git a/testdata/script/downstack_submit.txt b/testdata/script/downstack_submit.txt index 34fb8630..e3ced43e 100644 --- a/testdata/script/downstack_submit.txt +++ b/testdata/script/downstack_submit.txt @@ -36,9 +36,9 @@ This is feature 2 -- repo/feature3.txt -- -- golden/submit-log.txt -- -INF Created #1: $GITHUB_GIT_URL/alice/example/pull/1 -INF Created #2: $GITHUB_GIT_URL/alice/example/pull/2 -INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 +INF Created #1: $GITHUB_URL/alice/example/pull/1 +INF Created #2: $GITHUB_URL/alice/example/pull/2 +INF Created #3: $GITHUB_URL/alice/example/pull/3 -- golden/pulls.json -- [ { @@ -46,7 +46,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "8526d1a7195abb635f28bc93155b9155b76f3881" @@ -61,7 +61,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 2", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/2", + "html_url": "$GITHUB_URL/alice/example/pull/2", "head": { "ref": "feature2", "sha": "9806160f3b27dbff81e496260d26fc32f5ee5cf0" @@ -76,7 +76,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 3", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/3", + "html_url": "$GITHUB_URL/alice/example/pull/3", "head": { "ref": "feature3", "sha": "7122de7820daae3550784fd205f9064bcd046cce" diff --git a/testdata/script/repo_sync_merged_pr.txt b/testdata/script/repo_sync_merged_pr.txt index 59cbe7e3..c8d88b1c 100644 --- a/testdata/script/repo_sync_merged_pr.txt +++ b/testdata/script/repo_sync_merged_pr.txt @@ -53,7 +53,7 @@ Contents of feature2 "title": "Add feature1", "body": "", "merged": true, - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "9f1c9af063d2363d6a1381581bfab2a25d12be4c" diff --git a/testdata/script/repo_sync_unpushed_commits.txt b/testdata/script/repo_sync_unpushed_commits.txt index 83addc02..fecec515 100644 --- a/testdata/script/repo_sync_unpushed_commits.txt +++ b/testdata/script/repo_sync_unpushed_commits.txt @@ -51,7 +51,7 @@ Contents of feature2 "title": "Add feature1", "body": "", "merged": true, - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "9f1c9af063d2363d6a1381581bfab2a25d12be4c" diff --git a/testdata/script/stack_submit.txt b/testdata/script/stack_submit.txt index 48e69f09..ccedf347 100644 --- a/testdata/script/stack_submit.txt +++ b/testdata/script/stack_submit.txt @@ -51,9 +51,9 @@ This is feature 2 This is feature 3 -- golden/submit-log.txt -- -INF Created #1: $GITHUB_GIT_URL/alice/example/pull/1 -INF Created #2: $GITHUB_GIT_URL/alice/example/pull/2 -INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 +INF Created #1: $GITHUB_URL/alice/example/pull/1 +INF Created #2: $GITHUB_URL/alice/example/pull/2 +INF Created #3: $GITHUB_URL/alice/example/pull/3 -- golden/start.json -- [ { @@ -61,7 +61,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "8526d1a7195abb635f28bc93155b9155b76f3881" @@ -76,7 +76,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 2", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/2", + "html_url": "$GITHUB_URL/alice/example/pull/2", "head": { "ref": "feature2", "sha": "9806160f3b27dbff81e496260d26fc32f5ee5cf0" @@ -91,7 +91,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 3", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/3", + "html_url": "$GITHUB_URL/alice/example/pull/3", "head": { "ref": "feature3", "sha": "63b2d337c8172c9f79aec9c760efc95e3c0c8472" @@ -111,7 +111,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "merged": true, "title": "Add feature 1", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/1", + "html_url": "$GITHUB_URL/alice/example/pull/1", "head": { "ref": "feature1", "sha": "8526d1a7195abb635f28bc93155b9155b76f3881" @@ -126,7 +126,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 2", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/2", + "html_url": "$GITHUB_URL/alice/example/pull/2", "head": { "ref": "feature2", "sha": "ce97ed17bf5f23059c1a32bc21ca4f9a9a5e4c26" @@ -141,7 +141,7 @@ INF Created #3: $GITHUB_GIT_URL/alice/example/pull/3 "state": "open", "title": "Add feature 3", "body": "", - "html_url": "$GITHUB_GIT_URL/alice/example/pull/3", + "html_url": "$GITHUB_URL/alice/example/pull/3", "head": { "ref": "feature3", "sha": "82da921d2f4531778b8ba0e3a7ad39713a6358ef"