Merge v0.63.1 to master#5820
Merged
Merged
Conversation
RunWithOutput and RunWithOutputs each carried their own near-identical copy of the index.lock retry loop. Extract the loop into a single retryOnLockError helper so the retry policy lives in one place, ahead of changing that policy. Behavior is unchanged; the added tests characterize it (success and non-lock errors run once, a lock error in the output is retried). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The gpg helper runs commands like amend with StreamOutput, so their output isn't captured and a failed run returns an empty output string; the index.lock message is carried by the error instead. isRetryableError only inspects the output, so the retry loop never fires for these commands. In practice this means a `shift-A` amend issued while a foreground `git status` refresh briefly holds index.lock fails outright instead of retrying. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Have isRetryableError also inspect the returned error, not just the captured output. Streamed commands (amend, commit, and other operations run through the gpg helper) don't capture output, so their index.lock failures were slipping past the retry loop and surfacing to the user as a hard "Git command failed". Now they retry like every other command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The retry check matched the literal ".git/index.lock", which only ever appears for the main worktree. A linked worktree's lock is at .git/worktrees/<name>/index.lock and a submodule's is under its own git dir, so contention there was never retried. Match the bare "index.lock" fragment instead, which covers all of them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The retry budget was five fixed 50ms waits (250ms total). A foreground `git status` refresh can hold index.lock for longer than that on a large repo, so the retries could be exhausted before the lock clears. Wait 20ms before the first retry and double each time, giving seven attempts over a bit more than a second — enough to outlast a slow refresh while keeping the common case (a lock that clears almost immediately) fast. The initial delay is now a runner field so tests can zero it out instead of sleeping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In v0.63.0 we made a change to no longer use `GIT_OPTIONAL_LOCKS=0` on git commands that are part of a "foreground" refresh, meaning the refresh after a lazygit command or the focus-in refresh. We do this on purpose to keep git's mod date cache from becoming stale, which could make lazygit become slower over time. However, this caused a problem for users who work very fast: staging a file and then immediately pressing shift-A to amend while the staging's refresh is still running would show the dreaded index.lock error. We already had a retry-on-index-lock-error mechanism in place, but it wasn't used for commands like amend or commit; fix this so that the retry loop works for these too, and also make the retry window a little longer, and fix a problem where it wouldn't work in linked worktrees or submodules. Closes #5778.
Update and friends enqueued onto a fixed 256-slot channel with a non-blocking send that panicked when the channel was full. That guard was firing in real use: - Toggling a directory of several hundred files into a custom patch (reliably): the operation runs on a worker behind a waiting status, whose spinner enqueues a content-only render on every tick, and over the long operation these outrun the UI loop and overflow the buffer. - Editing the config in an editor that suspends lazygit: the editor subprocess runs on the UI thread, so the loop drains nothing for the whole editing session, and the full refresh fired on resume fans out across every scope at once — a burst of updates that overflows before the just-resumed loop catches up. - Any time the UI thread blocks for a long time, the periodic refreshes keep enqueuing and eventually overflow. The 256-slot buffer was chosen deliberately, with the panic as a "should never happen" guard, to preserve two properties: FIFO ordering of same-goroutine Update calls (an earlier goroutine-per-Update design reordered them), and no self-deadlock (a blocking send from the UI thread would block against the loop that drains it). But a fixed channel can only offer those by crashing on overflow. Replace it with an unbounded, order-preserving queue: a mutex-guarded slice plus a buffered(1) doorbell channel that wakes the main loop's select. Enqueuing appends and rings the doorbell; the loop drains the slice to empty on each wake. This keeps FIFO order and never blocks the caller, so there is no self-deadlock and no overflow to panic on — under a stall the queue just grows and then drains. This also removes an inconsistency: updateContentOnly did a plain blocking send while update panicked, so the two paths disagreed on what happened when the queue was full. Both now share the same enqueue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Now that the queue is unbounded, its depth is a useful signal for understanding how the event loop behaves under load — and we expect it to look very different across builds (e.g. master, which carries the bounce-state-updates-to-ui-thread work, versus the v0.63.0 release this fix ships in). Track the deepest the queue has ever been and log an Info line whenever that record is broken, so the numbers show up in the log for later reasoning. The mark is session-wide and doesn't reset when the queue drains. gocui has no logger of its own, so it exposes the new depth through a handler (matching the existing SetFocusHandler / SetOpenHyperlinkFunc pattern) that the gui registers to log via its own logger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In #5756 we changed the userEvents channel to a fixed 256-slot channel with a non-blocking send that panicked when the channel was full. It turns out that this panic can happen in real use: - Toggling a directory of several hundred files into a custom patch (reliably): the operation runs on a worker behind a waiting status, whose spinner enqueues a content-only render on every tick, and over the long operation these outrun the UI loop and overflow the buffer. - Editing the config in an editor that suspends lazygit: the editor subprocess runs on the UI thread, so the loop drains nothing for the whole editing session, and the full refresh fired on resume fans out across every scope at once — a burst of updates that overflows before the just-resumed loop catches up. - Any time the UI thread blocks for a long time, the periodic refreshes keep enqueuing and eventually overflow. The 256-slot buffer was chosen deliberately, with the panic as a "should never happen" guard, to preserve two properties: FIFO ordering of same-goroutine Update calls (an earlier goroutine-per-Update design reordered them), and no self-deadlock (a blocking send from the UI thread would block against the loop that drains it). But a fixed channel can only offer those by crashing on overflow. Replace it with an unbounded, order-preserving queue: a mutex-guarded slice plus a buffered(1) doorbell channel that wakes the main loop's select. Enqueuing appends and rings the doorbell; the loop drains the slice to empty on each wake. This keeps FIFO order and never blocks the caller, so there is no self-deadlock and no overflow to panic on — under a stall the queue just grows and then drains. Fixes #5772.
winPty.Close could block indefinitely, and it is called while holding the global PtyMutex and while the task's onDone sync.Once is executing, so blocking there wedges the task's entire cleanup chain: the next NewTask call blocks on <-notifyStopped while holding waitingMutex, every later task for that view queues up behind it, and onResize blocks on PtyMutex — a full UI freeze. (Reported by a user via go-deadlock's 30s watchdog; a regression from the ConPTY support introduced for v0.63.0.) ClosePseudoConsole is what blocks; before Windows 11 24H2 it can do so in two ways. It flushes the client's pending output into the out pipe, but a stopped task's scanner goroutine has already quit draining, so with a client that's still producing output the flush never completes; this can also wedge the background waiter's closeHpc, which runs with the pipes deliberately left open. And it waits for the console host to exit, but closing only delivers CTRL_CLOSE_EVENT to the attached client without terminating it, so a client that keeps running (git still computing an expensive diff, a pager waiting for input) keeps the host alive arbitrarily long. Run the teardown on a background goroutine so Close returns immediately no matter which of these strikes, and within it close our pipe ends before the pseudoconsole, without taking p.mu: breaking the pipes fails a pending flush fast, which also unblocks a waiter already stuck in one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Windows PTY support that was newly introduced in v0.63.0 had a potential deadlock problem: when switching between longer diffs, lazygit could lock up. This should hopefully be fixed with this PR.
The Get Latest Tag step used to pick the most recently created tag in the entire repo, regardless of whether it is reachable from the commit being released. In preparation for supporting releases from branches other than master, use the nearest tag that is an ancestor of the checked-out commit instead. This way, a patch release cut from an older release branch bumps that branch's own latest tag even when master already carries a newer release, and the "changes since last release" check compares against the release that actually precedes this one in history.
When creating a patch release from a branch called `v0.63.1`, the new tag would get the same name and pushing it would fail with `error: src refspec v0.63.1 matches more than one`.
This is useful for cutting a patch release for the previous version when master already contains work that shouldn't be released yet; for example, v0.63.1 had to be tagged and released by hand from a v0.63.1 branch off the v0.63.0 tag because the workflow could only release master. Scheduled runs are unaffected: with no input provided, the ref is empty and the checkout falls back to the default branch.
I keep getting slightly confused as to which is which, so make this extra clear. While at it, change the default to minor, this is the option that is more often used now that we don't have regular scheduled releases any more.
This is useful for cutting a patch release for the previous version when master already contains work that shouldn't be released yet. Scheduled runs are unaffected: with no input provided, the ref is empty and the checkout falls back to the default branch.
Resolve the pkg/gocui/gui.go conflict by keeping master's background-task structure (Update/update(background), taskManager) and applying the unbounded user-event queue on top — the same end state as if the fix had been written on master directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stefanhaller
added
the
ignore-for-release
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.