diff --git a/docs/architecture.md b/docs/architecture.md index 6ac53fd..d0c9fbd 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -400,8 +400,9 @@ stack. All three go through one reshape step in `pkg/app`. It snapshots the stac to change, rewrites them in a single `Update`, then hands the branches whose parent changed to the native restack, so nothing is checked out (init stops after the `Update` and just says which branches need a restack). `Update` grew for this: its callback may now append stacks (written to -the worktree's own file when it has one, else the common one, which is created with -`schemaVersion: 1` if there isn't one yet) and clear or set a stack's id and number, though it +the current worktree's own file, which is created with `schemaVersion: 1` if there isn't one +yet; gh stack only reads the file in the git dir it runs in, so a stack put in the main +checkout's file from a linked worktree was invisible to `gh stack submit` there) and clear or set a stack's id and number, though it still can't drop or reorder stacks. In that same `Update` every branch whose parent changed gets its recorded base fixed to where its own commits start (the recorded base if it's still in the branch's history, else the merge base with its old parent), because the restack replays diff --git a/pkg/backend/ghstack/backend.go b/pkg/backend/ghstack/backend.go index d728745..7e635e5 100644 --- a/pkg/backend/ghstack/backend.go +++ b/pkg/backend/ghstack/backend.go @@ -152,11 +152,11 @@ func (b *Backend) Load(_ context.Context, repo git.Repo) (*stack.Graph, error) { // and writes each file back, editing the JSON in place so members we don't // model, PR records and ordering survive. A file whose stacks fn didn't // change is not rewritten. fn may also append stacks (each with a trunk and -// at least one branch; they are written to the current worktree's file, else -// the main checkout's, else a new file in the common dir) and may set or -// clear a stack's ID and Number (clearing removes them from the record). It -// must not remove or reorder existing stacks; a stack left with no branches -// is removed. +// at least one branch; they are written to the current worktree's file, +// created if need be, since that is the only one gh stack reads there) and +// may set or clear a stack's ID and Number (clearing removes them from the +// record). It must not remove or reorder existing stacks; a stack left with +// no branches is removed. func (b *Backend) Update(ctx context.Context, repo git.Repo, fn func(*stack.Graph) error) error { var dirs []string for _, src := range sources(repo) { @@ -164,10 +164,10 @@ func (b *Backend) Update(ctx context.Context, repo git.Repo, fn func(*stack.Grap dirs = append(dirs, src.gitDir) } } - // The common dir is always locked, even with no file yet: Update may - // create the first stack's file there and two writers must not race it. - if !slices.Contains(dirs, repo.CommonDir) { - dirs = append(dirs, repo.CommonDir) + // The current git dir is always locked, even with no file yet: Update + // may create its file for a new stack and two writers must not race it. + if !slices.Contains(dirs, repo.GitDir) { + dirs = append(dirs, repo.GitDir) } slices.Sort(dirs) var unlocks []func() @@ -254,19 +254,18 @@ func (r *reading) parseAll() ([]*node, map[string]*node, error) { return roots, pool, nil } -// fileFor picks the file new stacks are written to: the current worktree's -// when it has one, else the main checkout's, else a fresh file in the -// common dir, which is where gh stack keeps it today. +// fileFor picks the file new stacks are written to: the current worktree's, +// created when it has none yet. gh stack only reads the file in the git dir +// it runs in, so a stack written anywhere else would be invisible to it from +// here (in the main checkout that is the common dir). func (r *reading) fileFor(repo git.Repo) int { - for _, dir := range []string{repo.GitDir, repo.CommonDir} { - for i, f := range r.files { - if f.src.gitDir == dir { - return i - } + for i, f := range r.files { + if f.src.gitDir == repo.GitDir { + return i } } r.files = append(r.files, loaded{ - src: source{repo.CommonDir, filepath.Dir(repo.CommonDir)}, + src: source{repo.GitDir, repo.TopLevel}, data: []byte("{\n \"schemaVersion\": 1,\n \"stacks\": []\n}\n"), f: &file{SchemaVersion: SchemaVersion}, }) diff --git a/pkg/backend/ghstack/update_test.go b/pkg/backend/ghstack/update_test.go index 3bbd9d7..fa47703 100644 --- a/pkg/backend/ghstack/update_test.go +++ b/pkg/backend/ghstack/update_test.go @@ -212,6 +212,36 @@ func TestUpdateAppendsStackToCurrentWorktreeFile(t *testing.T) { } } +func TestUpdateAppendsStackToNewWorktreeFile(t *testing.T) { + b, repo, mainPath, _ := updateFixture(t) + ctx := context.Background() + // A second worktree with no file of its own: gh stack only reads the + // worktree's file there, so the new stack must not go in the main one. + dir := filepath.Join(t.TempDir(), "fresh") + gittest.Run(t, repo.TopLevel, "worktree", "add", "-q", "-b", "f", dir) + wtRepo, err := git.New(exec.New()).Discover(ctx, dir) + if err != nil { + t.Fatal(err) + } + err = b.Update(ctx, wtRepo, func(g *stack.Graph) error { + g.Stacks = append(g.Stacks, stack.Stack{Trunk: "main", Branches: []stack.Branch{{Name: "f"}}}) + return nil + }) + if err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(filepath.Join(wtRepo.GitDir, FileName)) + if err != nil { + t.Fatal(err) + } + if f, err := parseFile(data); err != nil || len(f.Stacks) != 1 || f.Stacks[0].Branches[0].Branch != "f" { + t.Errorf("worktree file = %s (%v)", data, err) + } + if main, _ := os.ReadFile(mainPath); string(main) != mainFile { + t.Errorf("the main file changed:\n%s", main) + } +} + func TestUpdateCreatesTheFileForTheFirstStack(t *testing.T) { gittest.Isolate(t) dir := gittest.InitRepo(t)