Skip to content

Commit 37e32a5

Browse files
skarimskarim
committed
only lock for writes
1 parent 211e563 commit 37e32a5

14 files changed

Lines changed: 58 additions & 147 deletions

File tree

‎cmd/add.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cmd
22

33
import (
4-
"errors"
54
"fmt"
65

76
"github.com/cli/go-gh/v2/pkg/prompter"
@@ -52,12 +51,8 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {
5251

5352
result, err := loadStack(cfg, "")
5453
if err != nil {
55-
if errors.Is(err, ErrLockFailed) {
56-
return ErrLockFailed
57-
}
5854
return ErrNotInStack
5955
}
60-
defer result.Lock.Unlock()
6156
gitDir := result.GitDir
6257
sf := result.StackFile
6358
s := result.Stack
@@ -205,8 +200,7 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {
205200
}
206201

207202
if err := stack.Save(gitDir, sf); err != nil {
208-
cfg.Errorf("failed to save stack state: %s", err)
209-
return ErrSilent
203+
return handleSaveError(cfg, err)
210204
}
211205

212206
// Print summary

‎cmd/init.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,6 @@ func runInit(cfg *config.Config, opts *initOptions) error {
7373
}
7474

7575
// Load existing stack file
76-
lock, err := stack.Lock(gitDir)
77-
if err != nil {
78-
cfg.Errorf("another process is currently editing the stack — try again later")
79-
return ErrLockFailed
80-
}
81-
defer lock.Unlock()
82-
8376
sf, err := stack.Load(gitDir)
8477
if err != nil {
8578
cfg.Errorf("failed to load stack state: %s", err)
@@ -334,7 +327,7 @@ func runInit(cfg *config.Config, opts *initOptions) error {
334327
syncStackPRs(cfg, &sf.Stacks[len(sf.Stacks)-1])
335328

336329
if err := stack.Save(gitDir, sf); err != nil {
337-
return err
330+
return handleSaveError(cfg, err)
338331
}
339332

340333
// Print result

‎cmd/merge.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cmd
22

33
import (
4-
"errors"
54
"fmt"
65

76
"github.com/cli/go-gh/v2/pkg/browser"
@@ -36,12 +35,8 @@ func runMerge(cfg *config.Config, target string) error {
3635
// Standard stack loading and validation.
3736
result, err := loadStack(cfg, "")
3837
if err != nil {
39-
if errors.Is(err, ErrLockFailed) {
40-
return ErrLockFailed
41-
}
4238
return ErrNotInStack
4339
}
44-
defer result.Lock.Unlock()
4540
s := result.Stack
4641
currentBranch := result.CurrentBranch
4742

‎cmd/navigate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func BottomCmd(cfg *config.Config) *cobra.Command {
6969
}
7070

7171
func runNavigate(cfg *config.Config, delta int) error {
72-
result, err := loadStackReadOnly(cfg, "")
72+
result, err := loadStack(cfg, "")
7373
if err != nil {
7474
return ErrNotInStack
7575
}
@@ -183,7 +183,7 @@ func runNavigate(cfg *config.Config, delta int) error {
183183
}
184184

185185
func runNavigateToEnd(cfg *config.Config, top bool) error {
186-
result, err := loadStackReadOnly(cfg, "")
186+
result, err := loadStack(cfg, "")
187187
if err != nil {
188188
return ErrNotInStack
189189
}

‎cmd/push.go‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,6 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
4545
return ErrNotInStack
4646
}
4747

48-
lock, err := stack.Lock(gitDir)
49-
if err != nil {
50-
cfg.Errorf("another process is currently editing the stack — try again later")
51-
return ErrLockFailed
52-
}
53-
defer lock.Unlock()
54-
5548
sf, err := stack.Load(gitDir)
5649
if err != nil {
5750
cfg.Errorf("failed to load stack state: %s", err)
@@ -187,8 +180,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
187180
syncStackPRs(cfg, s)
188181

189182
if err := stack.Save(gitDir, sf); err != nil {
190-
cfg.Errorf("failed to save stack state: %s", err)
191-
return ErrSilent
183+
return handleSaveError(cfg, err)
192184
}
193185

194186
cfg.Successf("Pushed and synced %d branches", len(s.ActiveBranches()))

‎cmd/rebase.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,8 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {
8484

8585
result, err := loadStack(cfg, opts.branch)
8686
if err != nil {
87-
if errors.Is(err, ErrLockFailed) {
88-
return ErrLockFailed
89-
}
9087
return ErrNotInStack
9188
}
92-
defer result.Lock.Unlock()
9389
sf := result.StackFile
9490
s := result.Stack
9591
currentBranch := result.CurrentBranch
@@ -341,13 +337,6 @@ func continueRebase(cfg *config.Config, gitDir string) error {
341337
return ErrSilent
342338
}
343339

344-
lock, err := stack.Lock(gitDir)
345-
if err != nil {
346-
cfg.Errorf("another process is currently editing the stack — try again later")
347-
return ErrLockFailed
348-
}
349-
defer lock.Unlock()
350-
351340
sf, err := stack.Load(gitDir)
352341
if err != nil {
353342
cfg.Errorf("failed to load stack state: %s", err)

‎cmd/sync.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,8 @@ conflicts interactively.`,
4747
func runSync(cfg *config.Config, opts *syncOptions) error {
4848
result, err := loadStack(cfg, "")
4949
if err != nil {
50-
if errors.Is(err, ErrLockFailed) {
51-
return ErrLockFailed
52-
}
5350
return ErrNotInStack
5451
}
55-
defer result.Lock.Unlock()
5652
gitDir := result.GitDir
5753
sf := result.StackFile
5854
s := result.Stack
@@ -292,8 +288,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
292288
updateBaseSHAs(s)
293289

294290
if err := stack.Save(gitDir, sf); err != nil {
295-
cfg.Errorf("failed to save stack state: %s", err)
296-
return ErrSilent
291+
return handleSaveError(cfg, err)
297292
}
298293

299294
cfg.Printf("")

‎cmd/unstack.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package cmd
22

33
import (
4-
"errors"
5-
64
"github.com/github/gh-stack/internal/config"
75
"github.com/github/gh-stack/internal/stack"
86
"github.com/spf13/cobra"
@@ -37,12 +35,8 @@ func UnstackCmd(cfg *config.Config) *cobra.Command {
3735
func runUnstack(cfg *config.Config, opts *unstackOptions) error {
3836
result, err := loadStack(cfg, opts.target)
3937
if err != nil {
40-
if errors.Is(err, ErrLockFailed) {
41-
return ErrLockFailed
42-
}
4338
return ErrNotInStack
4439
}
45-
defer result.Lock.Unlock()
4640
gitDir := result.GitDir
4741
sf := result.StackFile
4842
s := result.Stack
@@ -56,8 +50,7 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error {
5650
// Remove from local tracking
5751
sf.RemoveStackForBranch(target)
5852
if err := stack.Save(gitDir, sf); err != nil {
59-
cfg.Errorf("failed to save stack state: %s", err)
60-
return ErrSilent
53+
return handleSaveError(cfg, err)
6154
}
6255
cfg.Successf("Stack removed from local tracking")
6356

‎cmd/utils.go‎

Lines changed: 13 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -71,34 +71,23 @@ type loadStackResult struct {
7171
StackFile *stack.StackFile
7272
Stack *stack.Stack
7373
CurrentBranch string
74-
Lock *stack.FileLock // caller must defer Lock.Unlock() if non-nil
7574
}
7675

7776
// loadStack is the standard way to obtain a Stack for the current (or given)
7877
// branch. It resolves the git directory, loads the stack file, determines the
7978
// branch, calls resolveStack (which may prompt for disambiguation), checks for
8079
// a nil stack, and re-reads the current branch (in case disambiguation caused
8180
// a checkout). Errors are printed via cfg and returned.
81+
//
82+
// loadStack does NOT acquire the stack file lock. The lock is acquired
83+
// automatically by stack.Save() when writing.
8284
func loadStack(cfg *config.Config, branch string) (*loadStackResult, error) {
8385
gitDir, err := git.GitDir()
8486
if err != nil {
8587
cfg.Errorf("not a git repository")
8688
return nil, fmt.Errorf("not a git repository")
8789
}
8890

89-
lock, err := stack.Lock(gitDir)
90-
if err != nil {
91-
cfg.Errorf("another process is currently editing the stack — try again later")
92-
return nil, ErrLockFailed
93-
}
94-
// Release the lock if we fail before returning it to the caller.
95-
success := false
96-
defer func() {
97-
if !success {
98-
lock.Unlock()
99-
}
100-
}()
101-
10291
sf, err := stack.Load(gitDir)
10392
if err != nil {
10493
cfg.Errorf("failed to load stack state: %s", err)
@@ -140,71 +129,25 @@ func loadStack(cfg *config.Config, branch string) (*loadStackResult, error) {
140129
return nil, fmt.Errorf("failed to get current branch: %w", err)
141130
}
142131

143-
success = true
144132
return &loadStackResult{
145133
GitDir: gitDir,
146134
StackFile: sf,
147135
Stack: s,
148136
CurrentBranch: currentBranch,
149-
Lock: lock,
150137
}, nil
151138
}
152139

153-
// loadStackReadOnly loads the stack without acquiring an exclusive lock.
154-
// Use this for commands that only read the stack and never call Save().
155-
func loadStackReadOnly(cfg *config.Config, branch string) (*loadStackResult, error) {
156-
gitDir, err := git.GitDir()
157-
if err != nil {
158-
cfg.Errorf("not a git repository")
159-
return nil, fmt.Errorf("not a git repository")
160-
}
161-
162-
sf, err := stack.Load(gitDir)
163-
if err != nil {
164-
cfg.Errorf("failed to load stack state: %s", err)
165-
return nil, fmt.Errorf("failed to load stack state: %w", err)
166-
}
167-
168-
branchFromArg := branch != ""
169-
if branch == "" {
170-
branch, err = git.CurrentBranch()
171-
if err != nil {
172-
cfg.Errorf("failed to get current branch: %s", err)
173-
return nil, fmt.Errorf("failed to get current branch: %w", err)
174-
}
175-
}
176-
177-
s, err := resolveStack(sf, branch, cfg)
178-
if err != nil {
179-
if errors.Is(err, errInterrupt) {
180-
return nil, errInterrupt
181-
}
182-
cfg.Errorf("%s", err)
183-
return nil, err
184-
}
185-
if s == nil {
186-
if branchFromArg {
187-
cfg.Errorf("branch %q is not part of a stack", branch)
188-
} else {
189-
cfg.Errorf("current branch %q is not part of a stack", branch)
190-
}
191-
cfg.Printf("Checkout an existing stack using `%s` or create a new stack using `%s`",
192-
cfg.ColorCyan("gh stack checkout"), cfg.ColorCyan("gh stack init"))
193-
return nil, fmt.Errorf("branch %q is not part of a stack", branch)
194-
}
195-
196-
currentBranch, err := git.CurrentBranch()
197-
if err != nil {
198-
cfg.Errorf("failed to get current branch: %s", err)
199-
return nil, fmt.Errorf("failed to get current branch: %w", err)
140+
// handleSaveError translates a stack.Save error into the appropriate user
141+
// message and exit error. Lock failures return ErrLockFailed (exit 8);
142+
// other write failures return ErrSilent (exit 1).
143+
func handleSaveError(cfg *config.Config, err error) error {
144+
var lockErr *stack.LockError
145+
if errors.As(err, &lockErr) {
146+
cfg.Errorf("another process is currently editing the stack — try again later")
147+
return ErrLockFailed
200148
}
201-
202-
return &loadStackResult{
203-
GitDir: gitDir,
204-
StackFile: sf,
205-
Stack: s,
206-
CurrentBranch: currentBranch,
207-
}, nil
149+
cfg.Errorf("failed to save stack state: %s", err)
150+
return ErrSilent
208151
}
209152

210153
// resolveStack finds the stack for the given branch, handling ambiguity when

‎cmd/view.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package cmd
33
import (
44
"bytes"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"os"
98
"os/exec"
@@ -44,18 +43,14 @@ func ViewCmd(cfg *config.Config) *cobra.Command {
4443
func runView(cfg *config.Config, opts *viewOptions) error {
4544
result, err := loadStack(cfg, "")
4645
if err != nil {
47-
if errors.Is(err, ErrLockFailed) {
48-
return ErrLockFailed
49-
}
5046
return ErrNotInStack
5147
}
52-
defer result.Lock.Unlock()
5348
gitDir := result.GitDir
5449
sf := result.StackFile
5550
s := result.Stack
5651
currentBranch := result.CurrentBranch
5752

58-
// Sync PR state
53+
// Sync PR state and save (best-effort).
5954
syncStackPRs(cfg, s)
6055
_ = stack.Save(gitDir, sf)
6156

0 commit comments

Comments
 (0)