Repository navigation
EXC-1975 Git.async: call back on error, spawn via jobstart - #43
Merged
Merged
Conversation
A raise inside the coroutine re-raised from a scheduled callback, so on_done never ran and every waiter on a fork-point measure stayed pending. step now reports the error with vim.notify and calls on_done(nil), which every consumer already reads as no fork point.
Each vim.system call blocks the main loop for about 2 ms idle and 8 ms under load; jobstart blocks for under a millisecond. Git.async now spawns through a local spawn() on jobstart, with stdout buffered and stdin closed. Git.system stays on vim.system for the diff, herdr and pr_target. The specs that hold or record async git now stub vim.fn.jobstart.
- Git.async must start on the main loop: in a fast event jobstart can't spawn, and the command reads as failed - spawn calls on_exit on the main loop; Spawned's code is -1 when the command could not start - fork_point.get_async hands nil when measuring raised an error - the vimdoc says only an error inside changeset is reported, not a failing git command
- spawn's buffered line list is `lines`, so the result's `stdout` field is the only `stdout` - tree_lifecycle_spec's release loop walks held `call`s, which are a spawner and its arguments, not processes
debug.traceback hands a table error back unchanged, so the notify's concatenation raised and stranded every waiter again. The value goes through tostring first. git_spec's async specs now restore vim.system and vim.notify in an after_each, so a run that times out can't leak them, and the vim.system check counts calls instead of raising inside a pcall.
Git.system now runs on jobstart, whose spawn blocks the main loop a fraction as long as vim.system's, and Git.async's private spawn folds into it, so the diff refresh, herdr and the gh query stop paying vim.system's stall too. - output comes back byte-exact: jobstart's line lists carry NUL as a newline inside an item, so it is turned back before joining; CRLF reads as LF, as vim.system's text mode did - stdin goes in through chansend; a timeout stops the job and reports 124; a failed start reports -1 with the reason - the result type drops signal and makes stdout/stderr strings, so the callers' `or ""` guards go - the specs hold Git.system calls through one spawns.holding helper, as closures, instead of three jobstart stubs returning a fake job id
- a command exiting just as its timeout fires reported 124; the timer
now marks a timeout only when jobstop finds the job still running
- joined rewrites only the lines holding a NUL and only CRLF output, so
a megabyte of diff no longer costs more than vim.system did
- docs: the runner's summary leaves out the blocking sync fallback,
stdin/stderr say what they carry, and CRLF says who needs it
- specs: the vim.system guard moves to describe("system"), stubs sit
at Git.system, notify is captured per block, and the held calls'
class is support.spawns.Held
The spec leaned on the fake gh's 1 s sleep to keep its ask in flight while HEAD moved. Under full-suite load the moves outran it, gh answered no PR first, and the second build rightly asked again: about a third of loaded runs failed, on main as on this branch. gh's call is now held until the second build has run.
- Git.system runs gh, herdr and any argv, with no git in it; it becomes Process.run in lua/changeset/process.lua, with its Opts and Completed types - git.lua is back to git and gh queries; Git.async, pr_target, diff and herdr call Process.run through the module table, so spawns.holding still holds every start - its specs move from git_spec to process_spec
- spawns.failing(fn, ...) runs fn with jobstart raising E2BIG and restores it even when fn raises, as gh.without does for PATH - process_spec, git_spec's pr_target and herdr_spec use it instead of hand-rolled stubs that leaked jobstart on a raise
- fork_point, review, review_switch and build specs that set FAKE_GH_DELAY so gh answered "after" something now hold gh through spawns.holding and release it where the answer belongs; two whose ordering was already fixed just drop the delay - the fake gh no longer sleeps, and FAKE_GH_DELAY is gone - build's "measures against the PR's target at once when switching back" passed without measuring since builds went async; it now waits for each build and fails when the target cache breaks - review's "drops a PR lookup that a branch switch superseded" now advances main so a stale answer would move its buffers, and fails when follow stops dropping it
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.
Part of: EXC-1975
Change Kind: bug fix
What changed
An error raised while changeset measured the fork point in the background used to escape from a scheduled callback. Whatever was waiting on that measure (a build, the sidebar opening, the gutter's base) then stayed pending for good.
Git.asyncnow reports the error throughvim.notify, whatever value it is, and calls back with nil, which every caller already reads as "no fork point".The diff refresh, herdr, the gh query and
Git.asyncall start processes through one runner. It moves out ofgit.luaintolua/changeset/process.luaasProcess.run, and starts processes throughjobstartinstead ofvim.system, whose spawn holds up the editor several times longer. Output stays byte-exact (NUL bytes survive, and CRLF still reads as LF), stdin is piped, and a timeout reports code 124.The specs that timed gh with a sleep (
FAKE_GH_DELAY) now hold gh's call instead, and the fake gh no longer sleeps. One of them had failed about a third of the time under load, on main too. Two others had been passing without exercising the check they were written for, and now fail when it breaks.What to test
mise run test tests/git_spec.lua tests/process_spec.lua:Git.async, string or not, shows oneChangeset:error with a traceback, and the callback gets nil;Process.run's specs cover NUL bytes, stdin, stderr, CRLF, the timeout, and a start that fails.linguist-generated, whose path reachesgit check-attron stdin.^M, and sending a review to a herdr agent still works.E475 … expected valid directoryinstead ofENOENT.