Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/includes/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions doc/src/cli/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

<!-- gs:version unreleased -->

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.
Expand Down
6 changes: 6 additions & 0 deletions internal/forge/github/forge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
37 changes: 37 additions & 0 deletions internal/forge/github/forge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion internal/forge/github/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions internal/forge/github/merge_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
20 changes: 20 additions & 0 deletions internal/forge/github/merge_range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,
Expand Down
23 changes: 15 additions & 8 deletions internal/forge/github/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions internal/forge/github/stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
28 changes: 24 additions & 4 deletions internal/forge/github/stacks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down Expand Up @@ -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,
}
}

Expand Down
Loading