Skip to content
Merged
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,4 @@ Compared to the typical workflow, there's no need to name branches, run `git add
| 5 | Invalid arguments or flags |
| 6 | Disambiguation required (branch belongs to multiple stacks) |
| 7 | Rebase already in progress |
| 8 | Stack is locked by another process |
3 changes: 1 addition & 2 deletions cmd/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error {
}

if err := stack.Save(gitDir, sf); err != nil {
cfg.Errorf("failed to save stack state: %s", err)
return ErrSilent
return handleSaveError(cfg, err)
}

// Print summary
Expand Down
2 changes: 1 addition & 1 deletion cmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func runInit(cfg *config.Config, opts *initOptions) error {
syncStackPRs(cfg, &sf.Stacks[len(sf.Stacks)-1])

if err := stack.Save(gitDir, sf); err != nil {
return err
return handleSaveError(cfg, err)
}

// Print result
Expand Down
2 changes: 1 addition & 1 deletion cmd/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func runMerge(cfg *config.Config, target string) error {
syncStackPRs(cfg, s)

// Persist the refreshed PR state.
_ = stack.Save(result.GitDir, result.StackFile)
stack.SaveNonBlocking(result.GitDir, result.StackFile)

// Resolve which branch to operate on.
var br *stack.BranchRef
Expand Down
3 changes: 1 addition & 2 deletions cmd/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,7 @@ func runPush(cfg *config.Config, opts *pushOptions) error {
syncStackPRs(cfg, s)

if err := stack.Save(gitDir, sf); err != nil {
cfg.Errorf("failed to save stack state: %s", err)
return ErrSilent
return handleSaveError(cfg, err)
}

cfg.Successf("Pushed and synced %d branches", len(s.ActiveBranches()))
Expand Down
4 changes: 2 additions & 2 deletions cmd/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func runRebase(cfg *config.Config, opts *rebaseOptions) error {

syncStackPRs(cfg, s)

_ = stack.Save(gitDir, sf)
stack.SaveNonBlocking(gitDir, sf)

merged := s.MergedBranches()
if len(merged) > 0 {
Expand Down Expand Up @@ -496,7 +496,7 @@ func continueRebase(cfg *config.Config, gitDir string) error {

syncStackPRs(cfg, s)

_ = stack.Save(gitDir, sf)
stack.SaveNonBlocking(gitDir, sf)

cfg.Printf("All branches in stack rebased locally with %s", s.Trunk.Branch)
cfg.Printf("To push up your changes and open/update the stack of PRs, run `%s`",
Expand Down
5 changes: 2 additions & 3 deletions cmd/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
} else {
// Persist refreshed PR state even on conflict, then bail out
// before pushing or reporting success.
_ = stack.Save(gitDir, sf)
stack.SaveNonBlocking(gitDir, sf)
return ErrConflict
}
}
Expand Down Expand Up @@ -288,8 +288,7 @@ func runSync(cfg *config.Config, opts *syncOptions) error {
updateBaseSHAs(s)

if err := stack.Save(gitDir, sf); err != nil {
cfg.Errorf("failed to save stack state: %s", err)
return ErrSilent
return handleSaveError(cfg, err)
}

cfg.Printf("")
Expand Down
3 changes: 1 addition & 2 deletions cmd/unstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ func runUnstack(cfg *config.Config, opts *unstackOptions) error {
// Remove from local tracking
sf.RemoveStackForBranch(target)
if err := stack.Save(gitDir, sf); err != nil {
cfg.Errorf("failed to save stack state: %s", err)
return ErrSilent
return handleSaveError(cfg, err)
}
cfg.Successf("Stack removed from local tracking")

Expand Down
22 changes: 22 additions & 0 deletions cmd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var (
ErrInvalidArgs = &ExitError{Code: 5} // invalid arguments or flags
ErrDisambiguate = &ExitError{Code: 6} // multiple stacks/remotes, can't auto-select
ErrRebaseActive = &ExitError{Code: 7} // rebase already in progress
ErrLockFailed = &ExitError{Code: 8} // could not acquire stack file lock
)

// ExitError is returned by commands to indicate a specific exit code.
Expand Down Expand Up @@ -77,6 +78,9 @@ type loadStackResult struct {
// branch, calls resolveStack (which may prompt for disambiguation), checks for
// a nil stack, and re-reads the current branch (in case disambiguation caused
// a checkout). Errors are printed via cfg and returned.
//
// loadStack does NOT acquire the stack file lock. The lock is acquired
// automatically by stack.Save() when writing.
func loadStack(cfg *config.Config, branch string) (*loadStackResult, error) {
gitDir, err := git.GitDir()
if err != nil {
Expand Down Expand Up @@ -133,6 +137,24 @@ func loadStack(cfg *config.Config, branch string) (*loadStackResult, error) {
}, nil
}

// handleSaveError translates a stack.Save error into the appropriate user
// message and exit error. Lock contention and stale-file detection both
// return ErrLockFailed (exit 8); other write failures return ErrSilent (exit 1).
func handleSaveError(cfg *config.Config, err error) error {
var lockErr *stack.LockError
if errors.As(err, &lockErr) {
cfg.Errorf("another process is currently editing the stack — try again later")
return ErrLockFailed
}
var staleErr *stack.StaleError
if errors.As(err, &staleErr) {
cfg.Errorf("stack file was modified by another process — please re-run the command")
return ErrLockFailed
}
cfg.Errorf("failed to save stack state: %s", err)
return ErrSilent
Comment thread
skarim marked this conversation as resolved.
}
Comment thread
skarim marked this conversation as resolved.

// resolveStack finds the stack for the given branch, handling ambiguity when
// a branch (typically a trunk) belongs to multiple stacks. If exactly one
// stack matches, it is returned directly. If multiple stacks match, the user
Expand Down
4 changes: 2 additions & 2 deletions cmd/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ func runView(cfg *config.Config, opts *viewOptions) error {
s := result.Stack
currentBranch := result.CurrentBranch

// Sync PR state
// Sync PR state and save (best-effort).
syncStackPRs(cfg, s)
_ = stack.Save(gitDir, sf)
stack.SaveNonBlocking(gitDir, sf)

if opts.asJSON {
return viewJSON(cfg, s, currentBranch)
Expand Down
89 changes: 89 additions & 0 deletions internal/stack/lock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package stack

import (
"fmt"
"os"
"path/filepath"
"time"
)

const lockFileName = "gh-stack.lock"

// LockError is returned when the stack file lock cannot be acquired.
// Callers can check for this with errors.As to distinguish lock failures
// from other errors.
type LockError struct {
Err error
}

func (e *LockError) Error() string { return e.Err.Error() }
func (e *LockError) Unwrap() error { return e.Err }
Comment thread
skarim marked this conversation as resolved.

// StaleError is returned when the stack file was modified on disk since it
// was loaded. This indicates another process wrote to the file concurrently.
// Callers can check for this with errors.As.
type StaleError struct {
Err error
}

func (e *StaleError) Error() string { return e.Err.Error() }
func (e *StaleError) Unwrap() error { return e.Err }

// LockTimeout is how long Lock() will wait for the exclusive lock before
// giving up. With the lock held only during file writes (milliseconds),
// this timeout primarily guards against a hung process holding the lock.
var LockTimeout = 5 * time.Second

// lockRetryInterval is the sleep between non-blocking lock attempts.
const lockRetryInterval = 100 * time.Millisecond

// FileLock provides an exclusive advisory lock on the stack file to prevent
// concurrent writes between multiple gh-stack processes.
type FileLock struct {
f *os.File
}

// Lock acquires an exclusive lock on the stack file in the given git directory.
// It retries with a non-blocking attempt every 100ms for up to LockTimeout.
//
// Most callers should not use Lock directly — stack.Save() acquires the lock
// automatically. Use Lock only when you need to hold the lock across multiple
// operations (e.g. Load-Modify-Save as an atomic unit).
Comment thread
skarim marked this conversation as resolved.
func Lock(gitDir string) (*FileLock, error) {
path := filepath.Join(gitDir, lockFileName)
f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0644)
if err != nil {
return nil, fmt.Errorf("opening lock file: %w", err)
}

deadline := time.Now().Add(LockTimeout)
for {
err := tryLockFile(f)
if err == nil {
return &FileLock{f: f}, nil
}
if !isLockBusy(err) {
// Unexpected error (e.g. bad fd) — don't retry.
f.Close()
return nil, fmt.Errorf("locking stack file: %w", err)
}
if time.Now().After(deadline) {
f.Close()
return nil, &LockError{Err: fmt.Errorf(
"timed out waiting for stack lock after %s — another gh-stack process may be running", LockTimeout)}
}
time.Sleep(lockRetryInterval)
}
Comment thread
skarim marked this conversation as resolved.
}

// Unlock releases the lock. The lock file is intentionally left on disk to
// avoid a race where another process opens the same path, blocks on flock,
// then wakes up holding a lock on an unlinked inode while a third process
// creates a new file and locks a different inode.
func (l *FileLock) Unlock() {
if l == nil || l.f == nil {
return
}
unlockFile(l.f)
l.f.Close()
}
Loading
Loading