diff --git a/doc/includes/cli-reference.md b/doc/includes/cli-reference.md index 871bf9b99..ce1d82e61 100644 --- a/doc/includes/cli-reference.md +++ b/doc/includes/cli-reference.md @@ -12,7 +12,7 @@ git-spice is a command line tool for stacking Git branches. * `-C`, `--dir=DIR`: Change to DIR before doing anything * `--[no-]prompt`: Whether to prompt for missing information -**Configuration**: [spice.forge.bitbucket.apiURL](/cli/config.md#spiceforgebitbucketapiurl), [spice.forge.bitbucket.kind](/cli/config.md#spiceforgebitbucketkind), [spice.forge.bitbucket.url](/cli/config.md#spiceforgebitbucketurl), [spice.forge.forgejo.apiURL](/cli/config.md#spiceforgeforgejoapiurl), [spice.forge.forgejo.url](/cli/config.md#spiceforgeforgejourl), [spice.forge.gitea.apiURL](/cli/config.md#spiceforgegiteaapiurl), [spice.forge.gitea.url](/cli/config.md#spiceforgegiteaurl), [spice.forge.github.apiUrl](/cli/config.md#spiceforgegithubapiurl), [spice.forge.github.url](/cli/config.md#spiceforgegithuburl), [spice.forge.gitlab.apiURL](/cli/config.md#spiceforgegitlabapiurl), [spice.forge.gitlab.oauth.clientID](/cli/config.md#spiceforgegitlaboauthclientid), [spice.forge.gitlab.removeSourceBranch](/cli/config.md#spiceforgegitlabremovesourcebranch), [spice.forge.gitlab.url](/cli/config.md#spiceforgegitlaburl), [spice.forge.kind](/cli/config.md#spiceforgekind), [spice.git.indexLockTimeout](/cli/config.md#spicegitindexlocktimeout), [spice.secret.backend](/cli/config.md#spicesecretbackend) +**Configuration**: [spice.forge.bitbucket.apiURL](/cli/config.md#spiceforgebitbucketapiurl), [spice.forge.bitbucket.kind](/cli/config.md#spiceforgebitbucketkind), [spice.forge.bitbucket.url](/cli/config.md#spiceforgebitbucketurl), [spice.forge.forgejo.apiURL](/cli/config.md#spiceforgeforgejoapiurl), [spice.forge.forgejo.url](/cli/config.md#spiceforgeforgejourl), [spice.forge.gitea.apiURL](/cli/config.md#spiceforgegiteaapiurl), [spice.forge.gitea.url](/cli/config.md#spiceforgegiteaurl), [spice.forge.github.apiUrl](/cli/config.md#spiceforgegithubapiurl), [spice.forge.github.stacks](/cli/config.md#spiceforgegithubstacks), [spice.forge.github.url](/cli/config.md#spiceforgegithuburl), [spice.forge.gitlab.apiURL](/cli/config.md#spiceforgegitlabapiurl), [spice.forge.gitlab.oauth.clientID](/cli/config.md#spiceforgegitlaboauthclientid), [spice.forge.gitlab.removeSourceBranch](/cli/config.md#spiceforgegitlabremovesourcebranch), [spice.forge.gitlab.url](/cli/config.md#spiceforgegitlaburl), [spice.forge.kind](/cli/config.md#spiceforgekind), [spice.git.indexLockTimeout](/cli/config.md#spicegitindexlocktimeout), [spice.secret.backend](/cli/config.md#spicesecretbackend) ## Shell diff --git a/doc/src/cli/config.md b/doc/src/cli/config.md index 11e7e1c90..1f7a827f0 100644 --- a/doc/src/cli/config.md +++ b/doc/src/cli/config.md @@ -328,6 +328,23 @@ or computed from the GitHub URL if not set. See also: [GitHub Enterprise](../setup/auth.md#github-enterprise). +### spice.forge.github.stacks + + + +Whether to use GitHub native stack operations. + +**Accepted values:** + +- `true` (default) +- `false` + +When set to `false`, submit commands skip GitHub native stack updates, +and merge commands use ordinary pull request merges +instead of atomic stack merges. +This setting affects only future git-spice operations; +it does not remove native stack associations that already exist on GitHub. + ### spice.forge.github.url URL of the GitHub instance used for GitHub requests. diff --git a/internal/forge/github/forge.go b/internal/forge/github/forge.go index 487997b74..dcd268d9b 100644 --- a/internal/forge/github/forge.go +++ b/internal/forge/github/forge.go @@ -35,6 +35,12 @@ type Options struct { // Token is a fixed token used to authenticate with GitHub. // This may be used to skip the login flow. Token string `name:"github-token" hidden:"" env:"GITHUB_TOKEN" help:"GitHub API token"` + + // Stacks controls whether GitHub native stack operations are used. + // The default is `on`. + // The `off` value makes stack updates and range merges return + // [forge.ErrUnsupported] before accessing GitHub. + Stacks bool `name:"github-stacks" hidden:"" config:"forge.github.stacks" default:"true" help:"Whether to use GitHub native stack operations. One of 'true' and 'false'."` } // Definition configures GitHub forge instances. diff --git a/internal/forge/github/forge_test.go b/internal/forge/github/forge_test.go index 6d4fdaab3..060049e3f 100644 --- a/internal/forge/github/forge_test.go +++ b/internal/forge/github/forge_test.go @@ -3,12 +3,49 @@ package github import ( "testing" + "github.com/alecthomas/kong" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/git/giturl" ) +func TestOptions_Stacks(t *testing.T) { + tests := []struct { + name string + give any + want bool + }{ + {name: "Default", want: true}, + {name: "Enabled", give: "true", want: true}, + {name: "Disabled", give: "false", want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var opts Options + parser, err := kong.New( + &opts, + kong.Resolvers(kong.ResolverFunc(func( + _ *kong.Context, + _ *kong.Path, + flag *kong.Flag, + ) (any, error) { + if flag.Tag.Get("config") == "forge.github.stacks" { + return tt.give, nil + } + return nil, nil + })), + ) + require.NoError(t, err) + + _, err = parser.Parse(nil) + require.NoError(t, err) + assert.Equal(t, tt.want, opts.Stacks) + }) + } +} + func TestURLs(t *testing.T) { tests := []struct { name string diff --git a/internal/forge/github/integration_test.go b/internal/forge/github/integration_test.go index 7f969f59d..cbd5c1be1 100644 --- a/internal/forge/github/integration_test.go +++ b/internal/forge/github/integration_test.go @@ -100,6 +100,9 @@ func TestIntegration(t *testing.T) { githubForge := github.Forge{ Log: silogtest.New(t), + Options: github.Options{ + Stacks: true, + }, } forgetest.RunIntegration(t, forgetest.IntegrationConfig{ @@ -509,7 +512,9 @@ func TestIntegration_DivergentStackMerge(t *testing.T) { gatewayClient := newGateway(t, httpClient) repo, err := github.NewRepository( - t.Context(), new(github.Forge), cfg.Owner, cfg.Repo, + t.Context(), &github.Forge{ + Options: github.Options{Stacks: true}, + }, cfg.Owner, cfg.Repo, silogtest.New(t), gatewayClient, "", ) require.NoError(t, err) diff --git a/internal/forge/github/merge_range.go b/internal/forge/github/merge_range.go index ab461f4bc..55256d3fb 100644 --- a/internal/forge/github/merge_range.go +++ b/internal/forge/github/merge_range.go @@ -19,6 +19,10 @@ func (r *Repository) PlanMergeRanges( ctx context.Context, changes []forge.StackChange, ) ([]forge.MergeRangePlan, error) { + if !r.stacksEnabled { + return nil, forge.ErrUnsupported + } + type requestedChange struct { change forge.ChangeID base int @@ -138,6 +142,10 @@ func (p *githubMergeRangePlan) Merge( ctx context.Context, request forge.MergeRangeRequest, ) (forge.MergeOperation, error) { + if !p.repository.stacksEnabled { + return nil, forge.ErrUnsupported + } + if len(request.Changes) != len(p.changes) { return nil, fmt.Errorf( "merge range request has %d changes, planned %d", diff --git a/internal/forge/github/merge_range_test.go b/internal/forge/github/merge_range_test.go index 045b2635b..f96259352 100644 --- a/internal/forge/github/merge_range_test.go +++ b/internal/forge/github/merge_range_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "go.abhg.dev/gs/internal/forge" "go.abhg.dev/gs/internal/gateway/github" + "go.abhg.dev/gs/internal/silog/silogtest" "go.uber.org/mock/gomock" ) @@ -234,6 +235,25 @@ func TestRepository_MergeRangeUnsupported(t *testing.T) { assert.ErrorIs(t, err, github.ErrNotFound) } +func TestRepository_PlanMergeRangesDisabled(t *testing.T) { + gateway := NewMockGithubGateway(gomock.NewController(t)) + repo, err := newRepository( + t.Context(), + new(Forge), + "acme", + "repo", + silogtest.New(t), + gateway, + "repo-id", + ) + require.NoError(t, err) + + // Disabled stack operations take precedence over request validation. + plans, err := repo.PlanMergeRanges(t.Context(), nil) + assert.Nil(t, plans) + require.ErrorIs(t, err, forge.ErrUnsupported) +} + func newMergeRangePlan( t *testing.T, gateway *MockGithubGateway, diff --git a/internal/forge/github/repository.go b/internal/forge/github/repository.go index b3d81b81a..e7a124709 100644 --- a/internal/forge/github/repository.go +++ b/internal/forge/github/repository.go @@ -71,6 +71,12 @@ type Repository struct { gateway githubGateway forge *Forge + // stacksEnabled snapshots the forge configuration when the repository is + // opened. When disabled, both native-stack operations report + // forge.ErrUnsupported without accessing the gateway, which lets + // forge-independent callers use their ordinary submit and merge paths. + stacksEnabled bool + identityIDsMu sync.RWMutex // guards userIDsCache and teamIDsCache // userIDsCache caches successful login lookups for this repository. // @@ -103,14 +109,15 @@ func newRepository( } return &Repository{ - owner: owner, - repo: repo, - log: log, - gateway: gateway, - repoID: repoID, - forge: forge, - userIDsCache: make(map[string]github.ID), - teamIDsCache: make(map[github.TeamName]github.ID), + owner: owner, + repo: repo, + log: log, + gateway: gateway, + repoID: repoID, + forge: forge, + stacksEnabled: forge.Options.Stacks, + userIDsCache: make(map[string]github.ID), + teamIDsCache: make(map[github.TeamName]github.ID), }, nil } diff --git a/internal/forge/github/stacks.go b/internal/forge/github/stacks.go index d5422ec12..08aeade15 100644 --- a/internal/forge/github/stacks.go +++ b/internal/forge/github/stacks.go @@ -34,6 +34,9 @@ func (r *Repository) PlanStackUpdate( ctx context.Context, changes []forge.StackChange, ) (forge.StackUpdatePlan, error) { + if !r.stacksEnabled { + return nil, forge.ErrUnsupported + } if err := r.gateway.CheckPullRequestStacks(ctx, r.owner, r.repo); err != nil { if errors.Is(err, github.ErrNotFound) { return nil, errors.Join(forge.ErrUnsupported, err) diff --git a/internal/forge/github/stacks_test.go b/internal/forge/github/stacks_test.go index c917f81bb..33fa74d98 100644 --- a/internal/forge/github/stacks_test.go +++ b/internal/forge/github/stacks_test.go @@ -367,6 +367,25 @@ func TestRepository_UpdateStackWriteNotFoundIsFailure(t *testing.T) { assert.NotErrorIs(t, err, forge.ErrUnsupported) } +func TestRepository_UpdateStackDisabled(t *testing.T) { + gateway := NewMockGithubGateway(gomock.NewController(t)) + repo, err := newRepository( + t.Context(), + new(Forge), + "acme", + "repo", + silogtest.New(t), + gateway, + "repo-id", + ) + require.NoError(t, err) + + _, err = repo.PlanStackUpdate(t.Context(), []forge.StackChange{ + {Change: &PR{Number: 1}, BaseBranch: "base"}, + }) + require.ErrorIs(t, err, forge.ErrUnsupported) +} + func TestRepository_UpdateStackMissingChange(t *testing.T) { gateway := NewMockGithubGateway(gomock.NewController(t)) expectPullRequests(t, gateway, map[int]*github.StackUpdatePullRequest{1: nil}) @@ -486,10 +505,11 @@ func TestRepository_UpdateStackReconnectsAboveMergedChange(t *testing.T) { func newStackRepository(t *testing.T, gateway githubGateway) *Repository { return &Repository{ - owner: "acme", - repo: "repo", - gateway: gateway, - log: silogtest.New(t), + owner: "acme", + repo: "repo", + gateway: gateway, + log: silogtest.New(t), + stacksEnabled: true, } }