From 55974404d7c67f7f5ccb9ee8235091d0571eb47d Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Wed, 4 Nov 2026 16:55:52 +0000 Subject: [PATCH] Stop submit at an empty branch in the middle of the stack A branch with no commits of its own (all of it already in the branch below, e.g. after a reorder or a pop moved its work elsewhere) can't get a pull request: GitHub refuses to open one with no commits. Sitting under other branches that is a dead end, since their pull requests have nothing to stack on, and submit only found out part way through pushing. Check before anything is pushed. An empty branch without an open pull request under another branch being submitted stops the submit with the git stack reorder that moves it to the top, or the pop or modify that gets rid of it. At the top of what is submitted it is only a notice, and a dry run says either without stopping. --- README.md | 5 ++++ pkg/app/submit.go | 58 ++++++++++++++++++++++++++++++++++++++++++ pkg/app/submit_test.go | 35 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/README.md b/README.md index 01689e2..e1b6a83 100644 --- a/README.md +++ b/README.md @@ -545,6 +545,11 @@ so the merged branch goes and the ones above it are restacked onto trunk before otherwise the next PR up would show the merged commits too. A dry run only says which branches that would be. +GitHub won't open a PR for a branch with no commits of its own, so submit stops before pushing +anything when such a branch (without a PR already) sits under another one it is about to submit, +and tells you the `git stack reorder` that moves it to the top (or to pop it, or commit on it). At +the top of what's submitted it's only a notice. + Branches are pushed with `--force-with-lease`, so after a restack your branches are force pushed. If a branch has commits on the remote that aren't in your local one (someone else pushed to it), submit refuses rather than overwrite them. `git sync` pulls them in when the remote is diff --git a/pkg/app/submit.go b/pkg/app/submit.go index d787ade..a580086 100644 --- a/pkg/app/submit.go +++ b/pkg/app/submit.go @@ -11,6 +11,7 @@ import ( "github.com/DomBlack/git-stack/pkg/config" "github.com/DomBlack/git-stack/pkg/forge" "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/shell" "github.com/DomBlack/git-stack/pkg/stack" ) @@ -192,6 +193,10 @@ func (a *App) submit(ctx context.Context, repo git.Repo, o SubmitOptions) (Submi } } + if err := a.checkEmpty(ctx, repo, s, live(s.Branches), len(live(scope)), before, o.DryRun, &res); err != nil { + return res, err + } + var newBranches []string for _, b := range live(scope) { pr, has := before[b.Name] @@ -767,3 +772,56 @@ func (a *App) closePending(ctx context.Context, repo git.Repo, prs []forge.PullR res.Notices = append(res.Notices, fmt.Sprintf("could not update the list of pull requests to close: %v", err)) } } + +// checkEmpty looks for branches about to be submitted that have no commits +// of their own (everything in them is in the branch below) and no open pull +// request. GitHub won't open a pull request with no commits, so at the top +// that is a notice; under another branch being submitted it stops the +// submit, since the pull requests above have nothing to stack on. The way +// out is to move it to the top, drop it or commit on it. branches are the +// stack's live branches, the first n of them being submitted. A dry run +// only says so. +func (a *App) checkEmpty(ctx context.Context, repo git.Repo, s *stack.Stack, branches []stack.Branch, n int, prs map[string]forge.PullRequest, dryRun bool, res *SubmitResult) error { + var empty []string + parent := s.Trunk + for i, b := range branches[:n] { + pr, has := prs[b.Name] + open := has && (pr.State == forge.StateOpen || pr.State == forge.StateDraft) + count, err := a.d.Git.CountCommits(ctx, repo, parent, b.Name) + parent = b.Name + if err != nil || count > 0 || open { + continue + } + if i == n-1 { + res.Notices = append(res.Notices, fmt.Sprintf("%s has no commits of its own, so GitHub won't open a pull request for it yet", b.Name)) + continue + } + empty = append(empty, b.Name) + } + if len(empty) == 0 { + return nil + } + order := slices.DeleteFunc(s.Names(), func(name string) bool { return slices.Contains(empty, name) }) + order = append(order, empty...) + msg := fmt.Sprintf("%s %s no commits of its own, so GitHub can't open a pull request for %s for the ones above to stack on", + joinNames(empty), pluralise(len(empty), "has", "have"), pluralise(len(empty), "it", "them")) + if dryRun { + res.Notices = append(res.Notices, msg+"; a real submit stops here") + return nil + } + // Branch names can't hold control characters, so quoting never fails. + quote := func(name string) string { + q, _ := shell.Arg(name) + return q + } + args := make([]string, len(order)) + for i, name := range order { + args[i] = quote(name) + } + first := empty[0] + return stack.New(stack.KindInvalidArgs, msg).WithSteps( + fmt.Sprintf("move %s to the top: git stack reorder %s", pluralise(len(empty), "it", "them"), strings.Join(args, " ")), + fmt.Sprintf("or drop %s: git stack checkout %s, then git stack pop", first, quote(first)), + fmt.Sprintf("or commit on %s: git stack checkout %s, then git stack modify", first, quote(first)), + ) +} diff --git a/pkg/app/submit_test.go b/pkg/app/submit_test.go index 457f4d4..5755948 100644 --- a/pkg/app/submit_test.go +++ b/pkg/app/submit_test.go @@ -855,3 +855,38 @@ func kindOf(err error) stack.Kind { } return stack.KindUnknown } + +// An empty branch under another one being submitted stops the submit +// before anything is pushed, saying how to move it to the top; at the top +// it is only a notice. +func TestSubmitRefusesAnEmptyBranchInTheMiddle(t *testing.T) { + deps, sf, _, repo, dir := submitFixture(t) + gittest.Run(t, dir, "branch", "empty", "a") + graph := stack.NewGraph([]stack.Stack{{Trunk: "main", Branches: []stack.Branch{{Name: "a"}, {Name: "empty"}, {Name: "b"}}}}) + deps.Meta = memMeta{graph} + sf.graph = graph + + res, err := app.New(deps).Submit(context.Background(), repo, app.SubmitOptions{DryRun: true}) + if err != nil || !slices.ContainsFunc(res.Notices, func(n string) bool { return strings.Contains(n, "empty has no commits of its own") }) { + t.Errorf("dry run: %v %q", err, res.Notices) + } + + _, err = app.New(deps).Submit(context.Background(), repo, app.SubmitOptions{NoEdit: true}) + se, ok := errors.AsType[*stack.Error](err) + if !ok || se.Kind != stack.KindInvalidArgs || !slices.Contains(se.NextSteps, "move it to the top: git stack reorder a b empty") { + t.Fatalf("err = %#v", err) + } + if len(sf.opts) != 0 { + t.Errorf("nothing may be submitted: %+v", sf.opts) + } + + // At the top it's only a notice. + graph = stack.NewGraph([]stack.Stack{{Trunk: "main", Branches: []stack.Branch{{Name: "a"}, {Name: "b"}, {Name: "empty"}}}}) + deps.Meta = memMeta{graph} + sf.graph = graph + gittest.Run(t, dir, "branch", "-f", "empty", "b") + res, err = app.New(deps).Submit(context.Background(), repo, app.SubmitOptions{NoEdit: true}) + if err != nil || !slices.ContainsFunc(res.Notices, func(n string) bool { return strings.Contains(n, "won't open a pull request for it yet") }) { + t.Errorf("top: %v %q", err, res.Notices) + } +}